feat: Update Toast and Tooltip styles to match Sana#4053
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (6)
✅ Files skipped from review due to trivial changes (3)
📝 WalkthroughWalkthroughToast and Tooltip are restyled with updated v16 tokens and layout values. KBD adds an “In Tooltip” example and story, and the v16 upgrade guides add Toast and Tooltip sections describing the visual changes. ChangesToast and Tooltip visual updates
Estimated code review effort: 2 (Simple) | ~15 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
There was a problem hiding this comment.
Pull request overview
Updates Toast and Tooltip component styling to align with Sana/v16 visual design, and documents these changes in the v16 upgrade guide. Also adds a KBD Storybook example demonstrating use of KBD within a Tooltip title.
Changes:
- Restyles
TooltipContainer(type level, padding, light surface + border, rounding, spacing). - Restyles
Toastcontainer and subcomponents (container padding/gap/border/shadow, icon sizing, close icon positioning/size, body spacing). - Adds KBD “In Tooltip” Storybook example and updates v16 upgrade guides (MDX + LLM copy) with Toast/Tooltip notes.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| modules/react/tooltip/lib/TooltipContainer.tsx | Tooltip container visual/token updates for Sana styling |
| modules/react/toast/lib/Toast.tsx | Toast container layout + v16 token styling (padding, border, surface, shadow) |
| modules/react/toast/lib/ToastBody.tsx | Adjust Toast body spacing to match new container padding strategy |
| modules/react/toast/lib/ToastIcon.tsx | Update Toast icon sizing approach |
| modules/react/toast/lib/ToastCloseIcon.tsx | Update close icon positioning and size |
| modules/labs-react/kbd/stories/KBD.stories.tsx | Adds new “InTooltip” story export |
| modules/labs-react/kbd/stories/KBD.mdx | Documents new “In Tooltip” example usage |
| modules/labs-react/kbd/stories/examples/InTooltip.tsx | New example showing KBD content within Tooltip title |
| modules/docs/mdx/16.0-UPGRADE-GUIDE.mdx | Adds Toast/Tooltip upgrade notes |
| modules/docs/llm/upgrade-guides/16.0-UPGRADE-GUIDE.md | Mirrors upgrade notes for LLM docs |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| borderRadius: system.legacy.shape.full, | ||
| border: `${px2rem(1)} solid ${system.color.border.default}`, | ||
| zIndex: -1, | ||
| margin: system.legacy.gap.xs, | ||
| backgroundColor: system.legacy.color.surface.contrast.default, | ||
| margin: 0, | ||
| backgroundColor: system.legacy.color.surface.default, |
| <KBD aria-keyshortcuts="Command+P"> | ||
| <KBD.Item> | ||
| <KBD variant="plain"> | ||
| <KBD.Item>⌘</KBD.Item> | ||
| <KBD.Item>C</KBD.Item> | ||
| </KBD> | ||
| </KBD.Item> | ||
| </KBD> | ||
| </div> | ||
| } | ||
| > | ||
| <SecondaryButton icon={copyIcon} /> |
| - `Toast.Body` no longer has a gap between text and link. | ||
| - `Toast.CloseIcon` is set to have absolute positioning and aligned with the top-right corner (in | ||
| LTR). | ||
| - `Toast.CloseIcon` was changed to be `extra-small` tertiary button. |
| - `Toast.Body` no longer has a gap between text and link. | ||
| - `Toast.CloseIcon` is set to have absolute positioning and aligned with the top-right corner (in | ||
| LTR). | ||
| - `Toast.CloseIcon` was changed to be `extra-small` tertiary button. |
| LTR). | ||
| - `Toast.CloseIcon` was changed to be `extra-small` tertiary button. | ||
|
|
||
| ### Tooltip |
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
modules/labs-react/kbd/stories/examples/InTooltip.tsx (1)
24-31: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winSimplify the KBD nesting structure.
The current structure nests
KBDinsideKBD.Iteminside an outerKBD, which is more complex than the documented pattern. The standard usage shown inKBD.mdxis a flatKBDwithKBD.Itemchildren. The outerKBDand its singleKBD.Itemwrapper add no visual or semantic value.♻️ Proposed simplification
- <KBD aria-keyshortcuts="Command+P"> - <KBD.Item> - <KBD variant="plain"> - <KBD.Item>⌘</KBD.Item> - <KBD.Item>C</KBD.Item> - </KBD> - </KBD.Item> - </KBD> + <KBD variant="plain"> + <KBD.Item aria-label="Command">⌘</KBD.Item> + <KBD.Item>C</KBD.Item> + </KBD>🤖 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/labs-react/kbd/stories/examples/InTooltip.tsx` around lines 24 - 31, The KBD example uses an unnecessary nested wrapper pattern, with a KBD inside KBD.Item inside an outer KBD, which should be simplified to the documented flat KBD composition. Update the InTooltip story to use the KBD component directly with KBD.Item children, removing the extra outer KBD and the single-item wrapper while preserving the same displayed shortcut keys and aria-keyshortcuts behavior.
🤖 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 261-273: The Toast entry in the upgrade guide is missing the
standard PR reference used by other Component Updates sections. Update the Toast
section in the docs upgrade guide to include a matching PR link, following the
same `**PR:**` pattern as Avatar, Buttons, and Card so it is consistent and
traceable.
In `@modules/docs/mdx/16.0-UPGRADE-GUIDE.mdx`:
- Around line 267-279: The Toast section under Component Updates is missing the
standard PR reference used by the other component entries. Update the Toast
subsection in the upgrade guide to include a matching `**PR:**` link for
traceability, consistent with the Avatar, Buttons, and Card sections, and place
it alongside the existing Toast notes so the section format stays uniform.
In `@modules/labs-react/kbd/stories/examples/InTooltip.tsx`:
- Around line 24-35: Update the InTooltip example to add accessible names where
assistive text is missing: in the KBD composition, give the glyph item for ⌘ an
aria-label with the spoken modifier name, and give the icon-only SecondaryButton
an aria-label that describes its action. Use the existing KBD and
SecondaryButton usages in InTooltip.tsx to locate the spots and keep the visible
UI unchanged.
- Around line 24-31: The shortcut metadata is attached to the wrong element and
uses the wrong value: in the InTooltip example, `aria-keyshortcuts` on the `KBD`
display should be changed to match the visual shortcut (`Command+C`), and the
actual `aria-keyshortcuts` should be moved onto the `SecondaryButton` control
that the shortcut activates. Update the `KBD`/`KBD.Item` tooltip markup
accordingly so the accessible shortcut exposed to assistive tech matches the
displayed keys and follows the documented pattern.
---
Nitpick comments:
In `@modules/labs-react/kbd/stories/examples/InTooltip.tsx`:
- Around line 24-31: The KBD example uses an unnecessary nested wrapper pattern,
with a KBD inside KBD.Item inside an outer KBD, which should be simplified to
the documented flat KBD composition. Update the InTooltip story to use the KBD
component directly with KBD.Item children, removing the extra outer KBD and the
single-item wrapper while preserving the same displayed shortcut keys and
aria-keyshortcuts behavior.
🪄 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: e2eea64a-0a8c-46b2-8322-3f722ce09f5f
📒 Files selected for processing (10)
modules/docs/llm/upgrade-guides/16.0-UPGRADE-GUIDE.mdmodules/docs/mdx/16.0-UPGRADE-GUIDE.mdxmodules/labs-react/kbd/stories/KBD.mdxmodules/labs-react/kbd/stories/KBD.stories.tsxmodules/labs-react/kbd/stories/examples/InTooltip.tsxmodules/react/toast/lib/Toast.tsxmodules/react/toast/lib/ToastBody.tsxmodules/react/toast/lib/ToastCloseIcon.tsxmodules/react/toast/lib/ToastIcon.tsxmodules/react/tooltip/lib/TooltipContainer.tsx
| ### Toast | ||
|
|
||
| #### Visual Updates | ||
|
|
||
| - Padding is now applied to the `Toast` container itself instead of being distributed across | ||
| `Toast.Body` and `Toast.Icon`. | ||
| - Gap between the icon, body, and close icon is now `base.size150` (12px). | ||
| - Shape uses `system.shape.xl`. | ||
| - `Toast.Body` no longer has a gap between text and link. | ||
| - `Toast.CloseIcon` is set to have absolute positioning and aligned with the top-right corner (in | ||
| LTR). | ||
| - `Toast.CloseIcon` was changed to be `extra-small` tertiary button. | ||
|
|
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Add a PR: link to the Toast section.
Every other component section under "Component Updates" includes a **PR:** link (e.g., Avatar → #3981, Buttons → #4000, Card → #4014). The new Toast section omits this. Add the PR link for consistency and traceability.
📝 Suggested fix
### Toast
+**PR:** [`#4053`](https://github.com/Workday/canvas-kit/pull/4053)
+
#### Visual Updates📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| ### Toast | |
| #### Visual Updates | |
| - Padding is now applied to the `Toast` container itself instead of being distributed across | |
| `Toast.Body` and `Toast.Icon`. | |
| - Gap between the icon, body, and close icon is now `base.size150` (12px). | |
| - Shape uses `system.shape.xl`. | |
| - `Toast.Body` no longer has a gap between text and link. | |
| - `Toast.CloseIcon` is set to have absolute positioning and aligned with the top-right corner (in | |
| LTR). | |
| - `Toast.CloseIcon` was changed to be `extra-small` tertiary button. | |
| ### Toast | |
| **PR:** [`#4053`](https://github.com/Workday/canvas-kit/pull/4053) | |
| #### Visual Updates | |
| - Padding is now applied to the `Toast` container itself instead of being distributed across | |
| `Toast.Body` and `Toast.Icon`. | |
| - Gap between the icon, body, and close icon is now `base.size150` (12px). | |
| - Shape uses `system.shape.xl`. | |
| - `Toast.Body` no longer has a gap between text and link. | |
| - `Toast.CloseIcon` is set to have absolute positioning and aligned with the top-right corner (in | |
| LTR). | |
| - `Toast.CloseIcon` was changed to be `extra-small` tertiary button. |
🤖 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/llm/upgrade-guides/16.0-UPGRADE-GUIDE.md` around lines 261 -
273, The Toast entry in the upgrade guide is missing the standard PR reference
used by other Component Updates sections. Update the Toast section in the docs
upgrade guide to include a matching PR link, following the same `**PR:**`
pattern as Avatar, Buttons, and Card so it is consistent and traceable.
| ### Toast | ||
|
|
||
| #### Visual Updates | ||
|
|
||
| - Padding is now applied to the `Toast` container itself instead of being distributed across | ||
| `Toast.Body` and `Toast.Icon`. | ||
| - Gap between the icon, body, and close icon is now `base.size150` (12px). | ||
| - Shape uses `system.shape.xl`. | ||
| - `Toast.Body` no longer has a gap between text and link. | ||
| - `Toast.CloseIcon` is set to have absolute positioning and aligned with the top-right corner (in | ||
| LTR). | ||
| - `Toast.CloseIcon` was changed to be `extra-small` tertiary button. | ||
|
|
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Add a PR: link to the Toast section.
Every other component section under "Component Updates" includes a **PR:** link (e.g., Avatar → #3981, Buttons → #4000, Card → #4014). The new Toast section omits this. Add the PR link for consistency and traceability.
📝 Suggested fix
### Toast
+**PR:** [`#4053`](https://github.com/Workday/canvas-kit/pull/4053)
+
#### Visual Updates📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| ### Toast | |
| #### Visual Updates | |
| - Padding is now applied to the `Toast` container itself instead of being distributed across | |
| `Toast.Body` and `Toast.Icon`. | |
| - Gap between the icon, body, and close icon is now `base.size150` (12px). | |
| - Shape uses `system.shape.xl`. | |
| - `Toast.Body` no longer has a gap between text and link. | |
| - `Toast.CloseIcon` is set to have absolute positioning and aligned with the top-right corner (in | |
| LTR). | |
| - `Toast.CloseIcon` was changed to be `extra-small` tertiary button. | |
| ### Toast | |
| **PR:** [`#4053`](https://github.com/Workday/canvas-kit/pull/4053) | |
| #### Visual Updates | |
| - Padding is now applied to the `Toast` container itself instead of being distributed across | |
| `Toast.Body` and `Toast.Icon`. | |
| - Gap between the icon, body, and close icon is now `base.size150` (12px). | |
| - Shape uses `system.shape.xl`. | |
| - `Toast.Body` no longer has a gap between text and link. | |
| - `Toast.CloseIcon` is set to have absolute positioning and aligned with the top-right corner (in | |
| LTR). | |
| - `Toast.CloseIcon` was changed to be `extra-small` tertiary button. |
🤖 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 267 - 279, The Toast
section under Component Updates is missing the standard PR reference used by the
other component entries. Update the Toast subsection in the upgrade guide to
include a matching `**PR:**` link for traceability, consistent with the Avatar,
Buttons, and Card sections, and place it alongside the existing Toast notes so
the section format stays uniform.
| <KBD aria-keyshortcuts="Command+P"> | ||
| <KBD.Item> | ||
| <KBD variant="plain"> | ||
| <KBD.Item>⌘</KBD.Item> | ||
| <KBD.Item>C</KBD.Item> | ||
| </KBD> | ||
| </KBD.Item> | ||
| </KBD> |
There was a problem hiding this comment.
🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win
Fix aria-keyshortcuts value mismatch: "Command+P" vs visual ⌘C.
The aria-keyshortcuts attribute is set to "Command+P" but the visual keys display ⌘ + C (Command+C). This mismatch will cause assistive technology to announce the wrong shortcut. It should be "Command+C".
Additionally, aria-keyshortcuts should be on the SecondaryButton (the control the shortcut triggers), not on the KBD display element inside the tooltip. The MDX documentation in this same PR (lines 106–121) explicitly states: "Expose the actual shortcut on the control it triggers."
🐛 Proposed fix
export const InTooltip = () => {
return (
<Tooltip
title={
<div className={flexContainer}>
<Subtext size="large">Copy to clipboard</Subtext>
- <KBD aria-keyshortcuts="Command+P">
- <KBD.Item>
- <KBD variant="plain">
- <KBD.Item>⌘</KBD.Item>
- <KBD.Item>C</KBD.Item>
- </KBD>
- </KBD.Item>
- </KBD>
+ <KBD variant="plain">
+ <KBD.Item aria-label="Command">⌘</KBD.Item>
+ <KBD.Item>C</KBD.Item>
+ </KBD>
</div>
}
>
- <SecondaryButton icon={copyIcon} />
+ <SecondaryButton icon={copyIcon} aria-label="Copy" aria-keyshortcuts="Command+C" />
</Tooltip>
);
};📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <KBD aria-keyshortcuts="Command+P"> | |
| <KBD.Item> | |
| <KBD variant="plain"> | |
| <KBD.Item>⌘</KBD.Item> | |
| <KBD.Item>C</KBD.Item> | |
| </KBD> | |
| </KBD.Item> | |
| </KBD> | |
| <KBD variant="plain"> | |
| <KBD.Item aria-label="Command">⌘</KBD.Item> | |
| <KBD.Item>C</KBD.Item> | |
| </KBD> |
🤖 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/labs-react/kbd/stories/examples/InTooltip.tsx` around lines 24 - 31,
The shortcut metadata is attached to the wrong element and uses the wrong value:
in the InTooltip example, `aria-keyshortcuts` on the `KBD` display should be
changed to match the visual shortcut (`Command+C`), and the actual
`aria-keyshortcuts` should be moved onto the `SecondaryButton` control that the
shortcut activates. Update the `KBD`/`KBD.Item` tooltip markup accordingly so
the accessible shortcut exposed to assistive tech matches the displayed keys and
follows the documented pattern.
| <KBD aria-keyshortcuts="Command+P"> | ||
| <KBD.Item> | ||
| <KBD variant="plain"> | ||
| <KBD.Item>⌘</KBD.Item> | ||
| <KBD.Item>C</KBD.Item> | ||
| </KBD> | ||
| </KBD.Item> | ||
| </KBD> | ||
| </div> | ||
| } | ||
| > | ||
| <SecondaryButton icon={copyIcon} /> |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Accessibility gaps: missing aria-label on ⌘ and on the icon-only button.
Two accessibility issues contradict the guidance documented in KBD.mdx:
-
⌘symbol lacksaria-label— The MDX (lines 89–94) states that glyphs like⌘are not reliably announced and must have anaria-labelwith the spoken name (e.g.,aria-label="Command"). -
Icon-only
SecondaryButtonlacksaria-label— A button with only an icon and no text content is not accessible to screen reader users.
🛡️ Proposed fix
- <KBD.Item>⌘</KBD.Item>
+ <KBD.Item aria-label="Command">⌘</KBD.Item>- <SecondaryButton icon={copyIcon} />
+ <SecondaryButton icon={copyIcon} aria-label="Copy" />📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <KBD aria-keyshortcuts="Command+P"> | |
| <KBD.Item> | |
| <KBD variant="plain"> | |
| <KBD.Item>⌘</KBD.Item> | |
| <KBD.Item>C</KBD.Item> | |
| </KBD> | |
| </KBD.Item> | |
| </KBD> | |
| </div> | |
| } | |
| > | |
| <SecondaryButton icon={copyIcon} /> | |
| <KBD aria-keyshortcuts="Command+P"> | |
| <KBD.Item> | |
| <KBD variant="plain"> | |
| <KBD.Item aria-label="Command">⌘</KBD.Item> | |
| <KBD.Item>C</KBD.Item> | |
| </KBD> | |
| </KBD.Item> | |
| </KBD> | |
| </div> | |
| } | |
| > | |
| <SecondaryButton icon={copyIcon} aria-label="Copy" /> |
🤖 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/labs-react/kbd/stories/examples/InTooltip.tsx` around lines 24 - 35,
Update the InTooltip example to add accessible names where assistive text is
missing: in the KBD composition, give the glyph item for ⌘ an aria-label with
the spoken modifier name, and give the icon-only SecondaryButton an aria-label
that describes its action. Use the existing KBD and SecondaryButton usages in
InTooltip.tsx to locate the spots and keep the visible UI unchanged.
Workday/canvas-kit
|
||||||||||||||||||||||||||||||||||||||||
| Project |
Workday/canvas-kit
|
| Branch Review |
issue3989_toast-tooltip
|
| Run status |
|
| Run duration | 02m 30s |
| 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.51%
|
|
|---|---|
|
|
1541
|
|
|
371
|
Accessibility
99.41%
|
|
|---|---|
|
|
5 critical
5 serious
0 moderate
2 minor
|
|
|
72
|
…ose/canvas-kit into issue3989_toast-tooltip
| outline: `${px2rem(1)} solid transparent`, | ||
| outlineOffset: `-${px2rem(1)}`, | ||
| borderRadius: system.legacy.shape.full, | ||
| border: `${px2rem(1)} solid ${system.color.border.default}`, |
There was a problem hiding this comment.
nit: Do we want to use system.legacy.color.border.default instead here since it will then have a fallback value?
| title={ | ||
| <div className={flexContainer}> | ||
| <Subtext size="large">Copy to clipboard</Subtext> | ||
| <KBD aria-keyshortcuts="Command+P"> |
| padding: 0, | ||
| gap: system.legacy.gap.xs, | ||
| borderRadius: system.legacy.shape.xl, | ||
| paddingBlock: system.padding.sm, |
There was a problem hiding this comment.
nit: Do we want to use system.legacy.* tokens for the padding, gap, border-radius, background color, and border here?
sheelah
left a comment
There was a problem hiding this comment.
LG! Left just a few minor comments.
Summary
Fixes: #3989
Update Toast and Tooltip styles to match Sana
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
Documentation
New Features
Bug Fixes