feat: Add Sana Canvas indicator component updates - #4087
Conversation
color-mix doesn't work well with colors with alpha values.
|
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:
📝 WalkthroughWalkthroughThis change updates Canvas Kit v16 indicator styling across Banner, Count Badge, Information Highlight, Pill, Skeleton, and Status Indicator. Information Highlight adds ChangesIndicators v16
Estimated code review effort: 3 (Moderate) | ~25 minutes Possibly related issues
Possibly related PRs
Suggested reviewers: Sequence Diagram(s)sequenceDiagram
participant Consumer
participant InformationHighlight
participant useInformationHighlightModel
participant informationHighlightStencil
Consumer->>InformationHighlight: set variant and ctaPlacement
InformationHighlight->>useInformationHighlightModel: read component state
useInformationHighlightModel->>informationHighlightStencil: pass variant, emphasis, and ctaPlacement
informationHighlightStencil->>InformationHighlight: apply CTA grid layout
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ 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 |
Workday/canvas-kit
 |
||||||||||||||||||||||||||||||||||||||||
| Project |
Workday/canvas-kit
|
| Branch Review |
sana-canvas-indicators
|
| Run status |
|
| Run duration | 02m 38s |
| Commit |
|
| Committer | Sheelah Brennan |
| View all properties for this run ↗︎ | |
| Test results | |
|---|---|
 |
0
|
 |
0
|
 |
17
|
 |
0
|
 |
809
|
| View all changes introduced in this branch ↗︎ | |
UI Coverage
19.54%
|
|
|---|---|
|
|
1534
|
|
|
370
|
Accessibility
99.47%
|
|
|---|---|
|
|
5 critical
5 serious
0 moderate
2 minor
|
|
|
72
|
sheelah
changed the title
sheelah
added
needs-a11y
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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/docs/llm/upgrade-guides/16.0-UPGRADE-GUIDE.md`:
- Around line 304-305: Update the Count Badge `inverse` entries in
modules/docs/llm/upgrade-guides/16.0-UPGRADE-GUIDE.md lines 304-305 and
modules/docs/mdx/16.0-UPGRADE-GUIDE.mdx lines 308-309 to explicitly state that
the variant is deprecated and provide the recommended replacement or migration
path, while retaining the existing color-change information.
In `@modules/react/information-highlight/stories/InformationHighlight.mdx`:
- Around line 80-83: Update the ctaPlacement “end” description in
modules/react/information-highlight/stories/InformationHighlight.mdx lines 80-83
to describe the link as vertically centered at the logical inline end rather
than right-aligned. Update
modules/react/information-highlight/stories/examples/CtaPlacementEnd.tsx lines
18-19 to replace “on the right” with equivalent logical-end wording.
🪄 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: 7a36fbf3-3a53-4f77-aa6d-bbd2b53498f5
📒 Files selected for processing (26)
modules/docs/llm/upgrade-guides/16.0-UPGRADE-GUIDE.mdmodules/docs/mdx/16.0-UPGRADE-GUIDE.mdxmodules/preview-react/status-indicator/lib/StatusIndicator.tsxmodules/preview-react/status-indicator/lib/StatusIndicatorIcon.tsxmodules/preview-react/status-indicator/lib/StatusIndicatorLabel.tsxmodules/react/badge/lib/CountBadge.tsxmodules/react/banner/lib/Banner.tsxmodules/react/banner/lib/hooks/useBannerIcon.tsxmodules/react/information-highlight/lib/InformationHighlight.tsxmodules/react/information-highlight/lib/hooks/useInformationHighlightModel.tsmodules/react/information-highlight/lib/parts/Body.tsxmodules/react/information-highlight/lib/parts/Heading.tsxmodules/react/information-highlight/lib/parts/Icon.tsxmodules/react/information-highlight/lib/parts/Link.tsxmodules/react/information-highlight/spec/InformationHighlight.spec.tsxmodules/react/information-highlight/stories/InformationHighlight.mdxmodules/react/information-highlight/stories/InformationHighlight.stories.tsmodules/react/information-highlight/stories/examples/Basic.tsxmodules/react/information-highlight/stories/examples/CtaPlacementEnd.tsxmodules/react/information-highlight/stories/visual_testing/testing.stories.tsxmodules/react/pill/lib/Pill.tsxmodules/react/pill/lib/PillIcon.tsxmodules/react/pill/lib/PillIconButton.tsxmodules/react/skeleton/lib/parts/SkeletonHeader.tsxmodules/react/skeleton/lib/parts/SkeletonShape.tsxmodules/react/skeleton/lib/parts/SkeletonText.tsx
williamjstanton
left a comment
williamjstanton
left a comment
There was a problem hiding this comment.
I stumbled on the Storybook Skeleton > Simulation example, and it is such a cool example!
I really wish we could incorporate our documented (on LoadingDots) screen reader live announcement support from here: https://workday.github.io/canvas-kit/?path=/docs/components-indicators-loading-dots--docs#accessibility
Good point. What's the best practice in terms of a11y for skeleton loaders? My understanding is that it's often best to hide them from assistive technology & have a single aria live polite status message. I can an issue to our backlog for this so we can take our time & get this right in our docs. |
Also adds documentation on link text recommendations
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/information-highlight/lib/parts/Link.tsx`:
- Around line 29-44: Update the Link component’s Hyperlink props ordering so the
fixed variant="secondary" is applied after handleCsProp(elemProps, ...),
preventing consumer-provided variant values from overriding the intended
styling.
🪄 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: 7d805342-0da6-46a6-9616-b9ae4c42be97
📒 Files selected for processing (6)
modules/react/information-highlight/lib/InformationHighlight.tsxmodules/react/information-highlight/lib/parts/Body.tsxmodules/react/information-highlight/lib/parts/Heading.tsxmodules/react/information-highlight/lib/parts/Link.tsxmodules/react/information-highlight/stories/InformationHighlight.mdxmodules/react/information-highlight/stories/examples/CtaPlacementEnd.tsx
🚧 Files skipped from review as they are similar to previous changes (2)
- modules/react/information-highlight/stories/InformationHighlight.mdx
- modules/react/information-highlight/lib/InformationHighlight.tsx
This is so that variant=secondary isn't overridden
Colocate part names with Heading, Body, and Link stencils and spread stencil.parts instead of hardcoding data-part attributes
1743d35 to
02e69e6
Compare
Yep, you got it!. Our |
Added #4096 for this. |
3 participants
Summary
Fixes: #3982
Release Category
Components
BREAKING CHANGES
An API change has been made in
InformationHighlight: The defaultvariantchanged frominformationaltodefault. Addvariant="informational"explicitly to preserve the previous appearance.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
ctaPlacementto Information Highlight (supports anendlayout), including updated stories/examples (now covers RTL).variant="default"support to Information Highlight.variantbehavior changed.ctaPlacement="end".