feat: Implement Sana Canvas Text Input, Text Area, Form Field, and Color Picker - #3992
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:
📝 WalkthroughWalkthroughTextInput, FormFieldGroupList, and ColorSwatch now use corner-shape stencil styling and updated status-state backgrounds. Sana Canvas story, example, and visual-testing coverage were added across input components, and the v16 upgrade guide now includes component notes. ChangesSana Canvas input styling and docs
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related issues
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 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 |
Workday/canvas-kit
 |
||||||||||||||||||||||||||||||||||||||||
| Project |
Workday/canvas-kit
|
| Branch Review |
feat/2026-06-15-sc-text-input-area-ff-cp
|
| Run status |
|
| Run duration | 02m 23s |
| Commit |
|
| Committer | James Fan |
| 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
|
|
|
71
|
d51ca0b to
b3f1b31
Compare
… feat/2026-06-15-sc-text-input-area-ff-cp
![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/mdx/16.0-UPGRADE-GUIDE.mdx`:
- Around line 120-124: Remove the duplicated grouped-input padding guidance in
the upgrade guide so the migration note only appears once. In the MDX section
covering grouped inputs, consolidate the two bullets into a single statement
that keeps the new 8px block and 4px inline values along with the “previously”
values, and delete the redundant duplicate wording.
In
`@modules/preview-react/color-picker/stories/visual-testing/ColorPicker.stories.tsx`:
- Around line 28-60: The top-level fragment returned by the theme .map() in
ColorPicker.stories.tsx is missing a React key, which causes the warning. Update
the mapped wrapper around StaticStates and ComponentStatesTable from the
shorthand fragment to React.Fragment with an explicit key derived from the theme
value (including the undefined case), so each rendered block is uniquely
identified.
🪄 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: 4f4b261d-e71f-4184-b9d8-d0694ed47df6
📒 Files selected for processing (5)
modules/docs/mdx/16.0-UPGRADE-GUIDE.mdxmodules/preview-react/color-picker/stories/visual-testing/ColorPicker.stories.tsxmodules/react/color-picker/lib/parts/ColorSwatch.tsxmodules/react/form-field/lib/FormFieldGroupList.tsxmodules/react/text-input/lib/TextInput.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- modules/react/form-field/lib/FormFieldGroupList.tsx
| textInputStencil({ | ||
| width: typeof width === 'number' ? px2rem(width) : width, | ||
| grow: grow === true ? 'true' : grow === false ? 'false' : undefined, | ||
| error, | ||
| }) |
There was a problem hiding this comment.
This fix addresses a TypeScript issue which appeared after introducing extends cornerShapeStencil:
Explanation of the fix from Cursor:
And why the error only happened after introducing extends cornerShapeStencil:
There was a problem hiding this comment.
connected to #3896, I'll talk to Alan about that the next week
| textInputStencil({ | ||
| width: typeof width === 'number' ? px2rem(width) : width, | ||
| grow: grow === true ? 'true' : grow === false ? 'false' : undefined, | ||
| error, | ||
| }) |
There was a problem hiding this comment.
we need to fix that issue finally :D I'll talk to Alan about that when he is back
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/color-picker/stories/color-preview/ColorPreview.stories.ts`:
- Around line 33-35: The SanaCanvas story is using the wrong renderer, so it
does not exercise the Sana Canvas variant. Update the SanaCanvas export in
ColorPreview.stories.ts to point to the correct story render function/component
instead of RefForwardingExample, and verify the story name remains aligned with
the intended Sana Canvas preview.
🪄 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: afa65cb7-5443-4f64-a9a3-601e7958b6db
📒 Files selected for processing (23)
.storybook/routes.jsmodules/docs/mdx/16.0-UPGRADE-GUIDE.mdxmodules/preview-react/color-picker/stories/examples/InputInteraction.tsxmodules/preview-react/color-picker/stories/visual-testing/ColorPicker.stories.tsxmodules/react/color-picker/stories/color-input/ColorInput.mdxmodules/react/color-picker/stories/color-input/ColorInput.stories.tsmodules/react/color-picker/stories/color-input/examples/SanaCanvas.tsxmodules/react/color-picker/stories/color-preview/ColorPreview.mdxmodules/react/color-picker/stories/color-preview/ColorPreview.stories.tsmodules/react/color-picker/stories/color-preview/examples/SanaCanvas.tsxmodules/react/color-picker/stories/visualTesting.stories.tsxmodules/react/form-field/stories/FormField.mdxmodules/react/form-field/stories/FormField.stories.tsmodules/react/form-field/stories/examples/SanaCanvas.tsxmodules/react/form-field/stories/visualTesting.stories.tsxmodules/react/text-area/stories/TextArea.mdxmodules/react/text-area/stories/TextArea.stories.tsmodules/react/text-area/stories/examples/SanaCanvas.tsxmodules/react/text-area/stories/visualTesting.stories.tsxmodules/react/text-input/stories/TextInput.mdxmodules/react/text-input/stories/TextInput.stories.tsmodules/react/text-input/stories/examples/SanaCanvas.tsxmodules/react/text-input/stories/visualTesting.stories.tsx
✅ Files skipped from review due to trivial changes (3)
- modules/react/color-picker/stories/color-preview/examples/SanaCanvas.tsx
- modules/preview-react/color-picker/stories/examples/InputInteraction.tsx
- modules/react/color-picker/stories/visualTesting.stories.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- modules/docs/mdx/16.0-UPGRADE-GUIDE.mdx
| width: system.legacy.size.xxs, | ||
| height: system.legacy.size.xxs, | ||
| borderRadius: system.legacy.shape.sm, | ||
| backgroundColor: color, |
There was a problem hiding this comment.
Medium: ColorSwatch does not preserve color meaning in Windows High Contrast / forced-colors mode. Since the swatch’s semantic value is its actual color, backgroundColor: color will likely be remapped by forced-colors, causing swatches to appear identical. Consider forcedColorAdjust: 'none' for the swatch color surface, plus a real border/outline fallback for white/selected states since box-shadow is forced to none.
There was a problem hiding this comment.
@williamjstanton This should be addressed now. I've added forcedColorAdjust: 'none' to the swatch color surface as well as the outline fallback.
| width: 'fit-content', | ||
| }, | ||
| modifiers: { | ||
| error: { |
There was a problem hiding this comment.
Medium: Error/caution visual affordances rely heavily on boxShadow, which is removed in forced-colors mode. For TextInput, TextArea inherits this behavior; for grouped fields, the group ring is entirely shadow-based. Add @media (forced-colors: active) fallbacks using real border/outline and system colors so Windows High Contrast users can still perceive invalid/caution states.
There was a problem hiding this comment.
@williamjstanton I'm consulting the MDN docs for system colors. Which color would you recommend I use for error and caution states?
For example, ActiveText is visually similar to our standard error color (red), but doesn't line up semantically (i.e., the form field error has nothing to do with the "text of active links").
There was a problem hiding this comment.
@williamjstanton Also, it wasn't quite clear from your original comment, but is it ok to leave TextInput and TextArea as-is given they both still have a border in the error and caution states?
There was a problem hiding this comment.
@williamjstanton I'm consulting the MDN docs for system colors. Which color would you recommend I use for error and caution states?
For example, ActiveText is visually similar to our standard error color (red), but doesn't line up semantically (i.e., the form field error has nothing to do with the "text of active links").
This is a really good question, and I don't know. There isn't any semantically correct option available at all. The recommendation is to use border differentiation like double but I don't think it is necessary in the case of FormFieldGroupList. I think we could try a simple transparent border or outline as a fallback?
.form-field.error {
border: 2px solid red; /* Standard mode color */
}
@media (forced-colors: active) {
.form-field.error {
/* Use a thicker or distinct border style with standard system colors */
border: 4px double ButtonBorder;
}
There was a problem hiding this comment.
I'm using outline to prevent layout shifts since FormFieldGroupList doesn't have a border to begin with. My plan was to maintain a similar thickness to the boxShadow for the outline:
error: {
boxShadow: `inset 0 0 0 ${px2rem(2)} ${system.legacy.color.brand.border.critical}`,
'@media (forced-colors: active)': {
outline: `solid ${px2rem(2)} ActiveText`,
},
},
I can double the thickness, however. Out of curiosity, what's the rationale behind differentiating the border if forced-colors is active?
Also, did you mean to use ButtonBorder (presents as black to me in macOS) in your example snippet?
There was a problem hiding this comment.
Also, did you mean to use
ButtonBorder(presents as black to me in macOS) in your example snippet?
Really? Presents as black? I'd assumed it would present as white on a dark background. I'll definitely need to test on a real Windows machine.
Rationale: Since we can't use color to differentiate a border on an input that already uses a border in a valid state (e.g. TextInput), that's where it is useful to use more thickness or a double style to differentiate the error states from valid states.
I don't think that rationale applies in this situation with the group list. (Because the border / outline / boxShadow is absent in valid state.)
There was a problem hiding this comment.
@williamjstanton Apologies, I was confused. I didn't realize Windows High Contrast mode was an example of a forced colors mode (I thought the two could be toggled independently of one another. ButtonBorder presented as black to me in macOS because I obviously wasn't on Windows High Contrast mode.
Appreciate the explanation. The original issue should be resolved:
- I've added the fallback outline to the grouped fields error and caution states. I used the same width as the boxShadow used in the error/caution states since there is no border/outline/boxShadow in the valid state.
- I've added the fallback outline to TextInput and TextArea. It displays at double the width of the focus ring to differentiate it from the focus ring (I did not want to modify the focus ring styling).
… feat/2026-06-15-sc-text-input-area-ff-cp # Conflicts: # modules/docs/mdx/16.0-UPGRADE-GUIDE.mdx
| color, | ||
| iconColor: pickForegroundColor(color), | ||
| withShadow: showCheck || lowerCasedColor === '#ffffff', | ||
| withShadow: showCheck || lowerCasedColor === '#ffffff' ? 'true' : undefined, |
RayRedGoose
added
approved
williamjstanton
left a comment
williamjstanton
left a comment
There was a problem hiding this comment.
Thanks for all the hard work on this, looks like the high contrast mode fixes are working well.
|
@RayRedGoose will update FormFieldGroupList padding in #4017. To keep things moving, I've reverted the padding changes in this PR. For reference, I also reverted the following addition to the Form Field > Visual Updates section in the v16 Upgrade Guide: |
4 participants
Summary
Fixes: #3983
Release Category
Components
Checklist
ready for reviewhas been added to PRFor the Reviewer
Summary by CodeRabbit
Summary by CodeRabbit