From 584ffe8f599d98b8d1e4dfbd554a480d14ade527 Mon Sep 17 00:00:00 2001 From: Ola Onikosi Date: Mon, 5 Oct 2026 11:31:04 -0400 Subject: [PATCH 1/2] chore(skills): Add contribute-component maintainer skill Co-authored-by: Cursor --- .../SKILL.md | 151 ++++++++++++++ .../references.md | 185 ++++++++++++++++++ 2 files changed, 336 insertions(+) create mode 100644 skills/sana-canvas-kit-contribute-component/SKILL.md create mode 100644 skills/sana-canvas-kit-contribute-component/references.md diff --git a/skills/sana-canvas-kit-contribute-component/SKILL.md b/skills/sana-canvas-kit-contribute-component/SKILL.md new file mode 100644 index 0000000000..ee78a62a35 --- /dev/null +++ b/skills/sana-canvas-kit-contribute-component/SKILL.md @@ -0,0 +1,151 @@ +--- +name: sana-canvas-kit-contribute-component +description: >- + Maintainer workflow for adding a new component to the canvas-kit repo itself (modules/react, + modules/preview-react, modules/labs-react) from a GitHub issue and Figma spec: research sibling + components, propose a plan, scaffold the package, build with collection models and Sana-aligned + stencils, add stories/tests/SSR/Cypress, validate, and write the PR summary. Use when the user + asks to implement a Canvas Kit GitHub issue, create a new Canvas Kit component, or contribute a + component to the library. Not for consumer apps — use /sana-canvas-kit-builder there. +--- + +# Contribute a Component to Canvas Kit + +Maintainer-only. This covers building a component **inside the canvas-kit repo**, where +[AGENTS.md](../../AGENTS.md) and [STYLE.md](../../STYLE.md) apply (`system.legacy.*` in `lib/`, +`cornerShapeStencil`, Chromatic class twins, public API impact). For components in a consumer app, +use `/sana-canvas-kit-builder` instead. + +Factory patterns → `/sana-canvas-kit-builder`. Token roles → `/sana-canvas-kit-tokens`. A11y +patterns → `/sana-canvas-kit-accessibility`. This skill adds the repo-specific workflow on top. + +## When to apply + +- "Implement GitHub issue #NNNN" for a new Canvas Kit component +- "Create a `` component in Canvas Kit" / "add `` to modules/react" +- Iterating on a new component's visuals to match the current Sana design + +## Workflow + +Copy this checklist and track progress: + +```text +- [ ] 1. Gather the spec: GitHub issue + Figma (metadata → design context per state) +- [ ] 2. Pick the target branch and package tier (Main / Preview / Labs) +- [ ] 3. Study the closest sibling components; grep for existing helpers before inventing +- [ ] 4. Propose a plan to the user (API, a11y roles, styling, open design/a11y questions) — wait +- [ ] 5. Scaffold the package (see file layout in references.md) +- [ ] 6. Model: extend collection models; don't hand-roll selection, guards, or callbacks +- [ ] 7. Subcomponents: compose collection elemProps hooks in the right order +- [ ] 8. Style with Sana-aligned stencils (system.legacy.*, cornerShapeStencil, class twins) +- [ ] 9. Stories, examples, MDX, visual-testing stories +- [ ] 10. Tests: unit (verifyComponent), SSR, Cypress with checkA11y +- [ ] 11. Validate: typecheck, test, lint, cypress, Storybook visual check against Figma +- [ ] 12. PR summary: changed files, assumptions, TODOs for needs-design / needs-a11y +``` + +### 1. Gather the spec + +```bash +gh issue view --json title,body,labels,comments +``` + +Figma (via the Figma MCP): + +1. Call `get_metadata` on the spec page node from the issue link. Page/canvas-level nodes often fail + with `get_design_context`; metadata gives you the child instance IDs and usually a + **Documentation** text node with anatomy, states, and a11y guidance — read it in full. +2. Call `get_design_context` (with `disableCodeConnect: true`) on one instance **per state** + (unselected, selected, disabled, error…). The output is Tailwind reference code: translate the + `var(--cnvs-sys-…)` names to tokens; never copy Tailwind or raw px. + +Labels like `needs-design` / `needs-a11y` don't block work, but every decision you make in those +areas becomes a TODO in the PR summary, not a silent guess in code. + +### 2. Branch and tier + +- New additive component → `prerelease/minor`. Breaking changes → `prerelease/major` only. Check + `git branch --show-current` and flag a mismatch rather than assuming. +- Main (`modules/react`) vs Preview (`modules/preview-react`) vs Labs: if the API is likely to + change after design/a11y review, ask whether Preview or Labs is the better home. + +### 3. Study siblings + +Read the closest existing components end to end (lib, hooks, stories, specs, Cypress) before writing +anything. Pick siblings by behavior, not by name: + +| Behavior | Read | +| --------------------------------------------- | ------------------------------------------------------------------------------------ | +| Compound component over a list with selection | `modules/react/segmented-control`, `modules/react/tabs` | +| Selection model, roving focus, keyboard | `modules/react/collection` (`useListModel`, `useSelectionListModel`, `useListItem*`) | +| Error / caution group ring, form wiring | `modules/preview-react/radio` (`RadioGroup`), `modules/react/checkbox` | +| Sana selected / hover / focus item styling | `modules/react/menu/lib/MenuItem.tsx` | +| Multi-selection examples | `modules/react/collection/stories/mdx/examples/MultiSelection.tsx` | + +Before adding any utility (SSR-safe layout effect, focus helper, id helper), grep the repo for an +existing one. If none exists, keep the local version private to the file that needs it. + +### 4. Propose a plan — then stop + +Present: public API (``, `.List`, `.Item`, `useModel`), model config with +defaults, ARIA roles per mode, keyboard behavior, styling approach, file list, and the open +design/a11y questions. Wait for the user before editing. + +### 5–7. Build + +Use the file layout and model/hook rules in [references.md](references.md#file-layout). Key rules: + +- Extend `useListModel`; pick `singleSelectionManager` / `multiSelectionManager` via config. + `createModelHook` already provides `should` / `on` — don't reimplement them. +- `composeHooks` runs right-to-left; `useListItemRegister` goes **last**. Compose + `useListItemRovingFocus` / `useListItemSelect` directly — never call them inside another + `createElemPropsHook` body. +- Add `index.ts` exports and `export * from './'` in `modules/react/index.ts`. + +### 8. Style for Sana + +v16 Sana parity is where first drafts most often miss. Follow the checklist in +[references.md](references.md#sana-styling-checklist) — it lists the token substitutions and +patterns that typecheck and design review caught on SelectionGroup. + +### 9–10. Stories and tests + +See [references.md](references.md#stories-and-tests) for the required story set, MDX structure, and +test conventions. + +### 11. Validate + +```bash +cd modules/react && yarn typecheck:src # fast package-level typecheck +yarn typecheck # full set before PR +yarn test modules/react/ +yarn eslint modules/react/ cypress/component/.spec.tsx --fix +yarn cypress:run --spec cypress/component/.spec.tsx +yarn start # Storybook on :9001, compare to Figma per state +``` + +Compare every state in Storybook (with the Sana theme) side by side with the Figma instances from +step 1. Typecheck passing does not mean it looks right. + +### 12. PR summary + +Use the template in [references.md](references.md#pr-summary-template). Title format: +`feat(): Add component`. Don't stage `.cursor/` or `.claude/` artifacts. + +## Anti-patterns + +- ❌ Editing files before the user approves the plan +- ❌ Copying Tailwind / px values from Figma output instead of mapping to tokens +- ❌ Non-legacy `system.*` tokens in `lib/` when a `system.legacy.*` path exists +- ❌ Inventing ARIA when a sibling component or APG pattern already covers it +- ❌ Shared helpers across example files (each example must be copy-pasteable) +- ❌ Inline JSX in the CSF file, DOM snapshot tests, or ad hoc markup in Cypress +- ❌ Touching tsconfig / build / CI config or unrelated MCP/generated files +- ❌ Using `yarn create-component` output as-is — its templates use `Box`, which new code must not + +## Additional resources + +- [references.md](references.md) — file layout, Sana styling checklist, known gotchas, test + conventions, PR template +- `modules/docs/mdx/CREATING_COMPOUND_COMPONENTS.mdx`, `API_PATTERN_GUIDELINES.mdx`, + `DOCUMENTATION_GUIDELINES.mdx` diff --git a/skills/sana-canvas-kit-contribute-component/references.md b/skills/sana-canvas-kit-contribute-component/references.md new file mode 100644 index 0000000000..4a250f22d9 --- /dev/null +++ b/skills/sana-canvas-kit-contribute-component/references.md @@ -0,0 +1,185 @@ +# Contribute a Component — References + +Detail for [SKILL.md](SKILL.md). Worked example throughout: `SelectionGroup` (issue #1969). + +## File layout + +```text +modules/react// +├── index.ts # export * from each lib file (no default exports) +├── README.md # 3–5 lines + Storybook link (copy a sibling's shape) +├── LICENSE # copy from a sibling package +├── lib/ +│ ├── .tsx # createContainer; subcomponent JSDoc inside subComponents +│ ├── List.tsx # createSubcomponent('div') +│ ├── Item.tsx # createSubcomponent('button') + stencil +│ └── hooks/ +│ ├── useModel.tsx # createModelHook extending useListModel +│ └── useItem.tsx # composeHooks(...) +├── spec/ +│ ├── tsconfig.json # {"extends": "../../../../tsconfig.spec.json"} +│ ├── SSR.spec.tsx # @jest-environment node + renderToString +│ └── .spec.tsx # verifyComponent + behavior +└── stories/ + ├── tsconfig.json # {"extends": "../../../../tsconfig.stories.json"} + ├── .stories.ts # CSF; every story is {render: ImportedExample} + ├── .mdx # doc page (structure below) + ├── testing.stories.tsx # Chromatic states, chromatic: {disable: false} + └── examples/.tsx # one self-contained example per file + +cypress/component/.spec.tsx # mounts imported examples only +modules/react/index.ts # add: export * from './'; +``` + +No per-component `package.json` or tsconfig path entries were needed for `modules/react`. + +## Model and hook rules + +```tsx +export const useThingModel = createModelHook({ + defaultConfig: { + ...useListModel.defaultConfig, + /** JSDoc every option, @default on enums/booleans */ + mode: 'single' as 'single' | 'multiple', + orientation: 'horizontal' as typeof useListModel.defaultConfig.orientation, + }, + requiredConfig: useListModel.requiredConfig, +})(config => { + const selection = + config.selection || + (config.mode === 'multiple' ? multiSelectionManager : singleSelectionManager); + const list = useListModel(useListModel.mergeConfig(config, {selection, shouldVirtualize: false})); + return {...list, state: {...list.state, mode: config.mode}, events: list.events}; +}); +``` + +```tsx +export const useThingItem = composeHooks( + useListItemSelect, + createElemPropsHook(useThingModel)((model, _, elemProps: {'data-id'?: string} = {}) => ({ + role: model.state.mode === 'single' ? 'radio' : 'checkbox', + 'aria-checked': isSelected(elemProps['data-id'] || '', model.state), + })), + useListItemRovingFocus, + useListItemRegister // always last +); +``` + +- Controlled usage: accept `selectedIds` and sync with `events.setSelectedIds` in an effect; + uncontrolled uses `initialSelectedIds`. +- `onSelect` can fire on mount when single-select auto-selects the first item — account for it in + tests (`mock.mockClear()` after render) or avoid the auto-select. + +## Sana styling checklist + +Apply in `lib/` stencils: + +- [ ] Radius `shape.md`–`shape.xxxl` → `extends: cornerShapeStencil` + + `[cornerShapeStencil.vars.shape]: system.legacy.shape.lg` (not a raw `borderRadius`). + Exception: stencils extending `buttonStencil` set `buttonStencil.vars.borderRadius`. +- [ ] Typography: explicit `fontFamily: system.fontFamily.default`, + `fontWeight: system.fontWeight.normal`, and `system.legacy.fontSize/lineHeight/letterSpacing` + for the level (see `MenuItem`). Render the label as a plain `` that inherits color; + wrapping in `Subtext` fights the selected color. +- [ ] Selected: `system.legacy.color.brand.surface.selected` + + `system.legacy.color.brand.fg.selected`. `system.color.surface.selected` / + `system.color.fg.selected` don't exist in tokens-web 4.4 even though Figma shows + `--cnvs-sys-color-surface-selected`. +- [ ] Selected chips drop the visible border (`borderColor: 'transparent'`) and don't shift on + hover/active — the border would compete with the focus indicator. +- [ ] Hover / pressed: `system.legacy.color.surface.overlay.hover.default` / `.pressed.default`, + scoped to unselected items. +- [ ] Focus: `outline: 2px solid system.legacy.color.brand.border.primary` with + `outlineOffset: -2px` (MenuItem pattern), suppressed only under + `[data-whatinput='mouse'|'touch'|'pointer']`. +- [ ] Disabled: `opacity: system.opacity.disabled`, `cursor: 'default'`, match + `:disabled, .disabled, [aria-disabled="true"]`. +- [ ] Every pseudo-state has a class twin: `'&:is(:hover, .hover)'`, `.active`, `.focus`, + `.disabled`. +- [ ] Icons: set `[systemIconStencil.vars.color]: 'currentColor'` on the item; don't color icons + separately. +- [ ] Spacing: `system.legacy.padding.*` / `gap.*`. There's no `system.legacy.gap.xxs`; 6px is + `base.legacy.size75`. +- [ ] Error / caution group ring: copy preview `RadioGroup` (inset `box-shadow` using + `system.legacy.color.brand.border.critical` / `.caution` + `focus.caution.inner`). + +## Known gotchas + +| Symptom | Cause | Fix | +| -------------------------------------------------------------------------------------------- | ------------------------------------------------------- | ---------------------------------------------------------------------------------------------------------------- | +| `Module '@workday/canvas-kit-react/common' has no exported member 'CSProps'` | Wrong package | Import `CSProps` from `@workday/canvas-kit-styling` | +| `Type ... has no properties in common with CSProps & {className, style}` from `handleCsProp` | Props interface doesn't extend `CSProps` | `interface XProps extends CSProps` | +| jsdom: `Cannot create property 'border-width' on string` | `border` shorthand plus a nested `borderColor` override | Use `borderWidth` / `borderStyle` / `borderColor` longhands | +| `useLayoutEffect does nothing on the server` from your file | Measurement effect in render path | No shared helper exists; use a local `typeof document !== 'undefined' ? React.useLayoutEffect : React.useEffect` | +| Same warning still appears in SSR spec | `useListItemRegister` → `useMountLayout` in collection | Pre-existing; not caused by the new component | +| `'React' is declared but never read` | Only types used | Drop the import | +| Figma `get_design_context` returns "nothing selected" | Called on a page/canvas node | Use `get_metadata` first, then call on an instance id | + +## Stories and tests + +Required stories (add RTL if siblings have one): Basic, each mode/variant, each width/size variant, +Disabled, Error, Caution, RTL. + +MDX order: imports → `` → `# Canvas Kit ` → description → `## Installation` → +`## Usage` (first example titled **"Basic Example"**, ``) → +variants → `### Accessibility` → `## Component API` +(``) → `## Specifications` +(``). + +Tests: + +- Unit spec starts with `verifyComponent(.Item, {modelFn: useModel})`, then role/ARIA + and callback assertions. No snapshots, no class-name assertions. +- `SSR.spec.tsx` renders the full compound tree with `renderToString`. +- Cypress: mount imported examples; every example gets + `it('should not have any axe errors', () => cy.checkA11y())`; cover click, arrow keys, Space, + disabled, RTL. Use Given/When/Then `describe`/`context`/`it` nesting. +- `testing.stories.tsx`: `StaticStates` + `ComponentStatesTable` with columns for Default, Hover, + Focus, Active, Disabled (via class twins) and rows per mode/selection; separate table for Error / + Caution. + +## PR summary template + +```markdown +## Summary + +Adds `` (), a compound component for . + +Resolves: # + +## Release Category + +Components + +## What's included + +- ``, `.List`, `.Item`, `useModel` +- Variants: +- States: +- Stories, MDX, Chromatic visual tests, unit + SSR + Cypress specs + +## Where Should the Reviewer Start? + +`modules/react//lib/hooks/useModel.tsx`, then `Item.tsx` stencil. + +## Areas for Feedback? + +### needs-design (TODO) + +- [ ] + +### needs-a11y (TODO) + +- [ ] + +## Assumptions + +- + +## Validation + +- [ ] yarn typecheck +- [ ] yarn test modules/react/ +- [ ] yarn cypress:run --spec cypress/component/.spec.tsx +- [ ] Storybook compared to Figma per state (Sana theme) +``` From 9de258616f85d2f2aaae637913ac0151def7a87d Mon Sep 17 00:00:00 2001 From: "manuel.carrera" Date: Mon, 5 Oct 2026 10:19:11 -0600 Subject: [PATCH 2/2] docs(skills): Align contribute guidance with focus-visible and collections Replace data-whatinput focus guidance with :focus-visible, note that createStencil already sets box-sizing, and document when to extend the collection system in AGENTS.md and related skills. Co-authored-by: Cursor --- AGENTS.md | 51 ++++++++++++------- modules/docs/mdx/API_PATTERN_GUIDELINES.mdx | 24 ++++----- skills/README.md | 9 ++-- skills/sana-canvas-kit-builder/SKILL.md | 19 ++++++- .../SKILL.md | 16 ++++-- .../references.md | 33 ++++++++++-- skills/sana-canvas-kit-review/SKILL.md | 2 + skills/sana-canvas-kit-styling/SKILL.md | 5 ++ 8 files changed, 112 insertions(+), 47 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 2eb61c9c31..2185cf6df6 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -160,6 +160,12 @@ Rules that come up often: `export default` metadata). No TS `enum` (use disjoint string unions); prop names never repeat the component name (`type`, not `buttonType`) and avoid direction/color-coupled names (`leftIcon` breaks under RTL). +- **Lists, menus, tabs, segmented controls, and grids** extend the collection system + (`useListModel`, `useGridModel`, `useOverflowListModel`, and item hooks from + `@workday/canvas-kit-react/collection`). Do not hand-roll selection, roving tabindex, or list + keyboard handling. See + [Collection.mdx](modules/react/collection/stories/mdx/Collection.mdx) and + [`/sana-canvas-kit-contribute-component`](skills/sana-canvas-kit-contribute-component/SKILL.md). Full reference: [CREATING_COMPOUND_COMPONENTS.mdx](modules/docs/mdx/CREATING_COMPOUND_COMPONENTS.mdx), [API_PATTERN_GUIDELINES.mdx](modules/docs/mdx/API_PATTERN_GUIDELINES.mdx). @@ -190,16 +196,17 @@ When implementing or changing a component: Check whether Tab/Shift+Tab is sufficient or whether the APG pattern calls for additional keys (arrow keys for a listbox/menu/tablist, Escape to dismiss, Enter/Space to activate). Don't ship mouse-only interaction. -- **Focus management** — visible focus indicators are required by default. Only suppress the - focus ring for mouse/touch/pointer input specifically, using the existing `data-whatinput` - pattern, never by removing `outline` unconditionally: +- **Focus management** — visible focus indicators are required by default. Use **`:focus-visible`** + so browser heuristics show the ring for keyboard (and other non-pointer) focus, and pair it with + a `&.focus` class twin for Chromatic/`StaticStates`. Never remove `outline` on `:focus` without a + `:focus-visible` replacement. Do not use `data-whatinput` or `InputProvider` (removed in v14). + See [`/sana-canvas-kit-accessibility`](skills/sana-canvas-kit-accessibility/SKILL.md) and the + `MenuItem` pattern: ```tsx - [`[data-whatinput='mouse'] &:focus, - [data-whatinput='touch'] &:focus, - [data-whatinput='pointer'] &:focus`]: { - outline: 'none', - border: 'none', + '&:is(.focus, :focus-visible)': { + outline: `2px solid ${system.legacy.color.brand.border.primary}`, + outlineOffset: '-2px', }, ``` - **Popups, portals, and live regions** have their own established patterns — check @@ -222,7 +229,9 @@ and the rest of [modules/docs/mdx/accessibility](modules/docs/mdx/accessibility) See [STYLE.md](./STYLE.md) for the full reference. The headline rules: - **Use `createStyles` / `createStencil` from `@workday/canvas-kit-styling`.** Define them at - module scope, never inside a render function. + module scope, never inside a render function. `createStencil` already injects + `box-sizing: border-box` — do not repeat `boxSizing: 'border-box'` in a stencil's `base`. + `createStyles` does **not** get that automatically. - **Don't use `Box`, `Flex`, `Grid`, `Stack`, `HStack`, or `VStack` in new code**, and don't use style props (`padding="s"`, `depth={1}`, `backgroundColor="frenchVanilla100"`, etc.) or Emotion's `styled()`. `Stack`/`HStack`/`VStack` were removed in v9; style props and `styled()` @@ -347,21 +356,24 @@ testing complete. - Call out the areas you want reviewer focus on ("Where Should the Reviewer Start?", "Areas for Feedback?" in the template) instead of leaving them blank. -## Agent skills (consumer apps) +## Agent skills -This file is the **maintainer** guide for working on Canvas Kit itself. The [`skills/`](./skills/) -directory is a separate, **consumer-facing** skill pack for apps that *use* `@workday/canvas-kit-react` -— tokens, styling, accessibility, component selection, migration, and similar tasks in downstream -codebases. +This file is the **maintainer** guide for working on Canvas Kit itself. Most skills under +[`skills/`](./skills/) are **consumer-facing** (apps that *use* `@workday/canvas-kit-react` — +tokens, styling, accessibility, component selection, migration). For adding a component **to this +repo**, use +[`/sana-canvas-kit-contribute-component`](skills/sana-canvas-kit-contribute-component/SKILL.md) +(maintainer-only). It builds on the consumer factory/styling/a11y skills plus +[STYLE.md](./STYLE.md) (`system.legacy.*`, Chromatic class twins, publishing). **Do not confuse the two.** Consumer skills teach `system.*` tokens and avoiding deprecated exports -in app code. Maintainer work in `modules/**/lib/**` follows [STYLE.md](./STYLE.md) (`system.legacy.*`, -Chromatic class twins, publishing). A maintainer-focused skill pack may come later; it does not -exist yet. +in app code. Maintainer work in `modules/**/lib/**` follows [STYLE.md](./STYLE.md) and the +contribute skill. | Resource | Audience | Use when | | -------- | -------- | -------- | | [`skills/README.md`](./skills/README.md) | Consumers + maintainers editing skills | Skill index, dependencies, scope | +| [`sana-canvas-kit-contribute-component`](skills/sana-canvas-kit-contribute-component/SKILL.md) | Maintainers | New component in `modules/react` (or preview/labs) from issue + Figma | | [Storybook → Agent Skills](modules/docs/mdx/agent-skills/Overview.mdx) | Humans browsing docs | Overview with links to each `SKILL.md` on GitHub | | [Storybook → AI For LLMs → MCP Docs](modules/mcp/stories/mdx/MCPDocs.mdx) | Consumers using MCP | Runtime upgrade guides, tokens, accessibility, component examples | @@ -391,8 +403,9 @@ Storybook stays in sync. Do not duplicate the full skill catalog here — link o - Import the package barrel or anything under `/lib/`. - Commit Cursor or Claude plan files, debug sessions, or other local AI session artifacts (`.cursor/`, `.claude/`, etc.). -- Add ARIA attributes speculatively, or suppress focus outlines unconditionally instead of - scoping to `[data-whatinput='mouse'|'touch'|'pointer']`. +- Add ARIA attributes speculatively, or suppress focus outlines with `data-whatinput` / + unconditional `outline: none` on `:focus` — use `:focus-visible` (plus `&.focus` for static + states) instead. - Modify `tsconfig*.json`, build config, or CI workflows without explaining why first. - Add DOM snapshot tests. - Ship a breaking change without an upgrade-guide entry. diff --git a/modules/docs/mdx/API_PATTERN_GUIDELINES.mdx b/modules/docs/mdx/API_PATTERN_GUIDELINES.mdx index 93aa9f3dc3..9a71d01cf6 100644 --- a/modules/docs/mdx/API_PATTERN_GUIDELINES.mdx +++ b/modules/docs/mdx/API_PATTERN_GUIDELINES.mdx @@ -17,7 +17,7 @@ Some of the below rules are inspired by painpoints we've encountered in this pro - [Patterns](#patterns) - [Event Handlers](#event-handlers-1) - [Grow Interface](#grow-interface) - - [Input Provider](#input-provider) + - [Focus indicators](#focus-indicators) - [Prop Spread Behavior](#prop-spread-behavior) - [Controlled Components](#controlled-components) - [Accessibility](#accessibility) @@ -139,22 +139,18 @@ If objects are desired, the keys follow these rules: - Then use the `grow` boolean prop in your styles to achieve the desired effect (e.g. `width: grow ? '100%' : undefined`) -#### Input Provider +#### Focus indicators -- All Canvas Kit components should support an `InputProvider` component to provide the cleanest - experience for mouse users. Read the docs - [here](https://github.com/Workday/canvas-kit/tree/master/modules/core/react#input-provider). -- Do not use `InputProvider` within your components. It is meant to be used only once in your - application. It does not require wrapping any children -- Make sure you provide fully accessible styling by default, and only override for mouse usage. -- +- Visible focus indicators are required by default. Use **`:focus-visible`** so browser heuristics + show the ring for keyboard (and other non-pointer) focus. Pair it with a `&.focus` class twin for + Chromatic / `StaticStates` visual testing. +- Do **not** use `data-whatinput` or `InputProvider` — those were removed in v14. Never remove + `outline` on `:focus` without a `:focus-visible` replacement. ```tsx -[`[data-whatinput='mouse'] &:focus, - [data-whatinput='touch'] &:focus, - [data-whatinput='pointer'] &:focus`]: { - outline: 'none', - border: 'none', +'&:is(.focus, :focus-visible)': { + outline: `2px solid ${system.legacy.color.brand.border.primary}`, + outlineOffset: '-2px', }, ``` diff --git a/skills/README.md b/skills/README.md index d29cee39fa..62a191691e 100644 --- a/skills/README.md +++ b/skills/README.md @@ -9,10 +9,10 @@ design system correctly. Written against the **current generation**: Canvas Kit `@workday/canvas-tokens-web` **4.4** (Sana Canvas theme). `/sana-canvas-kit-version` detects when an install is older and hands off to the upgrade path instead of applying current-gen tables as truth. -This is **not** the maintainer skill set for working on the `canvas-kit` repo's own source -(`system.legacy.*` in `modules/**/lib/**`, `cornerShapeStencil`, Chromatic class twins, publishing -concerns — see [STYLE.md](../STYLE.md) / [AGENTS.md](../AGENTS.md)). A maintainer-focused skill -pack is a possible future addition; don't assume it exists yet. +Most skills below are consumer-facing. For contributing a component **into the canvas-kit repo** +itself (`modules/**/lib/**`, `system.legacy.*`, Chromatic class twins, publishing), use +[`sana-canvas-kit-contribute-component`](sana-canvas-kit-contribute-component/SKILL.md) plus +[STYLE.md](../STYLE.md) / [AGENTS.md](../AGENTS.md). ## Skills @@ -28,6 +28,7 @@ pack is a possible future addition; don't assume it exists yet. | [`sana-canvas-kit-migration`](sana-canvas-kit-migration/SKILL.md) | Running `@workday/canvas-kit-codemod` across majors; upgrade-guide checklist | version | | [`sana-canvas-kit-builder`](sana-canvas-kit-builder/SKILL.md) | Building custom components with `createComponent`/`createContainer`/`createModelHook`/etc. | styling, tokens, a11y, design-principles | | [`sana-canvas-kit-review`](sana-canvas-kit-review/SKILL.md) | Report-only grep/checklist review of a diff/branch for design-system compliance | version, all of the above | +| [`sana-canvas-kit-contribute-component`](sana-canvas-kit-contribute-component/SKILL.md) | **Maintainer-only:** add a component to this repo from a GitHub issue + Figma (collection models, Sana stencils, stories, tests, PR) | builder, styling, tokens, a11y | ## How they fit together diff --git a/skills/sana-canvas-kit-builder/SKILL.md b/skills/sana-canvas-kit-builder/SKILL.md index 6d59f6ef00..844ab7ae50 100644 --- a/skills/sana-canvas-kit-builder/SKILL.md +++ b/skills/sana-canvas-kit-builder/SKILL.md @@ -200,6 +200,21 @@ export const useDisclosureModel = createModelHook({ }); ``` +## Lists and grids — use the collection system + +If the custom component is a **list, grid, or set of items** with selection, roving focus, or +arrow-key navigation, extend `@workday/canvas-kit-react/collection` instead of hand-rolling +selection/`useState` for the item set: + +- Models: `useListModel`, `useGridModel`, `useOverflowListModel` +- Item hooks: `useListItemRegister`, `useListItemSelect`, `useListItemRovingFocus` (compose via + `composeHooks`; register last unless overriding a prop register sets) +- Selection managers: `singleSelectionManager`, `multiSelectionManager` + +Keep a plain `createModelHook` (like the disclosure example above) for non-collection state +(visibility, ids, form flags). For contributing a collection-based component into the canvas-kit +repo, see `/sana-canvas-kit-contribute-component`. + ## Accessibility Wire a11y in `createElemPropsHook` (e.g. `aria-expanded`, `aria-controls`, `id` from @@ -210,7 +225,7 @@ Wire a11y in `createElemPropsHook` (e.g. `aria-expanded`, `aria-controls`, `id` ```text - [ ] Pick factory (createComponent vs createContainer + model) -- [ ] Define model with createModelHook if behavior is shared +- [ ] List/grid/selection/roving focus? → extend collection models; else createModelHook - [ ] Extract elemProps into createElemPropsHook (module scope) - [ ] Style with createStyles/createStencil + cs (/sana-canvas-kit-styling) - [ ] Use system tokens (/sana-canvas-kit-tokens) @@ -236,6 +251,7 @@ This is encouraged when the app knows its context (translations, test ids, analy ## Anti-patterns - ❌ `createModelHook` for a static styled div +- ❌ Hand-rolled selection / roving tabindex for a list or grid — use collection - ❌ `createElemPropsHook` inside render - ❌ Manual `React.Context` when `createContainer`/`createSubcomponent` handle it - ❌ `forwardRef` + Context by hand — use the factories @@ -250,6 +266,7 @@ This is encouraged when the app knows its context (translations, test ids, analy ``` No shared state, just markup/styles? → createComponent Root provides state to children? → createContainer + createModelHook +List/grid with selection or roving focus?→ extend useListModel / collection item hooks Child reads model from context? → createSubcomponent Reusable ARIA/event props for an element?→ createElemPropsHook Combining multiple elemProps hooks? → composeHooks (module scope) diff --git a/skills/sana-canvas-kit-contribute-component/SKILL.md b/skills/sana-canvas-kit-contribute-component/SKILL.md index ee78a62a35..827355261b 100644 --- a/skills/sana-canvas-kit-contribute-component/SKILL.md +++ b/skills/sana-canvas-kit-contribute-component/SKILL.md @@ -35,8 +35,8 @@ Copy this checklist and track progress: - [ ] 3. Study the closest sibling components; grep for existing helpers before inventing - [ ] 4. Propose a plan to the user (API, a11y roles, styling, open design/a11y questions) — wait - [ ] 5. Scaffold the package (see file layout in references.md) -- [ ] 6. Model: extend collection models; don't hand-roll selection, guards, or callbacks -- [ ] 7. Subcomponents: compose collection elemProps hooks in the right order +- [ ] 6. Model: if list/grid/selection/roving focus — extend collection models; else createModelHook +- [ ] 7. Subcomponents: for collection items, compose collection elemProps hooks in the right order - [ ] 8. Style with Sana-aligned stencils (system.legacy.*, cornerShapeStencil, class twins) - [ ] 9. Stories, examples, MDX, visual-testing stories - [ ] 10. Tests: unit (verifyComponent), SSR, Cypress with checkA11y @@ -95,9 +95,15 @@ design/a11y questions. Wait for the user before editing. Use the file layout and model/hook rules in [references.md](references.md#file-layout). Key rules: -- Extend `useListModel`; pick `singleSelectionManager` / `multiSelectionManager` via config. - `createModelHook` already provides `should` / `on` — don't reimplement them. -- `composeHooks` runs right-to-left; `useListItemRegister` goes **last**. Compose +- **Collection decision:** if the component is a list, grid, or set of items with selection, + roving focus, or arrow-key navigation (`SegmentedControl`, `Tabs`, `Menu`), extend collection + models — see [references.md](references.md#model-and-hook-rules). Skip collection for a single + control with no item set (button, banner, text field); use `createModelHook` alone. +- When using collection: extend `useListModel` (or `useGridModel` / `useOverflowListModel`); pick + `singleSelectionManager` / `multiSelectionManager` via config. `createModelHook` already + provides `should` / `on` — don't reimplement them. +- `composeHooks` runs right-to-left; `useListItemRegister` goes **last** (except when a later + hook must override a prop register sets — see `useSegmentedControlItem`). Compose `useListItemRovingFocus` / `useListItemSelect` directly — never call them inside another `createElemPropsHook` body. - Add `index.ts` exports and `export * from './'` in `modules/react/index.ts`. diff --git a/skills/sana-canvas-kit-contribute-component/references.md b/skills/sana-canvas-kit-contribute-component/references.md index 4a250f22d9..c368df8ba9 100644 --- a/skills/sana-canvas-kit-contribute-component/references.md +++ b/skills/sana-canvas-kit-contribute-component/references.md @@ -4,6 +4,9 @@ Detail for [SKILL.md](SKILL.md). Worked example throughout: `SelectionGroup` (is ## File layout +**Collection case** (list/grid with `List` + `Item` subcomponents). Non-collection components omit +`List`/`Item` and the collection-shaped hooks. + ```text modules/react// ├── index.ts # export * from each lib file (no default exports) @@ -35,6 +38,21 @@ No per-component `package.json` or tsconfig path entries were needed for `module ## Model and hook rules +### When to use the collection system + +| Use collection | Skip collection | +| -------------- | --------------- | +| List, grid, or set of items with selection, roving focus, or arrow-key navigation | Single control with no item set (button, banner, text field) | +| Examples: `SegmentedControl`, `Tabs`, `Menu`, `Select` | Examples: `PrimaryButton`, `Banner`, form inputs | + +Models from `@workday/canvas-kit-react/collection`: + +- `useListModel` — cursor + selection (most lists) +- `useGridModel` — 2D navigation +- `useOverflowListModel` — overflow/measure patterns + +### Collection model + item hooks + ```tsx export const useThingModel = createModelHook({ defaultConfig: { @@ -60,11 +78,16 @@ export const useThingItem = composeHooks( role: model.state.mode === 'single' ? 'radio' : 'checkbox', 'aria-checked': isSelected(elemProps['data-id'] || '', model.state), })), - useListItemRovingFocus, - useListItemRegister // always last + useListItemRovingFocus, // or useListItemActiveDescendant for aria-activedescendant patterns + useListItemRegister // last, unless a later hook must override a prop register sets ); ``` +- Typical order: `useListItemSelect` → component-specific elemProps → `useListItemRovingFocus` (or + `useListItemActiveDescendant`) → `useListItemRegister`. A hook may come **after** register only + to override a prop register sets (see `useSegmentedControlItem` and `disabled`). +- Dynamic items: pass `items` and render with `useListRenderItems`. Virtualization defaults **on** + for dynamic items — pass `shouldVirtualize: false` for short lists and popup menus. - Controlled usage: accept `selectedIds` and sync with `events.setSelectedIds` in an effect; uncontrolled uses `initialSelectedIds`. - `onSelect` can fire on mount when single-select auto-selects the first item — account for it in @@ -90,8 +113,10 @@ Apply in `lib/` stencils: - [ ] Hover / pressed: `system.legacy.color.surface.overlay.hover.default` / `.pressed.default`, scoped to unselected items. - [ ] Focus: `outline: 2px solid system.legacy.color.brand.border.primary` with - `outlineOffset: -2px` (MenuItem pattern), suppressed only under - `[data-whatinput='mouse'|'touch'|'pointer']`. + `outlineOffset: -2px` under `'&:is(.focus, :focus-visible)'` (MenuItem pattern). Use + `:focus-visible` + `&.focus` — not `data-whatinput`. +- [ ] Do **not** set `boxSizing: 'border-box'` on a stencil — `createStencil` already injects it + (copying `MenuItem` will repeat it unnecessarily). - [ ] Disabled: `opacity: system.opacity.disabled`, `cursor: 'default'`, match `:disabled, .disabled, [aria-disabled="true"]`. - [ ] Every pseudo-state has a class twin: `'&:is(:hover, .hover)'`, `.active`, `.focus`, diff --git a/skills/sana-canvas-kit-review/SKILL.md b/skills/sana-canvas-kit-review/SKILL.md index b5ca31a82d..bf0698e285 100644 --- a/skills/sana-canvas-kit-review/SKILL.md +++ b/skills/sana-canvas-kit-review/SKILL.md @@ -130,6 +130,8 @@ Read the actual JSX/TSX for changed files, not just grep matches: for a 3-option yes/no choice, `Modal` for a dense table)? (`/sana-canvas-kit-design-principles`) - **One primary action** — more than one `PrimaryButton` in the same view? (`/sana-canvas-kit-design-principles`) +- **Redundant stencil `boxSizing`** — is `boxSizing: 'border-box'` set inside a `createStencil` + `base`? Stencils already inject it; flag as unnecessary (`/sana-canvas-kit-styling`) ## Report format diff --git a/skills/sana-canvas-kit-styling/SKILL.md b/skills/sana-canvas-kit-styling/SKILL.md index 28bd8d2bfe..966733cf73 100644 --- a/skills/sana-canvas-kit-styling/SKILL.md +++ b/skills/sana-canvas-kit-styling/SKILL.md @@ -107,6 +107,10 @@ const formStyles = createStyles({ Use when styles depend on props, variants, component parts, or CSS variables. +`createStencil` already injects `box-sizing: border-box` on every stencil. Do **not** set +`boxSizing: 'border-box'` in `base` (or modifiers). `createStyles` does **not** get that +automatically — set `boxSizing` there only if you need it. + ```tsx import {createStencil, cssVar, handleCsProp} from '@workday/canvas-kit-styling' import {system} from '@workday/canvas-tokens-web' @@ -415,6 +419,7 @@ wrapping) lives in `/sana-canvas-kit-tokens` — check that before re-writing th - ❌ Style props: `` - ❌ `cs` on native HTML elements — use `className` (`cs` is for Canvas Kit components) - ❌ `createStyles` / `createStencil` inside render +- ❌ `boxSizing: 'border-box'` inside a `createStencil` `base` — stencils already include it - ❌ Inline `cs={{...}}` with 2+ properties in component code — extract to `createStyles` - ❌ `cs` arrays — use one `createStyles`/`createStencil`, or stencil modifiers - ❌ `mergeStyles` — use `handleCsProp`