feat: Add Sana styles to Canvas Switch, Radio & Checkbox#4017
feat: Add Sana styles to Canvas Switch, Radio & Checkbox#4017RayRedGoose wants to merge 11 commits into
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughRadio, Switch, and Checkbox stencils update token sources, sizes, colors, and selected state styles. Related visual testing stories add ChangesRadio, Switch, and Checkbox styling
Story updates
Upgrade guide documentation
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~28 minutes Possibly related issues
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Pull request overview
Adds Sana-aligned styling updates for the Switch, Radio, and Checkbox components to reflect updated token usage and visual specs (Issue #3984).
Changes:
- Updated sizing tokens across Checkbox/Radio/Switch (moving several values from
base.legacy.*tosystem.legacy.size.*) and adjusted spacing/offsets accordingly. - Updated accent colors (notably moving checked states toward
brand.accent.positive) and refined hover/focus/error ring visuals. - Updated Checkbox label rendering/styling (LabelText → Subtext) and introduced part-based styling in the container stencil.
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| modules/react/checkbox/lib/CheckboxRipple.tsx | Updates ripple sizing to new system.legacy.size.* tokens. |
| modules/react/checkbox/lib/CheckboxInput.tsx | Adjusts checkbox dimensions, hover ripple size, checked colors, focus/error ring visuals. |
| modules/react/checkbox/lib/CheckboxContainer.tsx | Updates label component/styling and adds stencil parts/modifiers for Sana spacing/disabled styling. |
| modules/react/checkbox/lib/CheckboxCheck.tsx | Tunes checkmark sizing/alignment and inverse variant colors. |
| modules/react/checkbox/lib/CheckBackground.tsx | Updates checkbox background shape/tokens and error styling variables. |
| modules/preview-react/switch/lib/SwitchInput.tsx | Updates input height token to align with new spec sizing. |
| modules/preview-react/switch/lib/SwitchIcon.tsx | Tweaks icon positioning to match updated switch geometry. |
| modules/preview-react/switch/lib/SwitchContainer.tsx | Updates container height token for Sana sizing. |
| modules/preview-react/switch/lib/SwitchCircle.tsx | Updates thumb sizing and checked translation distances. |
| modules/preview-react/switch/lib/SwitchBackground.tsx | Updates background height, padding, and muted accent background token. |
| modules/preview-react/radio/lib/StyledRadioButton.tsx | Updates radio sizing tokens, checked colors, and hover ripple sizing. |
| modules/preview-react/radio/lib/RadioLabel.tsx | Updates label min-height token. |
| modules/preview-react/radio/lib/RadioGroup.tsx | Adjusts group border radius/padding and simplifies error backgrounds. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Workday/canvas-kit
|
||||||||||||||||||||||||||||||||||||||||
| Project |
Workday/canvas-kit
|
| Branch Review |
issue3984-checkbox-radio
|
| Run status |
|
| Run duration | 02m 19s |
| Commit |
|
| Committer | Raisa Primerova |
| View all properties for this run ↗︎ | |
| Test results | |
|---|---|
|
|
0
|
|
|
0
|
|
|
17
|
|
|
0
|
|
|
809
|
| View all changes introduced in this branch ↗︎ | |
UI Coverage
19.59%
|
|
|---|---|
|
|
1533
|
|
|
371
|
Accessibility
99.37%
|
|
|---|---|
|
|
5 critical
5 serious
0 moderate
2 minor
|
|
|
68
|
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@modules/react/checkbox/lib/CheckboxContainer.tsx`:
- Around line 37-48: The disabled modifier in the modifiers object only
overrides the cursor for the container, but the labelPart still has cursor set
to 'pointer' from its base styles, causing the pointer cursor to remain visible
when the checkbox is disabled. Add a disabled true override within the modifiers
section for the labelPart that sets its cursor to 'default' to match the
disabled state behavior of the container.
In `@modules/react/checkbox/lib/CheckboxInput.tsx`:
- Around line 178-179: The error-ring box-shadow at lines 178-179 uses the same
px2rem(1) spread radius for both the errorRingColorOuter and errorRingColorInner
layers, causing the inner ring to paint over and hide the outer ring. Update the
spread radius values for each layer to use distinct values that preserve the
two-layer visual treatment. The first layer (errorRingColorOuter) and second
layer (errorRingColorInner) should each have different px2rem values to ensure
both rings are visible in the unchecked error state.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: d2e70c34-8bcc-40ec-b167-64a486cb0022
📒 Files selected for processing (13)
modules/preview-react/radio/lib/RadioGroup.tsxmodules/preview-react/radio/lib/RadioLabel.tsxmodules/preview-react/radio/lib/StyledRadioButton.tsxmodules/preview-react/switch/lib/SwitchBackground.tsxmodules/preview-react/switch/lib/SwitchCircle.tsxmodules/preview-react/switch/lib/SwitchContainer.tsxmodules/preview-react/switch/lib/SwitchIcon.tsxmodules/preview-react/switch/lib/SwitchInput.tsxmodules/react/checkbox/lib/CheckBackground.tsxmodules/react/checkbox/lib/CheckboxCheck.tsxmodules/react/checkbox/lib/CheckboxContainer.tsxmodules/react/checkbox/lib/CheckboxInput.tsxmodules/react/checkbox/lib/CheckboxRipple.tsx
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@modules/docs/mdx/16.0-UPGRADE-GUIDE.mdx`:
- Line 145: Replace the word "team" with "theme" in the phrases that mention
"`sana-canvas` team" in the UPGRADE-GUIDE.mdx file. This typo appears in three
locations where it should read "`sana-canvas` theme" instead. Search for all
instances of "`sana-canvas` team" and change them to "`sana-canvas` theme" to
correct the terminology.
- Around line 97-119: Replace all instances of the attribute name
`data-attribute="sana-canvas"` with `data-theme="sana-canvas"` throughout this
upgrade guide section. The correct attribute name that the runtime wiring uses
is `data-theme`, not `data-attribute`. Update this in all code examples showing
both global application (in the body tag) and local scoped application (in the
div tag) to ensure users enable the Sana Canvas theme correctly.
- Around line 140-141: The migration guide contains incorrect token namespace
paths that are missing the `.legacy` prefix. Update all references to
`system.color.brand.accent.positive` and `system.color.brand.accent.primary` to
use `system.legacy.color.brand.accent.positive` and
`system.legacy.color.brand.accent.primary` respectively. This change is needed
in three locations: the Checkbox section (around lines 140-141), the Radio
section (around lines 154-155), and the Switch section (around lines 168-171).
Add the `.legacy` prefix between `system.color` and `.brand.accent` in each
occurrence to align the documentation with the actual component implementation.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: d8303dc8-fe87-4074-99f4-88a3b1bf9723
📒 Files selected for processing (1)
modules/docs/mdx/16.0-UPGRADE-GUIDE.mdx
There was a problem hiding this comment.
🧹 Nitpick comments (1)
modules/docs/mdx/16.0-UPGRADE-GUIDE.mdx (1)
131-174: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd migration guidance for
inversedeprecation in Radio/Checkbox sections.These component update blocks list visual changes, but they should also call out the
inversevariant deprecation and codemod path so adopters don’t miss required migration work in v16.Suggested doc addition
### Checkbox **PR:** [`#3984`](https://github.com/Workday/canvas-kit/pull/3984) #### Visual Updates @@ - The hover ring is smaller to better align with the smaller size. The hover ring background color uses neutral instead of slate in `sana-canvas` theme. + +#### Deprecations + +- `inverse` is deprecated in v16. Use the standard Sana theming approach with + `data-theme="sana-canvas"` instead. +- 🤖 Codemod support: run the v16 codemod (`@workday/canvas-kit-codemod`) to apply supported + automatic migrations. @@ ### Radio @@ #### Visual Updates @@ - The hover ring is smaller to better align with the smaller size. The hover ring background color uses a neutral color instead of slate in `sana-canvas` theme. + +#### Deprecations + +- `inverse` is deprecated in v16. Use the standard Sana theming approach with + `data-theme="sana-canvas"` instead. +- 🤖 Codemod support: run the v16 codemod (`@workday/canvas-kit-codemod`) to apply supported + automatic migrations.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@modules/docs/mdx/16.0-UPGRADE-GUIDE.mdx` around lines 131 - 174, The Checkbox and Radio component update sections currently list only visual changes but omit important migration guidance for the deprecated `inverse` variant. Add a subsection under both the Checkbox and Radio sections that explicitly documents the deprecation of the `inverse` variant and provides the codemod path or migration instructions so adopters are aware of the required changes needed for v16 upgrade.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@modules/docs/mdx/16.0-UPGRADE-GUIDE.mdx`:
- Around line 131-174: The Checkbox and Radio component update sections
currently list only visual changes but omit important migration guidance for the
deprecated `inverse` variant. Add a subsection under both the Checkbox and Radio
sections that explicitly documents the deprecation of the `inverse` variant and
provides the codemod path or migration instructions so adopters are aware of the
required changes needed for v16 upgrade.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: f27f2c83-b298-44b3-aef4-df9adea8e543
📒 Files selected for processing (2)
modules/docs/mdx/16.0-UPGRADE-GUIDE.mdxmodules/react/checkbox/lib/CheckboxContainer.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- modules/react/checkbox/lib/CheckboxContainer.tsx
sheelah
left a comment
There was a problem hiding this comment.
LG - left just a few minor comments.
| </div> | ||
| ``` | ||
|
|
||
| Please ensure you are using Canvas Tokens v4.4.0 or later. The Sana theme will not be available with |
There was a problem hiding this comment.
Nit: I think we should bold this text since this version note is important.
| cursor: 'pointer', | ||
| height: base.legacy.size225, | ||
| width: base.legacy.size225, | ||
| height: system.legacy.size.xxxs, |
There was a problem hiding this comment.
Why are we using old system tokens here?
Medium: StyledRadioButton target is now below 24x24. The actual native input is reduced to system.legacy.size.xxxs for both width and height, while the 24px wrapper is not itself the interactive target. For the standalone StyledRadioButton, this likely regresses WCAG 2.2 target size expectations. Keep the visual radio at 20px if needed, but leave the invisible input target at least 24x24 and center the visual circle.
| @@ -78,8 +84,8 @@ | |||
|
|
|||
| [`&:where(:checked, :indeterminate) ~ [data-part="${checkboxBackgroundStencil.parts.background['data-part']}"]`]: | |||
There was a problem hiding this comment.
Medium: checked-state contrast now depends on success.main. Checkbox, radio, and switch checked states now use system.legacy.color.brand.accent.positive, while the icon/thumb foreground remains inverse/white. That is fine for dark success colors, but consumer themes may have light success colors. The visual test theme changing success.main to darkolivegreen helps the test pass, but does not protect consumers. Consider using a paired contrast token for the foreground, documenting a minimum contrast requirement for success.main, or adding themed contrast coverage with a light success color
| '&:checked, &.checked': { | ||
| '& ~ div:first-of-type': { | ||
| backgroundColor: system.legacy.color.brand.accent.primary, | ||
| backgroundColor: system.legacy.color.brand.accent.positive, |
There was a problem hiding this comment.
Might have the same potential contrast issue here if the branded positive color token is a light color?
|
|
||
| #### Visual Updates | ||
|
|
||
| - The Switch thumb is now **20px** (was 18px). The icon-thumb container width remains unchanged. |
There was a problem hiding this comment.
Can we double check this is true? The xxxs token resolves to 16px I think.
export const switchCircleStencil = createStencil({
base: {
width: system.legacy.size.xxxs,
height: system.legacy.size.xxxs,
| outline: 'none', | ||
| }, | ||
| // When not checked, the border is within the input | ||
| [`&:where(:focus-visible, .focus) ~ [data-part="${checkboxBackgroundStencil.parts.background['data-part']}"]`]: |
There was a problem hiding this comment.
Medium: Checked/indeterminate Checkbox focus can lose its high-contrast fallback. The PR changes the checked focus border from 2px to 1px. Since the outer focus treatment is boxShadow, Windows High Contrast may omit it, leaving no visible focus difference from the normal checked state.
|
|
||
| '&:focus-visible:checked + .cnvs-radio-check, &:focus-visible:hover:checked + .cnvs-radio-check, &.focus:checked + .cnvs-radio-check, &.focus:hover:checked + .cnvs-radio-check': | ||
| { | ||
| outline: 'transparent', |
There was a problem hiding this comment.
Low/pre-existing but worth fixing while touched: Preview Radio’s focus fallback uses outline: 'transparent' without width/style. That does not create the high-contrast-safe outline pattern you described. The repo’s stronger pattern is 1px/2px solid transparent.
Summary
Fixes: #3984
Added Sana styles to Canvas Switch, Radio & Checkbox
Release Category
Components
Checklist
ready for reviewhas been added to PRFor the Reviewer
Where Should the Reviewer Start?
Areas for Feedback? (optional)
Testing Manually
Screenshots or GIFs (if applicable)
Thank You Gif (optional)
Summary by CodeRabbit
Release Notes
Refactor
Style
Documentation
Tests
data-theme="sana-canvas".