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

Auto color and bg-color with data-bs-theme #2313

Merged
merged 12 commits into from
Nov 29, 2023

Conversation

julien-deramond
Copy link
Contributor

@julien-deramond julien-deramond commented Oct 13, 2023

Description

This PR tries a new approach with an automatic set of color: and background-color: linked to [data-bs-theme] attribute.

So when your create a <div data-bs-theme="dark"> the content will be automatically rendered with a dark background and a white text in Boosted.

Based on this modification, this PR also sets the Sass and/or CSS variables of components handling color and/or bg color to null by default, or inherit, and also sometimes sets the background to transparent.

More info

We decided to make big components (components with multiple layers) to inherit the color because it seemed more logical. Some components escape this logic such as spinners. It's because it's not really the color that is redefined but the use of currentColor.
All this is due to a technical issue we have when the theme is set directly on the component.

If the component sets color at the top level, it overrides the color coming from the data-bs-theme so if it's set to null or inherit, it will inherit the color above and avoid data-bs-theme color. On multiple-layer components it's easier to handle since we only need to avoid declaring color at the top level of this component.

Weird behavior for titles. For example h1 -> Redefines color but .h1 -> inherits when the data theme is placed on them directly.

For basic elements, form elements and input labels, we decided to keep the behavior as-is. Most likely the theme won't be applied to "basic" elements themselves. And the users could override this behavior if they are using this edge case.

Note

  • We got rid of text-body classes in dark mode page for basic elements introduced by Dark mode: basic elements #2355 to really test the impact of this PR in this context.
We chose not to modify color-mode() mixin, here's why...

Not covered in this PR

Based on this modification, it would be possible to do the following in components:

@include color-mode(light, true) {
  --#{$prefix}accordion-btn-icon: #{escape-svg($accordion-button-icon)};
  --#{$prefix}accordion-btn-active-icon: #{escape-svg($accordion-button-active-icon)};
}

@if $enable-dark-mode {
  @include color-mode(dark, true) {
    --#{$prefix}accordion-btn-icon: #{escape-svg($accordion-button-icon-dark)};
    --#{$prefix}accordion-btn-active-icon: #{escape-svg($accordion-button-active-icon-dark)};
  }
}

instead of the current in Bootstrap which doesn't work when a light accordion is declared within a dark container:

@if $enable-dark-mode {
  @include color-mode(dark) {
    .accordion-button::after {
      --#{$prefix}accordion-btn-icon: #{escape-svg($accordion-button-icon-dark)};
      --#{$prefix}accordion-btn-active-icon: #{escape-svg($accordion-button-active-icon-dark)};
    }
  }
}

@julien-deramond julien-deramond added the color mode Temporary label to highlight color mode issues label Oct 13, 2023
@netlify
Copy link

netlify bot commented Oct 13, 2023

Deploy Preview for boosted ready!

Name Link
🔨 Latest commit 98f8b23
🔍 Latest deploy log https://app.netlify.com/sites/boosted/deploys/65670da2cea27000082d6111
😎 Deploy Preview https://deploy-preview-2313--boosted.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

louismaximepiton

This comment was marked as resolved.

@louismaximepiton louismaximepiton force-pushed the main-jd-dark-mode-new-data-bs-theme branch from 21441eb to f641769 Compare October 17, 2023 14:14
@julien-deramond julien-deramond force-pushed the main-jd-dark-mode branch 3 times, most recently from df0ac35 to 22bf6d9 Compare October 31, 2023 05:19
@julien-deramond julien-deramond force-pushed the main-jd-dark-mode branch 2 times, most recently from 03af5c3 to f49a3ee Compare November 3, 2023 11:30
@julien-deramond julien-deramond force-pushed the main-jd-dark-mode-new-data-bs-theme branch from f641769 to 49312e0 Compare November 3, 2023 11:52
@julien-deramond julien-deramond force-pushed the main-jd-dark-mode branch 2 times, most recently from 35328ca to f0b8a1e Compare November 6, 2023 07:18
@julien-deramond julien-deramond force-pushed the main-jd-dark-mode-new-data-bs-theme branch 2 times, most recently from 5cbf7e9 to cf73de1 Compare November 7, 2023 07:51
@julien-deramond julien-deramond force-pushed the main-jd-dark-mode-new-data-bs-theme branch 2 times, most recently from ad18dca to 2ef1cb4 Compare November 8, 2023 08:27
@julien-deramond julien-deramond force-pushed the main-jd-dark-mode-new-data-bs-theme branch from 2ef1cb4 to b23e055 Compare November 8, 2023 13:56
@julien-deramond julien-deramond force-pushed the main-jd-dark-mode-new-data-bs-theme branch from b23e055 to 9fd4a45 Compare November 10, 2023 05:45
@julien-deramond julien-deramond force-pushed the main-jd-dark-mode-new-data-bs-theme branch 2 times, most recently from c0d10b7 to ae821eb Compare November 13, 2023 07:43
scss/_variables.scss Outdated Show resolved Hide resolved
@julien-deramond

This comment was marked as resolved.

@julien-deramond

This comment was marked as resolved.

@julien-deramond julien-deramond force-pushed the main-jd-dark-mode-new-data-bs-theme branch from 0b149a7 to 06c0124 Compare November 29, 2023 10:06
@julien-deramond julien-deramond merged commit b0b2994 into main-jd-dark-mode Nov 29, 2023
11 of 12 checks passed
@julien-deramond julien-deramond deleted the main-jd-dark-mode-new-data-bs-theme branch November 29, 2023 10:09
julien-deramond added a commit that referenced this pull request Nov 30, 2023
Co-authored-by: louismaxime.piton <louismaxime.piton@orange.com>
julien-deramond added a commit that referenced this pull request Dec 1, 2023
Co-authored-by: louismaxime.piton <louismaxime.piton@orange.com>
julien-deramond added a commit that referenced this pull request Dec 4, 2023
Co-authored-by: louismaxime.piton <louismaxime.piton@orange.com>
julien-deramond added a commit that referenced this pull request Dec 8, 2023
Co-authored-by: louismaxime.piton <louismaxime.piton@orange.com>
julien-deramond added a commit that referenced this pull request Dec 15, 2023
Co-authored-by: louismaxime.piton <louismaxime.piton@orange.com>
julien-deramond added a commit that referenced this pull request Dec 18, 2023
Co-authored-by: louismaxime.piton <louismaxime.piton@orange.com>
julien-deramond added a commit that referenced this pull request Dec 21, 2023
Co-authored-by: louismaxime.piton <louismaxime.piton@orange.com>
julien-deramond added a commit that referenced this pull request Dec 26, 2023
Co-authored-by: louismaxime.piton <louismaxime.piton@orange.com>
julien-deramond added a commit that referenced this pull request Dec 27, 2023
Co-authored-by: louismaxime.piton <louismaxime.piton@orange.com>
julien-deramond added a commit that referenced this pull request Jan 2, 2024
Co-authored-by: louismaxime.piton <louismaxime.piton@orange.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
color mode Temporary label to highlight color mode issues css v5
Projects
Development

Successfully merging this pull request may close these issues.

2 participants