Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added Active Links to Settings Section for Better Navigation #47

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

kzamanbd
Copy link

@kzamanbd kzamanbd commented Mar 3, 2025

This PR improves the Settings section by adding active links to enhance user navigation. Changes include:

  • Added active link states to improve usability.
  • Ensured consistent styling and accessibility.
  • Minor UI tweaks for a better user experience.

Let me know if you want any refinements! 🚀

@tnylea
Copy link
Contributor

tnylea commented Mar 3, 2025

@kzamanbd, thank you. Can you please submit screenshots of the UI changes.

Additionally, we have a PR here: #41 that is also trying to resolve the active states of those buttons. I feel like adding the current attribute may be a better approach. Let me know your thoughts. Thanks.

@tnylea tnylea added the Awaiting User Response Waiting for developers response label Mar 3, 2025
@kzamanbd
Copy link
Author

kzamanbd commented Mar 4, 2025

@tnylea Thanks for the feedback!

Here is my changes as like you mentioned PR

Screenshot 2025-03-04 at 9 42 29 AM

I also think :current="request()->routeIs('settings.profile')" will be a better approach

@alex-coding-alex
Copy link

alex-coding-alex commented Mar 5, 2025

image

If the APP_URL is set and the browser site matches that completely, then active links show fine. Just cloned the starter kit and the initial url was. http://127.0.0.1:8000, my default APP_URL was http://localhost.

After I added the port number and used localhost instead of 127.0.0.1, the active links shows. Though, it does seem like it'd trip people up in dev.

@kzamanbd kzamanbd closed this Mar 5, 2025
@kzamanbd kzamanbd deleted the enhancement/ui-improvements branch March 5, 2025 09:28
@kzamanbd kzamanbd restored the enhancement/ui-improvements branch March 5, 2025 12:41
@kzamanbd kzamanbd reopened this Mar 5, 2025
@tnylea
Copy link
Contributor

tnylea commented Mar 7, 2025

Ok. Yeah, perhaps this is something that we should add to the documentation or maybe we can comment somewhere in that file.

Let's leave the settings/layout.blade.php the way it is for now and if this keeps coming up maybe we'll reconsider. If you want to submit your other CSS class updates without that settings/layout.blade.php in here I can approve and merge that.

Go ahead and re-checkout the settings/layout.blade.php and we can get this merged in. Thanks!

@tnylea tnylea mentioned this pull request Mar 7, 2025
This reverts commit 74678fe.
@kzamanbd
Copy link
Author

kzamanbd commented Mar 7, 2025

I've left the settings/layout.blade.php file in my pull request

@tnylea
Copy link
Contributor

tnylea commented Mar 19, 2025

Thanks @kzamanbd,

This will add the active state to the buttons even if the APP_URL is not set correctly. This has come up a few times, so it'll be good to have this in there to make sure it's always active for the correct URI segment.

Appreciate it.

@tnylea tnylea added Approved Approved for Merge and removed Awaiting User Response Waiting for developers response labels Mar 19, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Approved Approved for Merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants