diff --git a/.github/agents/modular-ds-implementer.agent.md b/.github/agents/modular-ds-implementer.agent.md new file mode 100644 index 00000000000..b73eb925f1a --- /dev/null +++ b/.github/agents/modular-ds-implementer.agent.md @@ -0,0 +1,66 @@ +--- +name: modular-ds-implementer +description: Builds Primer React components using the modular design system's spectrum of abstraction model (config, presentational, base, and utility components). +tools: + - read + - search + - edit + - execute +skills: + - modular-ds-spectrum-model + - modular-ds-config-components + - modular-ds-presentational-components + - modular-ds-base-components + - modular-ds-utilities + - modular-ds-accessibility-contract + - modular-ds-decompose-existing-component + - modular-ds-tdd-a11y-test-backfill +--- + +You are a Primer React implementer specializing in the modular design system model. Before designing or changing a component, read the `modular-ds-spectrum-model` skill and align the implementation to it — especially which of config, presentational, base, or utility components the change actually needs. + +Also read the repo's coding standards before generating any code: + +- `.github/instructions/general-coding.instructions.md` +- `.github/instructions/typescript-react.instructions.md` +- `.github/instructions/css.instructions.md` + +## Two workflows + +- **Building a new component or component area.** Start with presentational components and a companion behavior hook. Add a base component underneath when there's accessibility structure or interactivity that warrants one. Add a config component only once common use-cases and opinionated defaults are established — don't build one speculatively. Use `modular-ds-config-components`, `modular-ds-presentational-components`, `modular-ds-base-components`, and `modular-ds-utilities` for the rules governing each. +- **Decomposing an existing monolithic component.** Follow `modular-ds-decompose-existing-component` — this is a non-breaking refactor by default; the public API must stay identical unless the task explicitly says otherwise. + +Not every component needs every API type. Don't build a config component, a base component, and a set of utilities symmetrically "because that's the model" — each one earns its place when there's real demand for the control it exposes. When it's not obvious whether a given API type is warranted, surface the decision to the user rather than assuming an answer. + +## Core rules + +- Use base components, shared hooks, utilities, and behaviors for accessibility primitives and low-level behavior before creating custom one-off implementations. Consolidate accessibility primitives for established ARIA Authoring Practices Guide patterns instead of reimplementing them per component — see `modular-ds-accessibility-contract` for how responsibility is split across API types. +- Keep behavior hooks internal unless a public hook is explicitly requested or clearly justified by consumer needs (`modular-ds-utilities`). +- Ensure config components compose presentational components and hooks rather than duplicating behavior (`modular-ds-config-components`). +- Keep markup and accessibility semantics flexible: preserve native semantics, including heading structure, and expose presentational pieces or slots when consumers need control over content, appearance, or semantics. +- Search for existing Primer components, hooks, utilities, and accessibility primitives before adding new ones. +- Do not expose `data-component` as a customizable prop at any API type — Primer owns `data-component` values as component identifiers. +- Avoid inventing visual styling without a concrete design reference, image, or specification. If styling isn't specified, keep styles minimal and structural so the component API and accessibility model can be evaluated independently. +- Prefer `HTMLElement` for default root refs and polymorphic component typing. Use narrower element types only when the API or behavior requires a specific element. +- Include the surfaces needed for adoption: source exports, tests, stories, docs metadata, and changesets when published package behavior changes. Use `modular-ds-tdd-a11y-test-backfill` for what to test at each API type, especially when decomposing an existing component. + +## Entry points + +Target entry points for each API type — check `packages/react/package.json`'s `exports` field first, since not all of these subpaths exist yet. Only `.` and `./experimental` are currently exported; `foundations` and `hooks` subpaths must be added when a component first needs them, not assumed to already be shipped. + +| API type | Experimental import | Stable import | +| -------------- | ---------------------------------------- | --------------------------- | +| Config | `@primer/react/experimental` | `@primer/react` | +| Presentational | `@primer/react/experimental` | `@primer/react` | +| Base | `@primer/react/foundations/experimental` | `@primer/react/foundations` | +| Utilities | `@primer/react/hooks/experimental` | `@primer/react/hooks` | + +`@primer/react` does not re-export base components or utilities — each is opt-in via its own entry point once it exists. All API types ship in one package version; stability is per-component (e.g. a hook can graduate to stable while its base component remains experimental). + +Create or update `index.ts` files to re-export the public API for each API type touched, and update the relevant experimental barrel files. Add or update package exports in `packages/react/package.json` only if the subpath doesn't already exist. + +## Validation + +Follow the validation order in `modular-ds-tdd-a11y-test-backfill` and fix any failures before reporting completion. + +When proposing or implementing work, explain which API type changed, why that level of abstraction is appropriate, and how the implementation can be extended without forking or overriding Primer internals. diff --git a/.github/agents/modular-ds-reviewer.agent.md b/.github/agents/modular-ds-reviewer.agent.md new file mode 100644 index 00000000000..c4ec4c86221 --- /dev/null +++ b/.github/agents/modular-ds-reviewer.agent.md @@ -0,0 +1,39 @@ +--- +name: modular-ds-reviewer +description: Reviews Primer React component changes for alignment with the modular design system's spectrum of abstraction model (config, presentational, base, and utility components). +tools: + - read + - search + - execute +skills: + - modular-ds-spectrum-model + - modular-ds-config-components + - modular-ds-presentational-components + - modular-ds-base-components + - modular-ds-utilities + - modular-ds-accessibility-contract + - modular-ds-decompose-existing-component + - modular-ds-tdd-a11y-test-backfill +--- + +You are a read-only reviewer for the modular design system model. Review the current changes and determine whether they follow the patterns in `modular-ds-spectrum-model` and its related skills. Never modify files. + +Check for: + +- Whether the change starts with presentational components and behavior hooks before introducing a config-driven API, and whether a config component (if present) is justified by an established, stable pattern rather than added speculatively (`modular-ds-config-components`, `modular-ds-presentational-components`). +- Whether behavior hooks are kept internal unless a public hook is explicitly requested or clearly justified by consumer needs (`modular-ds-utilities`). +- Whether config components compose presentational components and hooks instead of duplicating state, behavior, or markup. +- Whether base components or existing utilities should be reused for accessibility primitives, state, refs, timers, escape handling, or similar cross-component behavior, instead of one-off reimplementation (`modular-ds-base-components`, `modular-ds-utilities`). +- Whether accessibility primitives for established patterns, such as ARIA Authoring Practices Guide patterns, are consolidated instead of reimplemented across components. +- Whether established ARIA Authoring Practices Guide patterns use the expected pattern semantics and structure instead of defaulting to native elements that produce a different component model. +- Whether the accessibility responsibility for each requirement sits at the right API type, and whether consumer-facing responsibilities that aren't automatic are clearly documented (`modular-ds-accessibility-contract`). +- Whether consumers can customize appearance, content, semantics, and behavior without forking Primer or relying on overrides. +- Whether native accessibility semantics are preserved and low-level accessibility behavior is reusable, including heading-first structures when a heading labels an interactive control. +- Whether public hooks are justified by a clear consumer need instead of exposing sub-component internals by default, and whether any public hook has docs metadata and tests matching its API. +- Whether `data-component` values are owned by Primer and not exposed as customizable public props, at any API type. +- Whether visual styling is supported by a concrete design reference, image, or specification instead of being invented by the implementation. +- Whether root refs and element types default to `HTMLElement` unless a narrower element type is required. +- Whether exports, stories, tests, docs metadata, and changesets match the public API impact of the change. +- If the change decomposes an existing component, whether the public API stayed identical and whether accessibility behavior that was previously implicit or untested has adequate test backfill (`modular-ds-decompose-existing-component`, `modular-ds-tdd-a11y-test-backfill`). + +Report only actionable findings that show a concrete mismatch with the spectrum of abstraction model. For each finding, cite the file and line, name the relevant API type or principle, explain the impact, and suggest the smallest design-level correction. If the change follows the model, say so directly. diff --git a/.github/skills/modular-ds-accessibility-contract/SKILL.md b/.github/skills/modular-ds-accessibility-contract/SKILL.md new file mode 100644 index 00000000000..66392049495 --- /dev/null +++ b/.github/skills/modular-ds-accessibility-contract/SKILL.md @@ -0,0 +1,82 @@ +--- +name: modular-ds-accessibility-contract +description: 'Use when: determining which Primer React API type (config, presentational, base, or utility) should own a specific accessibility responsibility, or documenting that ownership for a new component. Covers a responsibility matrix mapping ARIA requirements to API types, worked examples for Dialog and Tabs, and how to build a matrix for a new ARIA pattern.' +--- + +# Modular DS — Accessibility Contract + +Each API type in the spectrum of abstraction model shifts accessibility responsibility to the consumer differently. This matrix defines what each API type handles automatically and what the consumer must provide themselves. + +## Responsibility matrix — Dialog example + +| Requirement | Utility | Base component | Presentational | Config | +| -------------------------------------- | -------------------- | --------------------------------- | ----------------- | ----------------------- | +| `role="dialog"` / `role="alertdialog"` | Consumer sets | ✅ Automatic | ✅ Inherited | ✅ Inherited | +| `aria-modal="true"` | Consumer sets | ✅ Automatic | ✅ Inherited | ✅ Inherited | +| `aria-labelledby` → title | Consumer wires | ✅ Auto-wired via context | ✅ Inherited | ✅ From `title` prop | +| `aria-describedby` → description | Consumer wires | ✅ Auto-wired if Description used | ✅ Inherited | ✅ From `subtitle` prop | +| Focus trapping | Consumer implements | ✅ Native `showModal()` | ✅ Inherited | ✅ Inherited | +| Escape closes | Consumer handles | ✅ Automatic | ✅ Inherited | ✅ Inherited | +| Focus moves into component | Consumer manages | ✅ Automatic | ✅ Inherited | ✅ Inherited | +| Focus returns on close | Consumer manages | ✅ Automatic | ✅ Inherited | ✅ Inherited | +| Visible close control | Consumer provides | ✅ Enforced by structure | ✅ Built-in | ✅ Built-in | +| Background inert | Consumer manages | ✅ Native `showModal()` | ✅ Inherited | ✅ Inherited | +| Scroll lock | `useScrollLock` hook | ✅ Automatic | ✅ Inherited | ✅ Inherited | +| Visible backdrop | Consumer provides | ⚠️ Consumer must style | ✅ Primer token | ✅ Primer token | +| Appropriate heading level | Consumer chooses | ⚠️ Consumer must choose | ✅ `

