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.

Sign up for GitHub

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

Jump to bottom

Fix Navbar colors for dark theme with global data-bs-theme attribute #38807

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Fix Navbar colors for dark theme with global data-bs-theme attribute #38807

wants to merge 1 commit into from
+2 −1

Conversation

itchyny
Copy link

@itchyny itchyny commented Jun 22, 2023

Description

This PR implements toggling Navbar colors for color modes with global data-bs-theme attribute.

Motivation & Context

The dark mode can be toggled with the data-bs-theme attribute on <html> element, as documented as follows.

As shown above, color mode styles are controlled by the data-bs-theme attribute. This attribute can be applied to the <html> element, or to any other element or Bootstrap component. If applied to the <html> element, it will apply to everything. If applied to a component or element, it will be scoped to that specific component or element.
https://getbootstrap.com/docs/5.3/customize/color-modes/
https://github.com/twbs/bootstrap/blob/v5.3.0/site/content/docs/5.3/customize/color-modes.md

Type of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Refactoring (non-breaking change)
  • Breaking change (fix or feature that would change existing functionality)

Checklist

  • I have read the contributing guidelines
  • My code follows the code style of the project (using npm run lint)
  • My change introduces changes to the documentation
  • I have updated the documentation accordingly
  • I have added tests to cover my changes
  • All new and existing tests passed

Live previews

Related issues

@itchyny itchyny requested a review from a team as a code owner June 22, 2023 03:37
@mdo
Copy link
Member

mdo commented Jun 25, 2023

Doesn't this just replicate the behavior that's already in our code base? When flipping into dark mode in our docs, our navbars automatically move to a dark mode version of themselves.

@itchyny
Copy link
Author

itchyny commented Jun 26, 2023

Sorry, I noticed that this selector is not applied even in the dark mode. The colors are already responsive (by the .navbar selector itself), the toggle icon is configured in the following selector, so these lines are just unused.

@itchyny itchyny closed this Jun 26, 2023
@itchyny itchyny reopened this Jul 1, 2023
@itchyny
Copy link
Author

itchyny commented Jul 1, 2023

There are color variables for navbar in dark theme, such as $navbar-dark-color and $navbar-dark-hover-color, but they are effective only when the theme attribute is added to the component. Thus theme switch toggle needs to implement something like follows.

function setTheme(theme) {
    document.documentElement.setAttribute('data-bs-theme', theme);
    document.querySelectorAll('.navbar').forEach(e => e.setAttribute('data-bs-theme', theme));
}

But this is ugly, and does not follow the documentation of color modes that explains about the global data-bs-theme attribute. The Bootstrap default theme sets theme-responsive colors for $navbar-light-color (strictly speaking the colors is defined using the emphasis colors which are responsive against the global attribute), but when users set each variables to the specific colors like bootswatch, the navbar colors does not follow the global data-bs-theme attribute.

@itchyny itchyny mentioned this pull request Jul 1, 2023
Open
@louismaximepiton louismaximepiton mentioned this pull request Jul 17, 2023
Open
9 tasks
@julien-deramond julien-deramond mentioned this pull request Sep 11, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
color mode css v5
Projects
v5.3.4
Status: Needs review
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@itchyny @mdo @julien-deramond