[CLOV-1412][bpk-stylesheets] Ship token CSS variables in base.css#4492
Open
GC Zhu (gc-skyscanner) wants to merge 11 commits into
Open
[CLOV-1412][bpk-stylesheets] Ship token CSS variables in base.css#4492GC Zhu (gc-skyscanner) wants to merge 11 commits into
GC Zhu (gc-skyscanner) wants to merge 11 commits into
Conversation
Import the three token-sync CSS files (primitives, light theme, dark theme) into the base stylesheet so all consumers of @Skyscanner/backpack-web/bpk-stylesheets get the design-token CSS custom properties at runtime. - index.scss: @use the three generated files; Sass compressed mode drops their license headers, so base.css keeps a single header - README: document dark mode (set data-theme="dark" on <html>) and flag the variables as internal-use-only; fix two stale base.js references in the Contributing section - base.css: regenerated; dark theme cascades via :root[data-theme=dark] Note: this is the first @use 'foo.css' (Sass importing plain CSS) in the repo. Existing @use callsites all reference SCSS. Supported since Dart Sass 1.23; repo is on 1.90. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR updates bpk-stylesheets so the generated design-token CSS custom properties (primitives + light/dark themes) are included in the shipped base.css, and documents how to enable dark mode via data-theme="dark".
Changes:
- Import token-sync generated CSS (primitives/light/dark) into
packages/bpk-stylesheets/index.scssvia@use. - Regenerate
packages/bpk-stylesheets/base.cssso it contains:rootvariables and dark overrides under:root[data-theme="dark"]. - Update
packages/bpk-stylesheets/README.mdwith token/dark-mode guidance and fix stalebase.jsreferences.
Reviewed changes
Copilot reviewed 2 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| packages/bpk-stylesheets/index.scss | Adds @use imports for token-sync generated CSS so variables are included in the stylesheet build. |
| packages/bpk-stylesheets/base.css | Regenerated output including token CSS variables and dark-theme overrides. |
| packages/bpk-stylesheets/README.md | Documents token variables and dark mode usage; updates contributing instructions to reference base.css. |
Files not reviewed (1)
- packages/bpk-stylesheets/base.css: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Visit https://backpack.github.io/storybook-prs/4492 to see this build running in a browser. |
Address review feedback on #4492: 1. Sass paths must resolve from the published package. Previously index.scss did `@use '../../token-sync/css/*.css'`, which works in-repo but breaks for consumers compiling index.scss from node_modules — the published tarball does not include token-sync/. Mirror the normalize.scss precedent: copy the three CSS files into packages/bpk-stylesheets/ at build time (gitignored via the existing *.css rule), and switch index.scss to local paths. 2. README link to ../../token-sync/ would 404 on npm. Replace with absolute GitHub URL. base.css output is byte-identical to before. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
|
Visit https://backpack.github.io/storybook-prs/4492 to see this build running in a browser. |
Make the new build:copy-token-css npm script consistent with its sibling build:copy-normal_css. - scripts/scss/copy-token-css.sh: set exec bit (100755) to match copy-normal_css.sh - package.json: drop the `bash` prefix and invoke directly as ./scripts/scss/copy-token-css.sh Both invocations work, but the file-mode/invocation mismatch was a latent footgun: a future contributor aligning package.json to the sibling pattern would have hit "permission denied" on a file committed without the exec bit. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Note in the Contributing section that index.scss depends on gitignored copy steps (normalize + token CSS), and that running build:stylesheets or build:sass standalone requires running the two copy scripts first. npm run build still handles this automatically via run-s build:*. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…bles Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
|
Visit https://backpack.github.io/storybook-prs/4492 to see this build running in a browser. |
1 similar comment
|
Visit https://backpack.github.io/storybook-prs/4492 to see this build running in a browser. |
GC Zhu (gc-skyscanner)
force-pushed
the
gc-skyscanner/clov-1412
branch
from
12ebd9b to
c91f063
Compare
|
Visit https://backpack.github.io/storybook-prs/4492 to see this build running in a browser. |
Previously index.scss reached into vendored token CSS via @use, which forced build:sass (compiles all SCSS via glob) to depend on copy:token-css and made it tricky to wire copy into build:stylesheets without duplicating the work during npm run build. Move the three token CSS imports out of index.scss and into index.js (after ./index.scss) so they reach base.css through webpack's CSS pipeline instead of sass. To keep the published base.css from ballooning with three duplicated Apache license blocks (one per vendored token CSS file), wire postcss-discard-comments into the .css rule's postcss chain so headers are stripped before WrapperPlugin re-adds the single Backpack license header. Net effect: - build:sass no longer needs copy:token-css. - build:copy-token-css renamed to copy:token-css so it is no longer picked up by run-s build:*; build:stylesheets invokes it directly. - npm run build runs copy:token-css exactly once (via build:stylesheets); npm run build:stylesheets is self-contained on a fresh checkout. base.css size: 21,365 → 22,083 bytes raw (+3.4%), but gzipped/brotli are slightly smaller (-30 / -28 bytes) due to more regular formatting. README updated to reflect the new layout. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
GC Zhu (gc-skyscanner)
force-pushed
the
gc-skyscanner/clov-1412
branch
from
c91f063 to
492fc58
Compare
|
Visit https://backpack.github.io/storybook-prs/4492 to see this build running in a browser. |
1 similar comment
|
Visit https://backpack.github.io/storybook-prs/4492 to see this build running in a browser. |
…line PostCSS plugin
GC Zhu (gc-skyscanner)
force-pushed
the
gc-skyscanner/clov-1412
branch
from
981a1a7 to
ac5162e
Compare
|
Visit https://backpack.github.io/storybook-prs/4492 to see this build running in a browser. |
Cherry-picked from #4441 to enable publishing dev versions of this branch for consumer-side verification. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
GC Zhu (gc-skyscanner)
temporarily deployed
to
Publishing
GitHub Actions
Inactive
|
Visit https://backpack.github.io/storybook-prs/4492 to see this build running in a browser. |
# Conflicts: # package.json
GC Zhu (gc-skyscanner)
requested a deployment
to
Publishing
GitHub Actions
Waiting
GC Zhu (gc-skyscanner)
force-pushed
the
gc-skyscanner/clov-1412
branch
from
b87aa43 to
4c3ee15
Compare
GC Zhu (gc-skyscanner)
requested a deployment
to
Publishing
GitHub Actions
Waiting
GC Zhu (gc-skyscanner)
force-pushed
the
gc-skyscanner/clov-1412
branch
from
4c3ee15 to
23d0f51
Compare
GC Zhu (gc-skyscanner)
requested a deployment
to
Publishing
GitHub Actions
Waiting
GC Zhu (gc-skyscanner)
force-pushed
the
gc-skyscanner/clov-1412
branch
from
23d0f51 to
c302e23
Compare
GC Zhu (gc-skyscanner)
requested a deployment
to
Publishing
GitHub Actions
Waiting
Replaces the inline strip-comments PostCSS plugin with css-minimizer-webpack-plugin (cssnano default preset). cssnano strips comments and minifies the output, shrinking base.css. The Backpack license header added by wrapper-webpack-plugin is now emitted as a `/*!` important comment so cssnano preserves it. base.css 22,083 -> 19,876 bytes raw, gzip 4,023 -> 3,915, brotli 3,235 -> 3,174. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
GC Zhu (gc-skyscanner)
temporarily deployed
to
Publishing
GitHub Actions
Inactive
|
Visit https://backpack.github.io/storybook-prs/4492 to see this build running in a browser. |
| }, | ||
|
|
||
| optimization: { | ||
| minimizer: ['...', new CssMinimizerPlugin()], |
Contributor
Author
There was a problem hiding this comment.
Add css-minimizer-webpack-plugin(cssnano) to minify base.css.
cssnanostrips vendor license comments from imported token-sync CSS and minifies output in one pass'...'inminimizerkeeps webpack's default JS minifier (TerserPlugin)
base.css: 22,083 → 19,876 bytes (-10%)
gzip: 4,023 → 3,915
brotli: 3,235 → 3,174
| const licenseHeader = `/* | ||
| // Use a `/*!` "important" comment so CssMinimizerPlugin (cssnano) preserves | ||
| // it through minification. | ||
| const licenseHeader = `/*! |
Contributor
Author
There was a problem hiding this comment.
Backpack license header switched to /*! so cssnano preserves it
- bpk-stylesheets: trim build pipeline notes to the essentials - token-sync: surface the `build:stylesheets` step after `tokens:sync` so regenerated tokens make it into `base.css` Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
GC Zhu (gc-skyscanner)
requested a deployment
to
Publishing
GitHub Actions
Waiting
|
Visit https://backpack.github.io/storybook-prs/4492 to see this build running in a browser. |
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.
Summary
Imports the three generated token-sync CSS files (primitives, light theme, dark theme) into `bpk-stylesheets/index.scss` so `base.css` ships the design-token CSS custom properties to every consumer of `@skyscanner/backpack-web/bpk-stylesheets`.
Commit 1 — Ship token CSS variables in base.css
Commit 2 — Vendor token CSS at build time
`token-sync/` is not part of the published `@skyscanner/backpack-web` package, so `@use '../../token-sync/css/...` paths would break for consumers compiling `index.scss` from `node_modules`. Fix mirrors the existing `normalize.css` → `normalize.scss` approach:
Note for maintainers
This is the first `@use 'foo.css'` (Sass importing a plain `.css` file) in the repo — every other `@use` callsite references SCSS. Supported since Dart Sass 1.23; the repo is on 1.90. Future token-sync CSS outputs can follow the same vendor-and-import pattern.
Test plan
🤖 Generated with Claude Code