` default | ✅ `

` default | +| Colour contrast | Consumer responsible | ⚠️ Consumer must ensure | ✅ Primer tokens | ✅ Primer tokens | + +## Key principles + +### Base component consumer responsibilities + +At the base component level, a foundation like a dialog ships a transparent backdrop by default. Per ARIA APG, `aria-modal="true"` should only be set when background content is **both** non-interactive and visually obscured — so consumers using a base component directly **must** provide visible backdrop styling to meet this requirement. Presentational components handle this automatically with Primer tokens. + +### `aria-describedby` guidance + +Per ARIA APG, omit `aria-describedby` when content has complex semantic structure (lists, tables, multiple paragraphs) — screen readers announce it as a flat string. At the base-component level or above, don't render a Description part if content is complex. At the utility/hook level, don't call `getDescriptionProps()`. + +### Initial focus guidance + +For components with complex semantic content, set an initial-focus ref to a static element at the top with `tabIndex={-1}` so assistive technology users can navigate the structure. For destructive actions, focus the least destructive control. + +### Dev-mode warnings + +A component's compound hook (see `modular-ds-utilities`) should fire a dev-mode warning when no accessible name is provided (neither a title part used, nor `aria-label` passed) or when required structural elements are missing. Check after render, once prop-getters have been called: + +```tsx +useEffect(() => { + if (process.env.NODE_ENV !== 'production' && open) { + queueMicrotask(() => { + if (!titleUsed.current && !ariaLabel) { + console.warn( + ': No accessible name provided. Use getTitleProps() on a title element, or pass aria-label.', + ) + } + }) + } +}, [open, ariaLabel]) +``` + +## Applying this to other ARIA patterns + +When building a component with a different ARIA pattern (tabs, menu, listbox, etc.), build a similar responsibility matrix: + +1. **Identify the ARIA APG pattern** — read the W3C ARIA Authoring Practices Guide for the relevant pattern. +2. **List all requirements** — roles, states, properties, keyboard interaction, focus management. +3. **Assign to API types**, following the same principle: utility/consumer does everything by default; base component automates ARIA wiring, focus management, and keyboard interaction; presentational inherits the base component and adds Primer-token styling; config inherits presentational and maps props to its children. +4. **Mark consumer responsibilities** (⚠️) — anything the base component does NOT handle automatically must be documented clearly. + +### Tabs example (skeleton) + +| Requirement | Utility | Base component | Presentational | Config | +| ------------------------------- | ------------------- | ------------------------- | ---------------- | ---------------- | +| `role="tablist"` | Consumer sets | ✅ Automatic | ✅ Inherited | ✅ Inherited | +| `role="tab"` on each tab | Consumer sets | ✅ Automatic | ✅ Inherited | ✅ Inherited | +| `role="tabpanel"` on each panel | Consumer sets | ✅ Automatic | ✅ Inherited | ✅ Inherited | +| `aria-selected` on active tab | Consumer manages | ✅ Automatic | ✅ Inherited | ✅ Inherited | +| `aria-controls` tab → panel | Consumer wires | ✅ Auto-wired via context | ✅ Inherited | ✅ Inherited | +| Arrow key navigation | `useFocusZone` hook | ✅ Automatic | ✅ Inherited | ✅ Inherited | +| Home/End to first/last tab | Consumer handles | ✅ Automatic | ✅ Inherited | ✅ Inherited | +| Hidden panels (`hidden` attr) | Consumer manages | ✅ Automatic | ✅ Inherited | ✅ Inherited | +| Focus indicator styling | Consumer styles | ⚠️ Consumer must style | ✅ Primer tokens | ✅ Primer tokens | diff --git a/.github/skills/modular-ds-base-components/SKILL.md b/.github/skills/modular-ds-base-components/SKILL.md new file mode 100644 index 00000000000..390cf434332 --- /dev/null +++ b/.github/skills/modular-ds-base-components/SKILL.md @@ -0,0 +1,52 @@ +--- +name: modular-ds-base-components +description: 'Use when: building or evaluating unstyled Primer React primitives, especially accessibility primitives or low-level behavior that other components should build on. Covers when to create a base component instead of baking behavior into a styled component, the unstyled CSS-reset convention, deciding which parts need a base equivalent, and consolidating ARIA Authoring Practices Guide patterns.' +--- + +# Modular DS — Base Components + +Base components are unstyled primitives used to build higher-level components. They carry no visual styling and enforce structural accessibility constraints, similar in spirit to [Base UI](https://base-ui.com/) or [Radix Primitives](https://www.radix-ui.com/primitives). + +```tsx +function Example() { + return ( + + + + + + + + ) +} +``` + +Other examples of things suited to base components: Combobox (filtering, selection), Listbox (selection), Popover, Tabs, Treeview. + +Shown above with dot-notation for readability — ship flat named exports per the RSC-safe convention in `modular-ds-presentational-components`, not an `Object.assign` composed export. + +## When to use a base component + +Use base components for accessibility primitives and low-level behaviors that need full markup and style control. Before adding custom behavior to a component, look for an existing base component, hook, or utility that can already provide the foundation — don't reimplement it. + +Accessibility primitives for established patterns (e.g. ARIA Authoring Practices Guide patterns) should be **consolidated and reused** rather than reimplemented across components. If you find yourself re-solving a pattern that already has a base component elsewhere in the repo, use it instead of writing a parallel implementation. + +## What a base component covers + +- Structural accessibility constraints (e.g. a title must be a descendant of a dialog). +- ARIA wiring via internal context, so consumers get correct semantics "for free" while retaining full markup control. +- No visual opinion at all — presentational components (see `modular-ds-presentational-components`) build on top and add Primer's design tokens and layout. + +## Deciding which parts need a base equivalent + +Not every presentational sub-part needs a base component. A base primitive earns its place when there's accessibility behavior or interactivity tied to it (e.g. a dialog root, a close control, an overlay). Purely structural parts — a label, a heading, a message wrapper — usually don't need one, since consumers can render their own markup and the surrounding base components continue to wire ARIA correctly via context. + +When it's not obvious whether a given part warrants its own base component, surface the decision explicitly rather than assuming. + +## CSS reset convention + +Where a base component needs to remove browser defaults (e.g. native ``/`