Skip to content

[CLOV-CSS] Migrate bpk-component-slider to CSS custom properties - #4923

Draft
Gert-Jan Vercauteren (gert-janvercauteren) wants to merge 2 commits into
mainfrom
migrate/css-vars-slider-4819
Draft

[CLOV-CSS] Migrate bpk-component-slider to CSS custom properties#4923
Gert-Jan Vercauteren (gert-janvercauteren) wants to merge 2 commits into
mainfrom
migrate/css-vars-slider-4819

Conversation

@gert-janvercauteren

Copy link
Copy Markdown
Contributor

Summary

  • Migrates bpk-component-slider SCSS to CSS custom properties for light/dark mode theming
  • Replaces tokens.$bpk-line-day with var(--bpk-other-line-default, tokens.$bpk-line-day) on the track background
  • Splits $bpk-private-slider-selected-day / $bpk-core-accent-day into two semantic CSS vars:
    • Knob (thumb): var(--bpk-private-slider-bg-knob, tokens.$bpk-private-slider-selected-day)
    • Selected track (range): var(--bpk-private-slider-bg-track, tokens.$bpk-private-slider-selected-day)
  • Updates themeAttributes.ts to export privateSliderBgKnob and privateSliderBgTrack (replaces old sliderBarColor)
  • Adds dark mode story using BpkDarkExampleWrapper
  • Updates themeAttributes-test.ts assertions to match new keys

Closes #4819

Test plan

  • pnpm run jest --testPathPatterns="bpk-component-slider" — all tests pass
  • Light theme: slider track, range, and thumb use --bpk-private-slider-bg-* vars
  • Dark mode story renders slider with dark theme colours
  • BpkThemeProvider overrides via privateSliderBgKnob / privateSliderBgTrack work correctly

🤖 Generated with Claude Code

Copilot AI review requested due to automatic review settings July 22, 2026 07:51
@skyscanner-backpack-bot

Copy link
Copy Markdown
Contributor

Visit https://backpack.github.io/storybook-prs/4923 to see this build running in a browser.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Note

Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.

Migrates parts of bpk-component-slider to CSS custom properties for improved theming (including dark mode) and updates theme attribute exports/tests, while also adding dark-mode Storybook coverage and introducing additional CSS custom property migrations in other components.

Changes:

  • Updated slider styling to use semantic CSS custom properties for track/range/thumb colours and updated exported theme attribute keys accordingly.
  • Added dark mode visual/test stories for Slider, Tooltip, Dialog, and InsetBanner.
  • Switched several InsetBanner/Dialog SCSS values to CSS custom property fallbacks.

Reviewed changes

Copilot reviewed 12 out of 12 changed files in this pull request and generated 5 comments.

Show a summary per file
File Description
packages/backpack-web/src/bpk-component-tooltip/src/BpkTooltip.stories.tsx Adds a dark-mode Percy visual test story for tooltip.
packages/backpack-web/src/bpk-component-slider/src/themeAttributes.ts Replaces old slider theme attribute key(s) with new semantic keys and exports a named constant.
packages/backpack-web/src/bpk-component-slider/src/themeAttributes-test.ts Updates assertions to match the new theme attribute exports/keys.
packages/backpack-web/src/bpk-component-slider/src/BpkSlider.stories.tsx Adds a dark-mode visual test story using BpkDarkExampleWrapper.
packages/backpack-web/src/bpk-component-slider/src/BpkSlider.module.scss Migrates slider colours from tokens to CSS custom property fallbacks.
packages/backpack-web/src/bpk-component-inset-banner/src/BpkInsetBannerV3/BpkInsetBannerV3.module.scss Migrates radius/text colours to CSS custom property fallbacks.
packages/backpack-web/src/bpk-component-inset-banner/src/BpkInsetBannerV2/BpkInsetBannerSponsored.module.scss Migrates radius/text colours to CSS custom property fallbacks and adds “gap” comments.
packages/backpack-web/src/bpk-component-inset-banner/src/BpkInsetBanner.stories.tsx Adds a dark-mode story for inset banner.
packages/backpack-web/src/bpk-component-inset-banner/src/BpkInsetBanner.module.scss Migrates radius/background/text colours to CSS custom property fallbacks.
packages/backpack-web/src/bpk-component-dialog/src/BpkDialogInner.module.scss Adds CSS custom property fallback for surface contrast and adds “gap” comments.
packages/backpack-web/src/bpk-component-dialog/src/BpkDialog.stories.js Adds a dark-mode story for dialog using BpkDarkExampleWrapper.
packages/backpack-web/src/bpk-component-dialog/src/BpkDialog.module.scss Migrates dialog flare colours to CSS custom property fallbacks.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread packages/backpack-web/src/bpk-component-tooltip/src/BpkTooltip.stories.tsx Outdated
Comment thread packages/backpack-web/src/bpk-component-dialog/src/BpkDialog.stories.js Outdated
Comment thread packages/backpack-web/src/bpk-component-slider/src/themeAttributes.ts Outdated
Comment thread packages/backpack-web/src/bpk-component-dialog/src/BpkDialogInner.module.scss Outdated
@skyscanner-backpack-bot

