bootstrap icon indicating copy to clipboard operation
bootstrap copied to clipboard

Fix light mode carousel in dark mode

Open julien-deramond opened this issue 1 year ago • 1 comments

[!CAUTION] Before merging, carousel.md modifications must be reverted

Description

This work has been done in collaboration with @louismaximepiton, please don't remove the "Co-authored-by" from the commit message if merged

This PR tackles specifically #40493. This work is based on https://github.com/twbs/bootstrap/pull/39295 spirit. The idea is to rely on custom properties so that there's never a specificity issue when nesting color modes (light mode in dark mode, and dark mode in light mode).

In order for it to be non-breaking, _variables-dark.scss uses the newly deprecated $carousel-dark-* Sass variables.

Please note that in the future, we'll be able to replace :root, [data-bs-theme="light"] { with @include color-mode(light, true) as suggested in https://github.com/twbs/bootstrap/pull/39295.

This part generates the following CSS:

.carousel-dark {
  --bs-carousel-indicator-active-bg: #000;
  --bs-carousel-caption-color: #000;
  --bs-carousel-control-icon-filter: invert(1) grayscale(100);
}

:root,
[data-bs-theme=light] {
  --bs-carousel-indicator-active-bg: #fff;
  --bs-carousel-caption-color: #fff;
  --bs-carousel-control-icon-filter: ;
}

[data-bs-theme=dark] {
  --bs-carousel-indicator-active-bg: #000;
  --bs-carousel-caption-color: #000;
  --bs-carousel-control-icon-filter: invert(1) grayscale(100);
}

After this PR

After this PR, we might do exactly the same thing for the problematic rendering of the accordions, close buttons, etc. and use the same technique.

@mdo and @twbs/css-review what do you think about this PR? Do you agree that it can be used, it doesn't bring regressions, and can be replicated for other issues (accordions, etc.)?

Type of changes

  • [x] Bug fix (non-breaking change which fixes an issue)

Checklist

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

Live previews

  • https://deploy-preview-40695--twbs-bootstrap.netlify.app/docs/5.3/components/carousel/

Related issues

Closes #40493

julien-deramond avatar Aug 06 '24 08:08 julien-deramond

You can ignore this message.. It started when I started it using webui-user.bat inside the Forge folder.

TomiTom1234 avatar Aug 13 '24 13:08 TomiTom1234