-
-
Notifications
You must be signed in to change notification settings - Fork 8.8k
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
Theme switch button position should be configurable #10881
Comments
Agree, this is a change we want to do, I even have todos in our code: export default function NavbarContent(): ReactNode {
const mobileSidebar = useNavbarMobileSidebar();
const items = useNavbarItems();
const [leftItems, rightItems] = splitNavbarItems(items);
const searchBarItem = items.find((item) => item.type === 'search');
return (
<NavbarContentLayout
left={
// TODO stop hardcoding items?
<>
{!mobileSidebar.disabled && <NavbarMobileSidebarToggle />}
<NavbarLogo />
<NavbarItems items={leftItems} />
</>
}
right={
// TODO stop hardcoding items?
// Ask the user to add the respective navbar items => more flexible
<>
<NavbarItems items={rightItems} />
<NavbarColorModeToggle className={styles.colorModeToggle} />
{!searchBarItem && (
<NavbarSearch>
<SearchBar />
</NavbarSearch>
)}
</>
}
/>
);
} Historically, the search and theme switch have been using hardcoded positions, and we tried to remain retrocompatible with that behavior. when introducing a new config.
This has been done this way for major version retrocompatibility. Changing that probably requires a breaking change because we don't want to impact the navbar order of existing sites in a minor version release (v3.x). I have my own idea, but I'd like to better understand what do you suggest us to change:
Worth mentioning: we also have a Worth mentioning: we have a "mobile drawer" too, where the navbar item position attribute have no impact, it's always rendered in the "drawer header" export default function NavbarMobileSidebarHeader(): ReactNode {
return (
<div className="navbar-sidebar__brand">
<NavbarLogo />
<NavbarColorModeToggle className="margin-right--md" />
<CloseButton />
</div>
);
} Can you please help us?
|
Thanks for your detailed response! I understand the need to keep things retrocompatible in minor versions. For a minor version (v3.x), maybe we could introduce a themeSwitchButton navbar item while keeping the default behavior unchanged. If the user explicitly sets a position, it could override the default without affecting existing sites. For a major version (v4.x), it might make sense to remove hardcoded positions completely and require users to explicitly define themeSwitchButton and search in the navbar items. This would make the navbar order more predictable and customizable. Do you have any timeline for v4? Just wondering if this change would be something to expect soon or later. |
Have you read the Contributing Guidelines on issues?
Description
Currently in the config file you can only either enable or disable the theme switch button through themeConfig->colorMode. But if there was an option of a navbar-item called themeSwitchButton and you could modify its position it would be good.
Example:
Has this been requested on Canny?
No response
Motivation
For example if you do not mention searchbar's position it is on the right of the theme switch button. But if you specifically say right, it moves to the left of the theme switch button. This is very unintuitive in my opinion. I believe the first item on the list that says right is on the leftmost side of the items on the right. With this simple feature we have more config options for the themeSwitchButton.
API design
No response
Have you tried building it?
No response
Self-service
The text was updated successfully, but these errors were encountered: