[CLOVER-532][BpkText] Add BpkText color prop with leverage css classname #3900
Conversation
|
Visit https://backpack.github.io/storybook-prs/3900 to see this build running in a browser. |
|
Visit https://backpack.github.io/storybook-prs/3900 to see this build running in a browser. |
|
Visit https://backpack.github.io/storybook-prs/3900 to see this build running in a browser. |
|
Visit https://backpack.github.io/storybook-prs/3900 to see this build running in a browser. |
|
Visit https://backpack.github.io/storybook-prs/3900 to see this build running in a browser. |
|
Visit https://backpack.github.io/storybook-prs/3900 to see this build running in a browser. |
|
Visit https://backpack.github.io/storybook-prs/3900 to see this build running in a browser. |
|
Visit https://backpack.github.io/storybook-prs/3900 to see this build running in a browser. |
| 'text-primary-dark': tokens.$bpk-text-primary-dark-color, | ||
| 'text-primary-light': tokens.$bpk-text-primary-light-color, | ||
| 'text-secondary-dark': tokens.$bpk-text-secondary-dark-color, | ||
| 'text-secondary-light': tokens.$bpk-text-secondary-light-color, |
There was a problem hiding this comment.
We'll need to check these.
|
Visit https://backpack.github.io/storybook-prs/3900 to see this build running in a browser. |
|
Visit https://backpack.github.io/storybook-prs/3900 to see this build running in a browser. |
|
Visit https://backpack.github.io/storybook-prs/3900 to see this build running in a browser. |
|
Visit https://backpack.github.io/storybook-prs/3900 to see this build running in a browser. |
|
Visit https://backpack.github.io/storybook-prs/3900 to see this build running in a browser. |
There was a problem hiding this comment.
Pull Request Overview
This PR adds a color prop to the BpkText component that constrains color values to predefined design system tokens instead of allowing arbitrary color strings. This change improves VDL consistency by preventing manual style overrides and ensures color values align with the design system.
Key changes:
- Replaces inline style-based color prop with CSS class-based approach using predefined tokens
- Exports TEXT_COLORS constants and TextColor type for consumer use
- Updates tests and documentation to reflect the new color prop implementation
Reviewed Changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| BpkText.tsx | Adds TEXT_COLORS constant, updates color prop type, and switches from inline styles to CSS classes |
| BpkText.module.scss | Defines SCSS map for text colors and generates CSS classes for each color token |
| BpkText-test.tsx | Updates tests to validate new color prop behavior with CSS classes instead of inline styles |
| index.ts | Exports TEXT_COLORS constant for external use |
| README.md | Updates documentation to show usage with TEXT_COLORS tokens |
| examples.module.scss | Adds example styles for demonstrating color inheritance |
| examples.js | Updates examples to use new TEXT_COLORS API |
Comments suppressed due to low confidence (2)
packages/bpk-component-text/src/BpkText-test.tsx:115
- The test for invalid color value should verify that no color class is applied when an invalid value is passed, but it only checks the base classes. Consider adding an assertion that no color-related CSS class is present.
const { getByText } = render(<BpkText color="invalid">{text}</BpkText>);
packages/bpk-component-text/src/BpkText-test.tsx:140
- Using
window.getComputedStylein tests can be unreliable in test environments. Consider using a more explicit assertion or mocking the computed styles to ensure consistent test behavior across different environments.
expect(window.getComputedStyle(getByText(text)).color).toBe(
| @each $color-name, $color-value in $text-colors { | ||
| &--#{$color-name} { | ||
| color: $color-value; | ||
| } |
There was a problem hiding this comment.
Use Sass loops - Sass: @each to generate the classnames
| @@ -58,6 +58,21 @@ export const TEXT_STYLES = { | |||
| editorial3: 'editorial-3', | |||
| } as const; | |||
|
|
|||
| export const TEXT_COLORS = { | |||
There was a problem hiding this comment.
The TEXT_COLORS is maintained for color prop tokens list we support with minimum set VDL consistency.
|
Visit https://backpack.github.io/storybook-prs/3900 to see this build running in a browser. |
* main: Bump actions/download-artifact in the artifacts-actions group (#3907) Bump actions/checkout from 4 to 5 (#3913) Fix closeText style of BpkModal in dark mode (#3919) Bump the babel group across 1 directory with 5 updates (#3917) Bump @babel/runtime in the npm_and_yarn group across 1 directory (#3884) Bump actions/cache from 4.2.3 to 4.2.4 (#3905) Bump webpack from 5.99.8 to 5.101.2 (#3916) QUAR-1046 Fix CTA Button Alignment, Logo Sizing, and Logo Vertical Alignment in Inset Banner (#3912) chore: removed duplicate aria-label (#3911) [CAPY-1594][BpkNavigationTabGroup/BpkBubble] Create and integrate 'new' bubble tooltip in navigation tab bar (#3909) [CLOV-381][BpkBreakpoint] update bpk breakpoint readme (#3910) [CYB-3904][BpkGraphicPromo] Render wrapper as semantic anchor tag to improve SEO (#3904) [CLOVER-532][BpkText] Add BpkText color prop with leverage css classname (#3900) fix: A11y and icon issue for inset banner sponsored (#3901) # Conflicts: # packages/bpk-component-text/README.md # packages/bpk-component-text/src/BpkText-test.tsx # packages/bpk-component-text/src/BpkText.tsx
Context
BpkText’s lack of default or token-based color support leads to widespread manual style overrides (average 14.88% per major page), undermining VDL consistency and reducing design system reliability and maintainability across Skyscanner products.
We have added color prop so that consumer can change
BpkTextcolor by config but not override before, and want to constraint forBpkTextcolor prop only allowed BPK text color tokens.Changes
BpkTextcolor prop with leverage css classname, replaces inline style-based color prop with CSS class-based approach using predefined tokensTEXT_COLORSconstants andTextColortype for consumer useRemember to include the following changes:
[Clover-123][BpkButton] Updating the colourREADME.md(If you have created a new component)README.md