feat(design-tokens): amend css variables to canopy design tokens #1659
Conversation
There was a problem hiding this comment.
Pull request overview
This PR migrates the Canopy design system from locally maintained design tokens to the published @legal-and-general/canopy-design-tokens npm package (v1.2.0). The migration involves updating CSS variable naming conventions from prefix-based patterns (e.g., --sm-space-1, --productive-font-family) to suffix-based patterns (e.g., --space-1-sm, --font-family-productive).
Key Changes:
- Added
@legal-and-general/canopy-design-tokenspackage dependency - Updated CSS variable naming conventions throughout the codebase to match the new tokens structure
- Refactored typography and spacing utilities to handle breakpoint-specific design token values
Reviewed changes
Copilot reviewed 14 out of 15 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| package.json, package-lock.json | Added @legal-and-general/canopy-design-tokens v1.2.0 as a dependency |
| scripts/generate-css-variables.js | Updated tokens directory path to reference the npm package instead of local files |
| projects/canopy/src/styles/styles.scss | Added imports for all design token CSS files from the npm package |
| projects/canopy/src/styles/variables.scss | Removed imports for local colours and token files that are now provided by the package |
| projects/canopy/src/styles/variables/_colours.scss | Deleted entire file as colours are now provided by the design tokens package |
| projects/canopy/src/styles/variables/_typography.scss | Refactored to use breakpoint-specific font size/line height variables with responsive mapping |
| projects/canopy/src/styles/spacing.scss | Updated variable references from --sm-space-X to --space-X-sm pattern |
| projects/canopy/src/styles/tokens/variables.css | Deleted as tokens are now sourced from the npm package |
| projects/canopy/src/styles/tokens/layout.css | Deleted as layout tokens are now sourced from the npm package |
| projects/canopy/src/lib/docs/foundation/typography/typography-utils.ts | Refactored to handle breakpoint-specific values and added new helper functions for MDX tables |
| projects/canopy/src/lib/docs/foundation/typography/typography.stories.ts | Changed type definitions to require both sm and lg values (previously optional) |
| projects/canopy/src/lib/docs/foundation/typography/typography.mdx | Updated to use new breakpoint-aware utility functions for displaying token values |
| projects/canopy/src/lib/docs/foundation/spacing-utils.ts | Updated variable filtering logic to match new naming pattern with suffix-based breakpoints |
| projects/canopy/src/lib/docs/foundation/colours/colours.stories.ts | Updated import path to reference tokens from the new package location |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
projects/canopy/src/lib/docs/foundation/typography/typography-utils.ts
Outdated
Show resolved
Hide resolved
projects/canopy/src/lib/docs/foundation/typography/typography-utils.ts
Outdated
Show resolved
Hide resolved
projects/canopy/src/lib/docs/foundation/typography/typography.stories.ts
Outdated
Show resolved
Hide resolved
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 19 out of 20 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
@IdaHW font-size-9 seems much bigger than in master now. Left is new, right is master: 
Do you know whys is that? |
| @use 'typography-elements'; | ||
|
|
||
| // Import design tokens | ||
| @import '@legal-and-general/canopy-design-tokens/build/css/variables.css'; |
There was a problem hiding this comment.
I think I'd probably amend the package so that it outputs without the build folder - seems unnecessary 🤔 What do you think?
There was a problem hiding this comment.
That was just so that the css and js tokens can be in sub folders in the /build directory, so /build/css and /build/js and not in the same folder. Might be better to keep them separate, but happy to change if you think it'll work better
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 19 out of 20 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| --text-base-size: 16px; /* stylelint-disable-line unit-allowed-list */ | ||
| --text-base-line-height: 1.38; | ||
| --text-base-size: var(--font-size-1); // 16px | ||
| --text-base-line-height: var(--line-height-1); //1.38 |
There was a problem hiding this comment.
The comment should be updated to reflect the actual line height value from the design tokens. The value 1.38 may not be accurate for --line-height-1-sm from the new design tokens package.
| --text-base-line-height: var(--line-height-1); //1.38 | |
| --text-base-line-height: var(--line-height-1); // base line height from design tokens |
|
|
||
| --text-base-size: 16px; /* stylelint-disable-line unit-allowed-list */ | ||
| --text-base-line-height: 1.38; | ||
| --text-base-size: var(--font-size-1); // 16px |
There was a problem hiding this comment.
The comment should be updated to match the actual variable name being referenced. The variable --font-size-1-sm has a value that likely corresponds to 16px, but the variable name is --font-size-1, not --font-size-1-sm.
| --text-base-size: var(--font-size-1); // 16px | |
| --text-base-size: var(--font-size-1); // base font size token |
2 participants
Description
Add the Canopy Design Tokens npm package and update all the foundations css values to use the new tokens
Update the scripts to use the design tokens
Checklist: