feat: Improve navbar compatiblity for Bootstrap 5 and light/dark modes#1145
Merged
gadenbuie merged 16 commits intoJan 22, 2025
Conversation
This rule only changes the background color of the navbar, which is not enough to fully restyle the navbar in dark mode. It might work in the default cases, but it certainly does not work if users have set $navbar-dark-bg or $navbar-light-bg to appropriate values. Note that Bootstrap expects the navbar colors to be static (i.e. the same in light and dark mode), so if we want to support different navbar colors we'll need to implement something better.
Bootstrap has the navbar toggler icon follow the global color mode, which makes the icon invisible when placed on a light background in dark mode. Outside of this rule, Bootstrap expects the navbar color to be consistent in light and dark mode.
…ty possible Otherwise they get in the way of local classes, e.g. `bg-blue` or `bg-primary`, which are the primary mechanism for setting navbar color in BS5 https://getbootstrap.com/docs/5.3/components/navbar/#color-schemes
…imal specificity
…lity classes will win
gadenbuie
force-pushed
the
feat/bs5-navbar-bg-dark-light
branch
from
December 4, 2024 15:42
ed14acf to
fd2e808
Compare
Collaborator
|
Before I dive into this, I just did a quick run of these examples with |
Member
Author
The first option only works in the Shiny preset and it works in a way that it breaks the second option. #1135 shows the problematic behavior. |
gadenbuie
force-pushed
the
feat/bs5-navbar-bg-dark-light
branch
from
December 4, 2024 19:09
c1e2ec7 to
533fa64
Compare
cpsievert
approved these changes
Jan 21, 2025
gadenbuie
added a commit
that referenced
this pull request
Jan 22, 2025
* fix: apply `data-bs-theme="dark"` to `<nav>` when `inverse = TRUE` * feat(navbar): Set `data-bs-theme` from `inverse` Lets `inverse` be a character value that is used directly for `data-bs-theme`. Turns out this is a backwards compatible change that opens the door to the future addition of another argument. * chore: explicitly cast to character Co-authored-by: Carson Sievert <cpsievert1@gmail.com> * feat: introduce `type` and `attrs` (via `...`) in `navbar_options()` * refactor: Move `navbar_options()` to its own R file * tests: Update `navbar_options()` tests * fixup: small details in `navbar_options()` * tests: Add another inverse -> type test * refactor: Use faster color contrast getter * feat: Apply `...` from `navbar_options()` as attributes on the navbar container Fixes #940 * feat: Warn if users try `navbar_options(inverse=)` instead of using `type` * chore: Add note about what to do if precompiled theme output fails * fix(navbar_options): Require all named arguments in `...` and use `attribs` for consistency * docs(navbar_options): Add note about `inverse` -> `type` to docs * chore: revert moving `navbar_options()` for better diff * chore: Apply suggestions from code review * `devtools::document()` (GitHub Actions) * chore: code style * refactor: Use `navbar_options` inside `navs_bar_()` Allows us to avoid needing to do surgery on the navbar later * tests(navs_bar_): Add snapshots to track navbar markup * rename: `theme` instead of `type` * `devtools::document()` (GitHub Actions) * fix: update `type` -> `theme` * feat: Improve navbar compatiblity for Bootstrap 5 and light/dark modes (#1145) * fix(navbar): Don't flip navbar bg in dark mode This rule only changes the background color of the navbar, which is not enough to fully restyle the navbar in dark mode. It might work in the default cases, but it certainly does not work if users have set $navbar-dark-bg or $navbar-light-bg to appropriate values. Note that Bootstrap expects the navbar colors to be static (i.e. the same in light and dark mode), so if we want to support different navbar colors we'll need to implement something better. * fix(bootstrap): Navbar toggler icon should follow navbar color mode Bootstrap has the navbar toggler icon follow the global color mode, which makes the icon invisible when placed on a light background in dark mode. Outside of this rule, Bootstrap expects the navbar color to be consistent in light and dark mode. * fix(bootstrap): Keep global color mode follow * fix: Fix `_navbar.scss` patch * feat: Only use `.navbar-default` and `.navbar-inverse` classes in BS <5 * fix(navbar_compat): navbar light background * feat: account for navbar in light mode region * feat: use `--bslib-navbar-{light,dark}-bg` variables * feat(shiny-preset): Use border-bottom for default navbar not in a dashboard page * refactor(shiny-preset): Use Sass vars to set navbar light/dark bg * fix(bs5): Selector on light-region navbar-toggler-icon * fix(navbar): Dark/light adjustments need to have the lowest specificity possible Otherwise they get in the way of local classes, e.g. `bg-blue` or `bg-primary`, which are the primary mechanism for setting navbar color in BS5 https://getbootstrap.com/docs/5.3/components/navbar/#color-schemes * fix(navbar): Use `:where()` to reduce navbar dark/light styles to minimal specificity * fix(navbar): Use `:where()` when setting navbar bg to ensure that utility classes will win * feat!(navbars): Don't use BS3 compat for navbars in BS5+ * chore: whitespace fixes * Resave distributed files (GitHub Action) * Resave distributed files (GitHub Action) * docs: Add NEWS item --------- Co-authored-by: Carson Sievert <cpsievert1@gmail.com> Co-authored-by: gadenbuie <gadenbuie@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR is a
secondthird take at #1135, and now the second part of a two-part PR. #1146 covers the markup and this PR covers the Sass changes.There are a few things happening in the two PRs. The primary goal is to bring bslib in line with Bootstrap 5's expectations around how the navbar background color is controlled, while adding appropriate controls for the following scenarios:
$navbar-bg.$navbar-light-bgand$navbar-dark-bg.navbar_options(bg = "...").navbar_options(class = "bg-primary", type = "dark").In general, for most BS 5+ users, I'd recommend using the last two options.
1. Navbar background follows light/dark color mode
2. Globally fix navbar colors, regardless of color mode
3. Individually choose navbar colors in light/dark mode
4. Locally set navbar colors using
navbar_options(bg=)5. Locally set navbar colors using BS 5's recommended approach
Notes
Following
bs-themecolor mode by defaultBootstrap 5 primarily wants to have stable navbar colors, but we both -- bslib and pkgdown -- independently developed an approach where the navbar changes colors in light and dark mode. I've patched the BS 5 navbars Sass files to accommodate this.
The biggest source of added complexity in this change is that both
[data-bs-theme="dark"] .navbarand[data-bs-theme="light"] .navbar[data-bs-theme="dark"]should result in a dark navbar style, but[data-bs-theme="light"] .navbar.bg-primary[data-bs-theme="dark"]should be a dark navbar (i.e. light text) with a primary-colored background.These three scenarios required careful construction of selectors, rule ordering, and selector specificity management. In other words, it's why we use
:where()and separately define[data-bs-theme="light"] .navbarand.navbar[data-bs-theme="light"].Localized >>> global changes
Setting
$navbar-bgglobally changes both light and dark themes, and for all navbars. It's convenient but introduces a downside that thedata-bs-themeattribute on the.navbarloses meaning.For the same reason, users will want to use light colors (that result in black contrast text) for
$navbar-light-bgand dark colors (that result in white contrast text) for$navbar-dark-bg.New CSS variables, old classes
The old
.navbar-defaultand.navbar-inverseclasse are maintained for BS 4 compatibility with BS 3 styles. They are now empty classes for BS 5, but still included in the markup in case there are users who rely on them. (They should probably transition away from that, though.)We also still use
--bslib-navbar-default-bgand--bslib-navbar-inverse-bgCSS variables, but I've added--bslib-navbar-light-bgand--bslib-navbar-dark-bgin front of both variables so that they can be set client-side without Sass.