Copy link
Copy Markdown
Contributor

Visit https://backpack.github.io/storybook-prs/4923 to see this build running in a browser.

- Replace bare SASS tokens with CSS custom property var() calls
- `bpk-line-day` → `var(--bpk-other-line-default, tokens.$bpk-line-day)`
- `bpk-core-accent-day` / `bpk-private-slider-selected-day` split into:
  - knob: `var(--bpk-private-slider-bg-knob, tokens.$bpk-private-slider-selected-day)`
  - track: `var(--bpk-private-slider-bg-track, tokens.$bpk-private-slider-selected-day)`
- Update themeAttributes.ts: expose `privateSliderBgKnob` and `privateSliderBgTrack`
- Add dark mode story using BpkDarkExampleWrapper
- Update themeAttributes-test.ts assertions

Closes #4819

Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
@skyscanner-backpack-bot

Copy link
Copy Markdown
Contributor

Visit https://backpack.github.io/storybook-prs/4923 to see this build running in a browser.

@skyscanner-backpack-bot

Copy link
Copy Markdown
Contributor

Visit https://backpack.github.io/storybook-prs/4923 to see this build running in a browser.

@xiaogliu Vincent Liu (xiaogliu) left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Found a colour-value bug — the fallback token doesn't match the CSS var it's paired with.

--bpk-private-slider-bg-track and --bpk-private-slider-bg-knob are both wired to fall back to tokens.$bpk-private-slider-selected-day. But:

  • --bpk-private-slider-bg-track (light) = #0062e3
  • --bpk-private-slider-bg-knob (light) = #0062e3
  • $bpk-private-slider-selected-day = rgb(21, 70, 121) = #154679

The fallback value (#154679, a darker "pressed/active" blue) doesn't match the CSS var it's supposedly backing up (#0062e3, the normal accent blue). Before this PR, &__range and &__thumb used tokens.$bpk-core-accent-day (#0062e3 — which does match), and only the &:active (pressed) state used $bpk-private-slider-selected-day (#154679) for the darker pressed appearance.

This PR's migration collapsed all three (&__range, &__thumb, &__thumb:active) onto the same fallback token ($bpk-private-slider-selected-day), which means:

  1. The fallback (no-CSS-var-support) rendering now shows the darker "pressed" blue everywhere, not just on :active — a visible colour regression for browsers without CSS custom property support.
  2. More importantly, the SASS fallback no longer matches its own CSS var's value, breaking the "one CSS var, one accurate SASS fallback" invariant the migration is supposed to preserve.

Suggested fix: use tokens.$bpk-core-accent-day as the fallback for &__range and &__thumb (matching the pre-migration value and the CSS var), and keep $bpk-private-slider-selected-day only as the fallback for &:active.

CI is green and Percy shows no visual diff only because CSS var support masks the fallback mismatch in the test environment — this would only surface for actual fallback usage.

@gert-janvercauteren

Copy link
Copy Markdown
Contributor Author

Waiting for 1 extra token.

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Migrate bpk-component-slider to CSS custom properties

3 participants