-
Notifications
You must be signed in to change notification settings - Fork 81
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
refactor: enable children on navbar switcher #2863
refactor: enable children on navbar switcher #2863
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm!
Hey Massao, I strongly recommend removing that behavior as it’s a bit misleading. The label 'Account Settings' should be shown in the page header. I'm working on a proposal that will eliminate the need to change the main navigation for different settings areas (org, space, account). In the meantime, I suggest we use the breadcrumb in the header to display the 'Account Settings' label and remove it from the main navigation. |
@damann the string is not hardcoded in there, it only allows us to also hand over strings. |
Thanks. That’s fine, but there shouldn’t be any text in that place that resembles the environment switcher. I hadn’t specified this earlier as I only came across it recently, and it got buried under other priorities 🙇. As mentioned, I’m working on a better alternative, but for now, the interim solution should be to remove or hide it. |
Purpose of PR
In some use cases we don't have space and environment, in those cases we pass a string that will appear on the switcher
Preview