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

Most components don't support theme/dark mode theme color changes #37976

Open
3 tasks done
Tracked by #37549 ...
yugabe opened this issue Jan 30, 2023 · 7 comments
Open
3 tasks done
Tracked by #37549 ...

Most components don't support theme/dark mode theme color changes #37976

yugabe opened this issue Jan 30, 2023 · 7 comments

Comments

@yugabe
Copy link

yugabe commented Jan 30, 2023

Prerequisites

Describe the issue

Even though the current release is only in alpha, the dark mode/theming support is quite suboptimal and it hurts much more than how much it solves.

Defining default theme colors by default should flow through to dark mode.

Components don't respect theme colors correctly when switching to dark mode. Cards, for example, don't use the theme's CSS variables, they use the color's SCSS variables directly, thus the colors defined in _variables-dark don't even matter. This prevents users from defining a dark mode alternative theme, and further, it prevents users from defining additional themes altogether.

The above, for example, can be attributed to the following compiled CSS:

.text-bg-primary {
    color: #fff!important;
    background-color: RGBA(13,110,253,var(--bs-bg-opacity,1))!important;
}

This comes from _color-bg.scss. These rules should always refer to CSS variables instead of the raw SCSS values so they can be overridden at runtime.

The problem is non-fully systematic and inconsistent. bg-#{$color} and text-#{$color} work correctly. text-bg-#{$color} works incorrectly. link-#{$color} is correct, although link-#{$color}:hover and link-#{$color}:active are incorrect.

Reduced test cases

The issue is visible on the Bootstrap docs page.

What operating system(s) are you seeing the problem on?

Windows

What browser(s) are you seeing the problem on?

Microsoft Edge

What version of Bootstrap are you using?

5.3.0-alpha1

@yugabe
Copy link
Author

yugabe commented Jan 31, 2023

I found the following problems that don't respect the current theme:

  1. Emphasis, subtle and border-subtle are derived from the theme colors themselves. Defining them as SCSS variables only causes problems downstream. Should be calculated and set in CSS vars only.
  2. "Light" and "Dark" don't really make sense in light, dark or whatever themes at all, especially seeing how they can be considered their own themes. I suggest these to be removed altogether from the theme colors.
  3. "Primary" is used pretty consistently as an accent and an active color. Secondary, on the other hand, is used almost interchangeably with shades of gray. "Secondary", thus, shouldn't be available to configure either.
  4. Wherever $primary is referenced for example in CSS rules, it won't respect the current theme. Some very serious culprits here are $component-active-bg: $primary !default; and $focus-ring-color: rgba($primary, $focus-ring-opacity) !default;. These should refer to var(--#{prefix}primary) instead, but it'll cause some additional problems, such as $input-focus-border-color: tint-color($component-active-bg, 50%) !default; - this expects a value at build time to function correctly.

@michaellenaghan
Copy link

Different, but related I think: setting something like $headings-color doesn't work as expected.

Imagine setting $body-color to, say, $gray-700, and $headings-color to $gray-900. You end up with the wrong heading color in, for example, active list group items:

Screenshot 2023-02-17 at 3 02 19 PM

Card titles seems to escape the problem, but only because a) .card-title has higher specificity than h1, etc., and b) the .card-title color is "wrong" — i.e., the same as $body-color rather than darker:

Screenshot 2023-02-17 at 3 06 26 PM

Setting $card-title-color to $headings-color fixes that, but breaks other things:

Screenshot 2023-02-17 at 3 09 29 PM

If I'm doing things the wrong way, obviously I don't know what the right way is. :-)

This isn't a theming issue per se; all of these examples fall into the "light" theme. But. But it seems like some concept is missing. I can't define one universal headings color when that color will be used on various backgrounds?

It seems like anywhere you can define a color — at least, a text color — you need to be able to define two: one for use on "light" backgrounds, and one for use on "dark" backgrounds. The contrast ratio helper would then choose which one to use. Or something like that.

@sanjaygupta972004

This comment was marked as resolved.

@julien-deramond

This comment was marked as resolved.

@sanjaygupta972004

This comment was marked as resolved.

@pau1phi11ips
Copy link

  1. "Light" and "Dark" don't really make sense in light, dark or whatever themes at all, especially seeing how they can be considered their own themes. I suggest these to be removed altogether from the theme colors.

I think I'd prefer them to just swap over. If you design/layout in light mode and use .bg-dark. It'll switch to the .bg-light color in dark mode.

@Banner-Keith
Copy link

  1. "Light" and "Dark" don't really make sense in light, dark or whatever themes at all, especially seeing how they can be considered their own themes. I suggest these to be removed altogether from the theme colors.

I think I'd prefer them to just swap over. If you design/layout in light mode and use .bg-dark. It'll switch to the .bg-light color in dark mode.

Dark and light should have the names changed but still exist. Generally speaking light is a slightly different color from the background color and dark is higher contrast from the background color. I'd really like to see them keep some color like those two but name them something more appropriate.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants