diff --git a/.changeset/blankslate-accessible-visuals.md b/.changeset/blankslate-accessible-visuals.md new file mode 100644 index 00000000000..2995c2b612e --- /dev/null +++ b/.changeset/blankslate-accessible-visuals.md @@ -0,0 +1,5 @@ +--- +'@primer/react': patch +--- + +Blankslate: Treat visuals as decorative, allow standalone primary actions, and render primary actions with defined `href` values as links. diff --git a/.changeset/strict-banner-title.md b/.changeset/strict-banner-title.md new file mode 100644 index 00000000000..034fbc97034 --- /dev/null +++ b/.changeset/strict-banner-title.md @@ -0,0 +1,5 @@ +--- +'@primer/react': patch +--- + +Banner: Report a development error when more than one title is provided diff --git a/.github/agents/component-spec-reviewer.agent.md b/.github/agents/component-spec-reviewer.agent.md new file mode 100644 index 00000000000..6a7726c19a5 --- /dev/null +++ b/.github/agents/component-spec-reviewer.agent.md @@ -0,0 +1,139 @@ +--- +name: component-spec-reviewer +description: Reviews Primer React components for adherence to their component specs and identifies where specs and component behavior are out of sync. When spec files themselves are changed, challenges every modification and requires the author to justify why the change is necessary. +tools: + - read + - search + - execute +skills: + - component-spec +--- + +You are a read-only reviewer for Primer React component specs. Review the +requested component or the components affected by the current changes. Never +modify files. + +## When spec files are changed + +When a change modifies a `SPEC.md`, `spec/README.md`, or any feature file in a +`spec/` folder, treat every spec modification as suspect until proven necessary. +Your job is to make the author prove that each change is justified. + +For every added, removed, or modified requirement, ask the following questions +and raise a blocking concern if any answer is unsatisfactory: + +1. **What observable component behavior or implementation change necessitates + this spec change?** A spec change without a corresponding implementation + change, test change, or newly discovered gap requires explicit explanation. + If the author has not provided one, request it. + +2. **Does this change narrow, loosen, or remove an existing contract?** + Loosening a `MUST` to a `SHOULD`, removing a requirement, or narrowing its + scope weakens the consumer contract. Require the author to explain the + consumer impact and confirm that consumers relying on the old contract have + been considered. + +3. **Is the new or changed requirement verifiable?** A requirement that cannot + be demonstrated by a test, story, or inspection of the implementation is + unenforceable. Reject vague, aspirational, or unverifiable additions unless + the author commits to adding coverage. + +4. **Does the change introduce scope creep?** If the change adds requirements + beyond what the accompanying implementation delivers, flag it. Spec + requirements must describe what the component currently does, not what it + might do in the future. + +5. **Is the Log section updated?** Every notable spec change must add an entry + to the `## Log` section with the date, author, and a one-sentence + justification. If the change is notable (adds, removes, or significantly + revises a requirement) and the Log was not updated, raise a blocking concern. + +Reject spec changes that: + +- Weaken requirements (downgrades, removals, or scope reductions) without + evidence that the original requirement was incorrect or the component behavior + has changed. +- Add new requirements that are not implemented, not testable, or not + accompanied by coverage. +- Reword requirements in ways that change meaning without explaining why the + original wording was wrong. +- Omit a Log entry for a notable change. + +Be skeptical. The default position is that a spec change is unnecessary. The +author must overcome that presumption with evidence. + +## Alignment review + +When a change touches component implementation, tests, stories, or docs +metadata (with or without spec changes), also perform the alignment review +below. + +Start by locating and reading the component's local `SPEC.md` or +`spec/README.md` and all linked feature files. Then inspect the relevant +implementation, styles, tests, Storybook stories, `*.docs.json` metadata, and +public exports needed to evaluate the documented contract. + +Review the component and its spec in both directions: + +- Verify that the implementation satisfies each applicable `MUST`, `MUST NOT`, + `SHOULD`, `SHOULD NOT`, and `MAY` requirement. +- Identify implemented consumer-facing markup, behavior, public API + relationships, or accessibility requirements that are missing from the spec. +- Identify stale spec requirements that no longer match the implementation or + supported public API. +- Verify that semantic markup, roles, attributes, focus behavior, keyboard + behavior, callbacks, state, responsive behavior, and accessibility + relationships match the relevant feature contract. +- Verify that public API relationships, precedence rules, controlled and + uncontrolled behavior, event ordering, ref targets, and prop forwarding match + the spec. +- Compare individual prop types, defaults, descriptions, deprecations, exports, + and subcomponent inventories against `*.docs.json` rather than expecting the + spec to duplicate them. +- Verify that tests and Storybook stories link directly to the relevant stable + spec heading and that the linked coverage demonstrates the documented + behavior. +- Verify that feature headings and links resolve, remain unique, and describe + consumer-facing features rather than implementation details. + +Apply these review principles: + +- Treat the spec as the intended contract, but do not assume it is correct when + the implementation, docs metadata, established component behavior, or + accessibility requirements provide evidence that it is stale. State whether + the component or the spec should change and explain why. +- Distinguish component responsibilities from consumer responsibilities. Do not + report a component defect for a requirement explicitly assigned to consumers + unless the component prevents consumers from satisfying it or its + documentation and examples contradict it. +- Review semantic contracts, not incidental wrappers, CSS classes, generated + IDs, or other unstable implementation details unless the spec intentionally + makes them public. +- Do not require prop inventories or information representable in + `*.docs.json` to be repeated in a `Public API` section. +- Expect broad accessibility considerations at the top level and complex + interaction-specific accessibility requirements beside the relevant feature. +- Do not request requirement IDs, verification sections, property/value + matrices, or dedicated server-rendering sections. +- Do not treat a missing test or story as proof that behavior is incorrect. + Report missing coverage only when it leaves a concrete spec contract + unprotected or when an existing test or story claims coverage that it does + not provide. +- When reviewing a change, focus findings on mismatches introduced or exposed by + that change. When asked to audit a whole component, review the complete + component contract. + +## Reporting + +Report only actionable findings. For each finding: + +1. Cite the implementation, test, story, docs metadata, or export file and line. +2. Cite the relevant spec file, heading, and line. +3. State the expected contract and the observed mismatch. +4. Explain the consumer or accessibility impact. +5. Recommend the smallest correction, including whether to update the component, + the spec, or its coverage. + +Order findings by impact. Spec-change justification failures come first. If +the component adheres to its spec and every spec change is justified, say so +directly. diff --git a/.github/skills/component-spec/SKILL.md b/.github/skills/component-spec/SKILL.md new file mode 100644 index 00000000000..ccce4d01a79 --- /dev/null +++ b/.github/skills/component-spec/SKILL.md @@ -0,0 +1,220 @@ +--- +name: component-spec +description: | + Use when creating a new component; adding, changing, or removing a feature; or + evaluating a change to a component in order to make sure everything is in sync +--- + +# Component Spec + +A component spec documents the intended consumer-facing contract for a Primer +component. It covers the component's features, semantic markup, behavior, public +API relationships, and accessibility requirements. + +All components that are publicly exported from Primer must have a component +spec. Spec files must be kept up-to-date as functionality is added, updated, or +removed. Spec files must be complete and accurate because they are used to +generate documentation for Primer components. + +Before changing a component, read its local component spec. Update the spec when +a change adds, updates, or removes a feature or changes its markup, behavior, +public API contract, or accessibility requirements. + +When referencing a component spec from source, tests, or Storybook feature +stories, link to the relevant heading in the local `SPEC.md` file. + +`SPEC.md` files may be broken up for larger components. Instead of a single +file, a component may have a `spec` folder with a `README.md` file as the index +and a separate Markdown file for each feature. Link to the relevant heading or +feature file when using this format. + +## Organization + +Organize specs around consumer-facing features. Every component has at least one +feature, even if its only feature is named `Default`. + +Feature sections may include: + +- `Markup` for semantic structure and stable element or attribute relationships +- `Behavior` for state, interactions, callbacks, and other runtime requirements +- `Public API` for relationships or constraints that cannot be represented in + `*.docs.json` +- `Accessibility` for requirements specific to that feature + +Only include subsections that are relevant to the feature. + +Keep every heading unique within its Markdown file so the spec passes +`npm run lint:md`. In a single-file spec, qualify subsection headings with the +feature name, such as `#### Actions behavior` or +`#### Dismissal accessibility`. Feature files in a split spec may use generic +subsection headings because each feature has its own file. + +### Public API + +Do not repeat information that can be represented in `*.docs.json`. Prop names, +types, defaults, individual prop descriptions, deprecations, exports, and +subcomponent inventories belong in docs metadata. + +Use `Public API` sections for contracts that cannot be understood by reading an +individual prop definition, including: + +- relationships between props +- precedence or conflicts between options +- controlled and uncontrolled state requirements +- event ordering or cancellation behavior +- ref targets +- prop forwarding targets + +### Accessibility + +Use the top-level `Accessibility` section for broad considerations that apply to +the component as a whole. For a simple component, this section may contain all +of its accessibility requirements. + +For complex behavior or interactions, place accessibility requirements beside +the relevant feature or behavior so the semantic, keyboard, focus, and +announcement requirements remain in context. + +Classify motion and reduced-motion requirements as accessibility requirements. + +Distinguish defaults provided by the component from responsibilities left to +consumers. Prefer safe component defaults when the component can reliably +provide them, rather than documenting avoidable consumer work. + +### Markup + +Document the semantic contract rather than unstable implementation details. +Include elements, roles, attributes, and relationships that consumers or +assistive technologies depend on. + +Include an HTML code block in each markup section to show the rendered +structure. The code block should contain the stable semantic elements, +attributes, and relationships that form the public contract. Use comments to +explain optional or conditional parts. Omit unstable wrappers, internal +elements, and CSS class names from the code block. + +Name an exact HTML element only when that element's semantics are important, +when accessibility behavior depends on it, or when it is intentionally a stable +part of the public contract. For example, distinguishing a button from a link or +documenting a heading level is useful; naming an incidental presentational +wrapper is not. Do not require exact CSS classes, internal wrappers, or +implementation-specific DOM unless they are intentionally part of the public +contract. + +### Behavior + +Document behavior that the component actively implements or coordinates. Do not +list the absence of unrelated automatic behavior, such as stating that a +component does not announce itself or does not reorder children, unless +consumers might reasonably expect that behavior from the component pattern or +the distinction is necessary to prevent misuse. + +When a feature includes user interactions, document the keyboard keys that +trigger each action. Focus on keys the component explicitly handles or that +differ from an element's native defaults; do not repeat standard browser +defaults for native elements unless the distinction matters for consumers. + +When a feature exposes multiple states, document which states are allowed and +in what context each state is permissible. Describe the conditions that make a +state valid, note any states that cannot coexist, and clarify whether the +component or the consumer is responsible for entering and leaving each state. + +When a feature includes responsive behavior that changes what content is +visible, what interactions are available, or what is rendered at different +viewport or container sizes, document those changes. Describe what appears, +disappears, or changes form and at which sizes. Do not document presentational +changes such as padding, spacing, or font size unless they directly affect +composition or available interactions. + +### Presentation and layout + +Do not create features or requirements that merely restate visual props such as +border, size, width, or spacing, or that describe generic CSS implementation. +Include presentation or responsive behavior only when it changes composition, +visibility, interaction, semantics, or another consumer-facing contract that +callers need to understand. + +### Normative language + +Use these uppercase terms for declarative requirements: + +- `MUST` and `MUST NOT` for required or prohibited behavior +- `SHOULD` and `SHOULD NOT` for expected behavior where documented exceptions + may exist +- `MAY` for explicitly permitted behavior + +Explanatory prose does not need normative language. + +## Log + +Every spec file MUST include a `Log` section at the end. The Log records notable +decisions and significant changes made to the spec over time. + +Add a new entry to the Log whenever a change: + +- Adds, removes, or significantly revises a requirement +- Resolves a design question or chooses among competing approaches +- Documents why a requirement was deliberately excluded or limited + +Each log entry MUST include the date in `YYYY-MM-DD` format, the author, and a +one-sentence description of the decision or change. Routine housekeeping such as +typo fixes, formatting, or wording clarifications do not require log entries. + +Keep entries in reverse-chronological order. Format entries as a flat list: + +```markdown +## Log + +- **2024-01-15** (@author): Added `Actions` feature covering primary and secondary action rendering. +- **2024-01-10** (@author): Chose `
` landmark over `
` to support named region navigation. +``` + +## Keeping specs in sync + +Tests and Storybook feature stories should link directly to the spec heading +that describes the behavior or feature they cover. Only feature stories should +define `spec` parameters; Playground and other non-feature stories should not +include spec references. Do not add requirement IDs or a verification section +to the spec. + +Keep feature headings unique and stable because tests and feature stories may +link to their generated Markdown anchors. + +For example, a test may link to a feature: + +```tsx +/** + * @see ./SPEC.md#delayed-appearance + */ +it('waits before rendering', () => { + // ... +}) +``` + +A Storybook feature story may link to one or more features through parameters: + +```tsx +WithDelay.parameters = { + spec: ['./SPEC.md#delayed-appearance'], +} +``` + +Generic repository requirements, such as support for server rendering, do not +need dedicated sections in each component spec. + +Run `npm run lint:md` after adding or updating component specs. + +## Templates + +Read and use the appropriate template before authoring a component spec: + +- Use [`templates/SPEC.md`](./templates/SPEC.md) for a component whose features + fit clearly in one file. +- Use the templates in [`templates/spec`](./templates/spec) when a component + needs separate files for its features: + - [`README.md`](./templates/spec/README.md) is the spec index. + - [`default.md`](./templates/spec/default.md) contains the default feature. + - [`feature.md`](./templates/spec/feature.md) is copied and renamed for each + additional feature. + +Only keep template sections that apply to the component or feature. diff --git a/.github/skills/component-spec/templates/SPEC.md b/.github/skills/component-spec/templates/SPEC.md new file mode 100644 index 00000000000..bddb8fb382c --- /dev/null +++ b/.github/skills/component-spec/templates/SPEC.md @@ -0,0 +1,72 @@ +# {Component} spec + + + +## Accessibility + + + +## Features + +### Default + + + +#### Default markup + + + +```html + +``` + +#### Default behavior + + + +#### Default public API + + + +#### Default accessibility + + + +## Glossary + + + +## Log + + diff --git a/.github/skills/component-spec/templates/spec/README.md b/.github/skills/component-spec/templates/spec/README.md new file mode 100644 index 00000000000..272661fa698 --- /dev/null +++ b/.github/skills/component-spec/templates/spec/README.md @@ -0,0 +1,38 @@ +# {Component} spec + + + +## Accessibility + + + +## Features + +- [Default](./default.md) +- [{Feature name}](./feature.md) + + + +## Glossary + + + +## Log + + diff --git a/.github/skills/component-spec/templates/spec/default.md b/.github/skills/component-spec/templates/spec/default.md new file mode 100644 index 00000000000..fe13653b61f --- /dev/null +++ b/.github/skills/component-spec/templates/spec/default.md @@ -0,0 +1,44 @@ +# Default + + + +## Markup + + + +```html + +``` + +## Behavior + + + +## Public API + + + +## Accessibility + + diff --git a/.github/skills/component-spec/templates/spec/feature.md b/.github/skills/component-spec/templates/spec/feature.md new file mode 100644 index 00000000000..272d7eb35c1 --- /dev/null +++ b/.github/skills/component-spec/templates/spec/feature.md @@ -0,0 +1,43 @@ +# {Feature name} + + + +## Markup + + + +```html + +``` + +## Behavior + + + +## Public API + + + +## Accessibility + + diff --git a/e2e/components/Banner.test.ts b/e2e/components/Banner.test.ts index e5bfa8d35f8..999a5c5a105 100644 --- a/e2e/components/Banner.test.ts +++ b/e2e/components/Banner.test.ts @@ -4,81 +4,159 @@ import {themes} from '../test-helpers/themes' import {viewports} from '../test-helpers/viewports' const stories: Array<{title: string; id: string; viewports?: Array}> = [ + /** + * @see ../../packages/react/src/Banner/SPEC.md#default + * @see ../../packages/react/src/Banner/SPEC.md#actions + * @see ../../packages/react/src/Banner/SPEC.md#dismissal + * @see ../../packages/react/src/Banner/SPEC.md#layout + */ { title: 'Default', id: 'components-banner--default', viewports: ['primer.breakpoint.xs', 'primer.breakpoint.sm'], }, + /** + * @see ../../packages/react/src/Banner/SPEC.md#variants-and-leading-visuals + */ { title: 'Critical', id: 'components-banner-features--critical', }, + /** + * @see ../../packages/react/src/Banner/SPEC.md#dismissal + */ { title: 'Dismiss', id: 'components-banner-features--dismiss', }, + /** + * @see ../../packages/react/src/Banner/SPEC.md#actions + * @see ../../packages/react/src/Banner/SPEC.md#dismissal + */ { title: 'Dismiss With Actions', id: 'components-banner-features--dismiss-with-actions', }, + /** + * @see ../../packages/react/src/Banner/SPEC.md#variants-and-leading-visuals + * @see ../../packages/react/src/Banner/SPEC.md#dismissal + */ { title: 'Info', id: 'components-banner-features--info', }, + /** + * @see ../../packages/react/src/Banner/SPEC.md#variants-and-leading-visuals + * @see ../../packages/react/src/Banner/SPEC.md#dismissal + */ { title: 'Success', id: 'components-banner-features--success', }, + /** + * @see ../../packages/react/src/Banner/SPEC.md#variants-and-leading-visuals + * @see ../../packages/react/src/Banner/SPEC.md#dismissal + */ { title: 'Upsell', id: 'components-banner-features--upsell', }, + /** + * @see ../../packages/react/src/Banner/SPEC.md#variants-and-leading-visuals + * @see ../../packages/react/src/Banner/SPEC.md#dismissal + */ { title: 'Warning', id: 'components-banner-features--warning', }, + /** + * @see ../../packages/react/src/Banner/SPEC.md#variants-and-leading-visuals + * @see ../../packages/react/src/Banner/SPEC.md#actions + * @see ../../packages/react/src/Banner/SPEC.md#layout + */ { title: 'WithActions', id: 'components-banner-features--with-actions', viewports: ['primer.breakpoint.xs', 'primer.breakpoint.sm'], }, + /** + * @see ../../packages/react/src/Banner/SPEC.md#default + */ { title: 'WithHiddenTitle', id: 'components-banner-features--with-hidden-title', }, + /** + * @see ../../packages/react/src/Banner/SPEC.md#default + * @see ../../packages/react/src/Banner/SPEC.md#actions + * @see ../../packages/react/src/Banner/SPEC.md#layout + */ { title: 'WithHiddenTitleAndActions', id: 'components-banner-features--with-hidden-title-and-actions', viewports: ['primer.breakpoint.xs', 'primer.breakpoint.sm'], }, + /** + * @see ../../packages/react/src/Banner/SPEC.md#default + * @see ../../packages/react/src/Banner/SPEC.md#actions + * @see ../../packages/react/src/Banner/SPEC.md#dismissal + * @see ../../packages/react/src/Banner/SPEC.md#layout + */ { title: 'DismissibleWithHiddenTitleAndActions', id: 'components-banner-features--dismissible-with-hidden-title-and-actions', viewports: ['primer.breakpoint.xs', 'primer.breakpoint.sm'], }, + /** + * @see ../../packages/react/src/Banner/SPEC.md#default + * @see ../../packages/react/src/Banner/SPEC.md#actions + * @see ../../packages/react/src/Banner/SPEC.md#dismissal + * @see ../../packages/react/src/Banner/SPEC.md#layout + */ { title: 'DismissibleWithHiddenTitleAndSecondaryAction', id: 'components-banner-features--dismissible-with-hidden-title-and-secondary-action', viewports: ['primer.breakpoint.xs', 'primer.breakpoint.sm'], }, + /** + * @see ../../packages/react/src/Banner/SPEC.md#default + * @see ../../packages/react/src/Banner/SPEC.md#layout + */ { title: 'InSidebar', id: 'components-banner-examples--in-sidebar', }, + /** + * @see ../../packages/react/src/Banner/SPEC.md#actions + * @see ../../packages/react/src/Banner/SPEC.md#dismissal + * @see ../../packages/react/src/Banner/SPEC.md#layout + */ { title: 'Multiline', id: 'components-banner-examples--multiline', viewports: ['primer.breakpoint.xs', 'primer.breakpoint.sm'], }, + /** + * @see ../../packages/react/src/Banner/SPEC.md#actions + * @see ../../packages/react/src/Banner/SPEC.md#layout + */ { title: 'ActionsInline', id: 'components-banner-features--actions-layout-inline', viewports: ['primer.breakpoint.xs'], }, + /** + * @see ../../packages/react/src/Banner/SPEC.md#actions + * @see ../../packages/react/src/Banner/SPEC.md#layout + */ { title: 'ActionsStacked', id: 'components-banner-features--actions-layout-stacked', }, + /** + * @see ../../packages/react/src/Banner/SPEC.md#actions + * @see ../../packages/react/src/Banner/SPEC.md#layout + */ { title: 'FlushInsideDialog', id: 'components-banner-features--flush-inside-dialog', @@ -122,4 +200,147 @@ test.describe('Banner', () => { } }) } + + test.describe('Banner accessibility behavior', () => { + /** + * @see ../../packages/react/src/Banner/SPEC.md#accessibility + */ + test('moves focus to essential feedback after a user action @avt', async ({page}) => { + await visit(page, { + id: 'components-banner-examples--with-user-action', + }) + + await page.getByRole('button', {name: 'Update profile'}).click() + + await expect(page.getByRole('region', {name: 'Error'})).toBeFocused() + }) + + /** + * @see ../../packages/react/src/Banner/SPEC.md#accessibility + * @see ../../packages/react/src/Banner/SPEC.md#actions + */ + test('moves focus to dynamic feedback with a required action @avt', async ({page}) => { + await visit(page, { + id: 'components-banner-examples--with-required-action-after-user-action', + }) + + await page.getByRole('button', {name: 'Submit changes'}).click() + + await expect(page.getByRole('region', {name: 'Changes not saved'})).toBeFocused() + await expect(page.getByRole('button', {name: 'Review errors'})).toBeVisible() + }) + + /** + * @see ../../packages/react/src/Banner/SPEC.md#accessibility + */ + test('updates content within a persistent live region @avt', async ({page}) => { + await visit(page, { + id: 'components-banner-examples--with-announcement', + }) + + const announcement = page.getByTestId('announcement') + const initialAnnouncement = await announcement.elementHandle() + if (!initialAnnouncement) { + throw new Error('Expected the Banner announcement source to be present') + } + + await page.getByRole('radio', {name: 'Choice two'}).check() + await expect(announcement).toHaveText('This is a message for choice two') + + const updatedAnnouncement = await announcement.elementHandle() + if (!updatedAnnouncement) { + throw new Error('Expected the Banner announcement source to remain present') + } + expect(await initialAnnouncement.evaluate((node, current) => node === current, updatedAnnouncement)).toBe(true) + + await expect(page.locator('live-region')).toHaveCount(1) + await expect(page.locator('live-region #polite')).toContainText('This is a message for choice two') + }) + + /** + * @see ../../packages/react/src/Banner/SPEC.md#accessibility + * @see ../../packages/react/src/Banner/SPEC.md#dismissal + */ + test('moves focus after a dismissed Banner is removed @avt', async ({page}) => { + await visit(page, { + id: 'components-banner-examples--dismiss-banner', + }) + + await page.getByRole('button', {name: 'Dismiss banner'}).click() + + await expect(page.getByRole('heading', {name: 'Example page title'})).toBeFocused() + }) + + /** + * @see ../../packages/react/src/Banner/SPEC.md#accessibility + * @see ../../packages/react/src/Banner/SPEC.md#actions + * @see ../../packages/react/src/Banner/SPEC.md#dismissal + */ + test('provides minimum target sizes for interactive controls @avt', async ({page}) => { + await visit(page, { + id: 'components-banner--default', + }) + + const controls = page.getByRole('button') + for (let index = 0; index < (await controls.count()); index++) { + const control = controls.nth(index) + const box = await control.boundingBox() + if (!box) { + throw new Error(`Expected Banner control ${index + 1} to be visible`) + } + + expect(box.width).toBeGreaterThanOrEqual(24) + expect(box.height).toBeGreaterThanOrEqual(24) + } + }) + + /** + * @see ../../packages/react/src/Banner/SPEC.md#accessibility + * @see ../../packages/react/src/Banner/SPEC.md#layout + */ + test('reflows without horizontal scrolling at narrow widths and 200% zoom @avt', async ({page}) => { + const hasHorizontalOverflow = () => + page.evaluate(() => { + const documentWidth = Math.max(document.documentElement.scrollWidth, document.body.scrollWidth) + return documentWidth > document.documentElement.clientWidth + }) + + await page.setViewportSize({width: 320, height: 256}) + await visit(page, { + id: 'components-banner-examples--multiline', + }) + expect(await hasHorizontalOverflow()).toBe(false) + + await page.setViewportSize({width: 640, height: 512}) + await visit(page, { + id: 'components-banner-examples--multiline', + }) + await page.evaluate(() => { + document.documentElement.style.zoom = '2' + }) + expect(await hasHorizontalOverflow()).toBe(false) + }) + }) + + test.describe('Banner layout behavior', () => { + /** + * @see ../../packages/react/src/Banner/SPEC.md#layout + */ + test('uses reduced padding for the compact layout', async ({page}) => { + const getPadding = () => + page.getByRole('region').evaluate(element => Number.parseFloat(getComputedStyle(element).paddingBlockStart)) + + await visit(page, { + id: 'components-banner--default', + }) + const defaultPadding = await getPadding() + + await visit(page, { + id: 'components-banner-features--compact', + }) + const compactPadding = await getPadding() + + expect(compactPadding).toBeLessThan(defaultPadding) + }) + }) }) diff --git a/package-lock.json b/package-lock.json index e763e69eb71..bb2fb77a400 100644 --- a/package-lock.json +++ b/package-lock.json @@ -6594,6 +6594,10 @@ "resolved": "packages/react-compiler-check", "link": true }, + "node_modules/@primer/spec": { + "resolved": "packages/spec", + "link": true + }, "node_modules/@primer/styled-react": { "resolved": "packages/styled-react", "link": true @@ -9175,7 +9179,6 @@ }, "node_modules/@types/debug": { "version": "4.1.12", - "dev": true, "license": "MIT", "dependencies": { "@types/ms": "*" @@ -9319,7 +9322,6 @@ }, "node_modules/@types/mdast": { "version": "4.0.4", - "dev": true, "license": "MIT", "dependencies": { "@types/unist": "*" @@ -9332,7 +9334,6 @@ }, "node_modules/@types/ms": { "version": "0.7.31", - "dev": true, "license": "MIT" }, "node_modules/@types/node": { @@ -9476,7 +9477,6 @@ }, "node_modules/@types/unist": { "version": "2.0.11", - "dev": true, "license": "MIT" }, "node_modules/@types/yargs": { @@ -12783,7 +12783,6 @@ }, "node_modules/decode-named-character-reference": { "version": "1.0.2", - "dev": true, "license": "MIT", "dependencies": { "character-entities": "^2.0.0" @@ -12795,7 +12794,6 @@ }, "node_modules/decode-named-character-reference/node_modules/character-entities": { "version": "2.0.2", - "dev": true, "license": "MIT", "funding": { "type": "github", @@ -12945,7 +12943,6 @@ }, "node_modules/dequal": { "version": "2.0.3", - "dev": true, "license": "MIT", "engines": { "node": ">=6" @@ -12978,7 +12975,6 @@ }, "node_modules/devlop": { "version": "1.1.0", - "dev": true, "license": "MIT", "dependencies": { "dequal": "^2.0.0" @@ -18095,7 +18091,6 @@ }, "node_modules/mdast-util-from-markdown": { "version": "2.0.1", - "dev": true, "license": "MIT", "dependencies": { "@types/mdast": "^4.0.0", @@ -18118,7 +18113,6 @@ }, "node_modules/mdast-util-from-markdown/node_modules/@types/unist": { "version": "3.0.3", - "dev": true, "license": "MIT" }, "node_modules/mdast-util-frontmatter": { @@ -18267,7 +18261,6 @@ }, "node_modules/mdast-util-to-string": { "version": "4.0.0", - "dev": true, "license": "MIT", "dependencies": { "@types/mdast": "^4.0.0" @@ -18338,7 +18331,6 @@ "version": "4.0.2", "resolved": "https://registry.npmjs.org/micromark/-/micromark-4.0.2.tgz", "integrity": "sha512-zpe98Q6kvavpCr1NPVSCMebCKfD7CA2NqZ+rykeNhONIJBpc1tFKt9hucLGwha3jNTNI8lHpctWJWoimVF4PfA==", - "dev": true, "funding": [ { "type": "GitHub Sponsors", @@ -18374,7 +18366,6 @@ "version": "2.0.3", "resolved": "https://registry.npmjs.org/micromark-core-commonmark/-/micromark-core-commonmark-2.0.3.tgz", "integrity": "sha512-RDBrHEMSxVFLg6xvnXmb1Ayr2WzLAWjeSATAoxwKYJV94TeNavgoIdA0a9ytzDSVzBy2YKFK+emCPOEibLeCrg==", - "dev": true, "funding": [ { "type": "GitHub Sponsors", @@ -18618,7 +18609,6 @@ }, "node_modules/micromark-factory-destination": { "version": "2.0.1", - "dev": true, "funding": [ { "type": "GitHub Sponsors", @@ -18638,7 +18628,6 @@ }, "node_modules/micromark-factory-label": { "version": "2.0.1", - "dev": true, "funding": [ { "type": "GitHub Sponsors", @@ -18684,7 +18673,6 @@ }, "node_modules/micromark-factory-space": { "version": "2.0.1", - "dev": true, "funding": [ { "type": "GitHub Sponsors", @@ -18703,7 +18691,6 @@ }, "node_modules/micromark-factory-title": { "version": "2.0.1", - "dev": true, "funding": [ { "type": "GitHub Sponsors", @@ -18724,7 +18711,6 @@ }, "node_modules/micromark-factory-whitespace": { "version": "2.0.1", - "dev": true, "funding": [ { "type": "GitHub Sponsors", @@ -18745,7 +18731,6 @@ }, "node_modules/micromark-util-character": { "version": "2.1.0", - "dev": true, "funding": [ { "type": "GitHub Sponsors", @@ -18764,7 +18749,6 @@ }, "node_modules/micromark-util-chunked": { "version": "2.0.1", - "dev": true, "funding": [ { "type": "GitHub Sponsors", @@ -18782,7 +18766,6 @@ }, "node_modules/micromark-util-classify-character": { "version": "2.0.1", - "dev": true, "funding": [ { "type": "GitHub Sponsors", @@ -18802,7 +18785,6 @@ }, "node_modules/micromark-util-combine-extensions": { "version": "2.0.1", - "dev": true, "funding": [ { "type": "GitHub Sponsors", @@ -18821,7 +18803,6 @@ }, "node_modules/micromark-util-decode-numeric-character-reference": { "version": "2.0.2", - "dev": true, "funding": [ { "type": "GitHub Sponsors", @@ -18839,7 +18820,6 @@ }, "node_modules/micromark-util-decode-string": { "version": "2.0.1", - "dev": true, "funding": [ { "type": "GitHub Sponsors", @@ -18860,7 +18840,6 @@ }, "node_modules/micromark-util-encode": { "version": "2.0.1", - "dev": true, "funding": [ { "type": "GitHub Sponsors", @@ -18905,7 +18884,6 @@ }, "node_modules/micromark-util-html-tag-name": { "version": "2.0.1", - "dev": true, "funding": [ { "type": "GitHub Sponsors", @@ -18920,7 +18898,6 @@ }, "node_modules/micromark-util-normalize-identifier": { "version": "2.0.1", - "dev": true, "funding": [ { "type": "GitHub Sponsors", @@ -18938,7 +18915,6 @@ }, "node_modules/micromark-util-resolve-all": { "version": "2.0.1", - "dev": true, "funding": [ { "type": "GitHub Sponsors", @@ -18956,7 +18932,6 @@ }, "node_modules/micromark-util-sanitize-uri": { "version": "2.0.1", - "dev": true, "funding": [ { "type": "GitHub Sponsors", @@ -18976,7 +18951,6 @@ }, "node_modules/micromark-util-subtokenize": { "version": "2.1.0", - "dev": true, "funding": [ { "type": "GitHub Sponsors", @@ -18997,7 +18971,6 @@ }, "node_modules/micromark-util-symbol": { "version": "2.0.0", - "dev": true, "funding": [ { "type": "GitHub Sponsors", @@ -19014,7 +18987,6 @@ "version": "2.0.2", "resolved": "https://registry.npmjs.org/micromark-util-types/-/micromark-util-types-2.0.2.tgz", "integrity": "sha512-Yw0ECSpJoViF1qTU4DC6NwtC4aWGt1EkzaQB8KPPyCRR8z9TWeV0HbEFGTO+ZY1wB22zmxnJqhPyTpOVCpeHTA==", - "dev": true, "funding": [ { "type": "GitHub Sponsors", @@ -26290,7 +26262,6 @@ "version": "6.0.1", "resolved": "https://registry.npmjs.org/unist-util-is/-/unist-util-is-6.0.1.tgz", "integrity": "sha512-LsiILbtBETkDz8I9p1dQ0uyRUWuaQzd/cuEeS1hoRSyW5E5XGmTzlwY1OrNzzakGowI9Dr/I8HVaw4hTtnxy8g==", - "dev": true, "license": "MIT", "dependencies": { "@types/unist": "^3.0.0" @@ -26302,7 +26273,6 @@ }, "node_modules/unist-util-is/node_modules/@types/unist": { "version": "3.0.3", - "dev": true, "license": "MIT" }, "node_modules/unist-util-position-from-estree": { @@ -26342,7 +26312,6 @@ }, "node_modules/unist-util-stringify-position": { "version": "4.0.0", - "dev": true, "license": "MIT", "dependencies": { "@types/unist": "^3.0.0" @@ -26354,12 +26323,10 @@ }, "node_modules/unist-util-stringify-position/node_modules/@types/unist": { "version": "3.0.3", - "dev": true, "license": "MIT" }, "node_modules/unist-util-visit": { "version": "5.0.0", - "dev": true, "license": "MIT", "dependencies": { "@types/unist": "^3.0.0", @@ -26373,7 +26340,6 @@ }, "node_modules/unist-util-visit-parents": { "version": "6.0.1", - "dev": true, "license": "MIT", "dependencies": { "@types/unist": "^3.0.0", @@ -26386,12 +26352,10 @@ }, "node_modules/unist-util-visit-parents/node_modules/@types/unist": { "version": "3.0.3", - "dev": true, "license": "MIT" }, "node_modules/unist-util-visit/node_modules/@types/unist": { "version": "3.0.3", - "dev": true, "license": "MIT" }, "node_modules/universalify": { @@ -29361,6 +29325,19 @@ "rolldown": "^1.1.4" } }, + "packages/spec": { + "name": "@primer/spec", + "version": "0.0.0", + "dependencies": { + "mdast-util-from-markdown": "2.0.1", + "mdast-util-to-string": "4.0.0", + "unist-util-visit": "5.0.0" + }, + "devDependencies": { + "@primer/vitest-config": "^0.0.0", + "typescript": "^6.0.3" + } + }, "packages/styled-react": { "name": "@primer/styled-react", "version": "1.1.0", diff --git a/packages/react/src/Banner/Banner.examples.stories.tsx b/packages/react/src/Banner/Banner.examples.stories.tsx index b7ffbaa77db..d871f4fad18 100644 --- a/packages/react/src/Banner/Banner.examples.stories.tsx +++ b/packages/react/src/Banner/Banner.examples.stories.tsx @@ -47,6 +47,43 @@ export const WithUserAction = () => { ) } +WithUserAction.parameters = { + spec: ['./SPEC.md#accessibility'], +} + +export const WithRequiredActionAfterUserAction = () => { + const [hasError, setHasError] = React.useState(false) + const bannerRef = React.useRef>(null) + const focus = useFocus() + + return ( + <> + {hasError ? ( + Review errors} + /> + ) : null} + + + ) +} + +WithRequiredActionAfterUserAction.parameters = { + spec: ['./SPEC.md#accessibility', './SPEC.md#actions'], +} + export const WithAnnouncement = () => { type Choice = 'one' | 'two' | 'three' const messages: Map = new Map([ @@ -60,7 +97,7 @@ export const WithAnnouncement = () => { <> {messages.get(selected)}} + description={{messages.get(selected)}} onDismiss={action('onDismiss')} primaryAction={Button} secondaryAction={Button} @@ -90,6 +127,10 @@ export const WithAnnouncement = () => { ) } +WithAnnouncement.parameters = { + spec: ['./SPEC.md#accessibility'], +} + export const WithCustomHeading = () => { return ( { ) } +WithCustomHeading.parameters = { + spec: ['./SPEC.md#default', './SPEC.md#actions', './SPEC.md#dismissal'], +} + export const InSidebar = () => { return ( <> @@ -139,6 +184,10 @@ export const InSidebar = () => { ) } +InSidebar.parameters = { + spec: ['./SPEC.md#default', './SPEC.md#layout'], +} + export const Multiline = () => { return ( { ) } +Multiline.parameters = { + spec: ['./SPEC.md#actions', './SPEC.md#dismissal', './SPEC.md#layout'], +} + export const DismissBanner = () => { const ref = React.useRef>(null) const [banner, setBanner] = React.useState | null>({ @@ -187,3 +240,7 @@ export const DismissBanner = () => { ) } + +DismissBanner.parameters = { + spec: ['./SPEC.md#accessibility', './SPEC.md#dismissal'], +} diff --git a/packages/react/src/Banner/Banner.features.stories.tsx b/packages/react/src/Banner/Banner.features.stories.tsx index ee58da760ce..674a8be13ad 100644 --- a/packages/react/src/Banner/Banner.features.stories.tsx +++ b/packages/react/src/Banner/Banner.features.stories.tsx @@ -33,6 +33,10 @@ export const Critical = () => { ) } +Critical.parameters = { + spec: ['./SPEC.md#default', './SPEC.md#variants-and-leading-visuals'], +} + export const Info = () => { return ( { ) } +Info.parameters = { + spec: ['./SPEC.md#default', './SPEC.md#variants-and-leading-visuals', './SPEC.md#dismissal'], +} + export const Success = () => { return ( { ) } +Success.parameters = { + spec: ['./SPEC.md#default', './SPEC.md#variants-and-leading-visuals', './SPEC.md#dismissal'], +} + export const Upsell = () => { return ( { ) } +Upsell.parameters = { + spec: ['./SPEC.md#default', './SPEC.md#variants-and-leading-visuals', './SPEC.md#dismissal'], +} + export const Warning = () => { return ( { ) } +Warning.parameters = { + spec: ['./SPEC.md#default', './SPEC.md#variants-and-leading-visuals', './SPEC.md#dismissal'], +} + export const Dismiss = () => { return ( { ) } +Dismiss.parameters = { + spec: ['./SPEC.md#default', './SPEC.md#dismissal'], +} + export const DismissWithActions = () => { return ( { ) } +DismissWithActions.parameters = { + spec: ['./SPEC.md#default', './SPEC.md#actions', './SPEC.md#dismissal'], +} + export const WithHiddenTitle = () => { return ( { ) } +WithHiddenTitle.parameters = { + spec: ['./SPEC.md#default'], +} + export const WithHiddenTitleAndActions = () => { return ( { ) } +WithHiddenTitleAndActions.parameters = { + spec: ['./SPEC.md#default', './SPEC.md#actions'], +} + export const DismissibleWithHiddenTitleAndActions = () => { return ( { ) } +DismissibleWithHiddenTitleAndActions.parameters = { + spec: ['./SPEC.md#default', './SPEC.md#actions', './SPEC.md#dismissal'], +} + export const DismissibleWithHiddenTitleAndSecondaryAction = () => { return ( { ) } +DismissibleWithHiddenTitleAndSecondaryAction.parameters = { + spec: ['./SPEC.md#default', './SPEC.md#actions', './SPEC.md#dismissal'], +} + export const WithActions = () => { return ( { ) } +WithActions.parameters = { + spec: ['./SPEC.md#default', './SPEC.md#variants-and-leading-visuals', './SPEC.md#actions'], +} + +export const ActionsAsLinks = () => { + return ( + + Extend subscription + + } + secondaryAction={ + + Learn more + + } + variant="warning" + /> + ) +} + +ActionsAsLinks.parameters = { + spec: ['./SPEC.md#actions'], +} + export const CustomIcon = () => { return ( { ) } +CustomIcon.parameters = { + spec: ['./SPEC.md#variants-and-leading-visuals'], +} + export const FlushInsideDialog = () => { const onDialogClose = React.useCallback(() => {}, []) @@ -288,6 +364,24 @@ export const FlushInsideDialog = () => { ) } +FlushInsideDialog.parameters = { + spec: ['./SPEC.md#actions', './SPEC.md#layout'], +} + +export const Compact = () => { + return ( + + ) +} + +Compact.parameters = { + spec: ['./SPEC.md#default', './SPEC.md#layout'], +} + export const ActionsLayoutStacked = () => { return ( @@ -352,6 +446,10 @@ export const ActionsLayoutStacked = () => { ) } +ActionsLayoutStacked.parameters = { + spec: ['./SPEC.md#actions', './SPEC.md#layout'], +} + export const ActionsLayoutInline = () => { return ( @@ -405,3 +503,7 @@ export const ActionsLayoutInline = () => { ) } + +ActionsLayoutInline.parameters = { + spec: ['./SPEC.md#actions', './SPEC.md#layout'], +} diff --git a/packages/react/src/Banner/Banner.stories.tsx b/packages/react/src/Banner/Banner.stories.tsx index 986e5d5ee5f..541f4c663b7 100644 --- a/packages/react/src/Banner/Banner.stories.tsx +++ b/packages/react/src/Banner/Banner.stories.tsx @@ -32,12 +32,15 @@ export const Default = () => { ) } +Default.parameters = {} + const iconMap = { GitPullRequestIcon: , CopilotIcon: , } export const Playground: StoryObj = { + parameters: {}, render: ({onDismiss, primaryAction, secondaryAction, leadingVisual, ...rest}) => { // Map the string selection to the actual icon component const leadingVisualElement = leadingVisual && iconMap[leadingVisual as keyof typeof iconMap] diff --git a/packages/react/src/Banner/Banner.test.tsx b/packages/react/src/Banner/Banner.test.tsx index 73d771ad36a..2b47df843aa 100644 --- a/packages/react/src/Banner/Banner.test.tsx +++ b/packages/react/src/Banner/Banner.test.tsx @@ -1,6 +1,7 @@ import {describe, expect, it, vi} from 'vitest' import {render, screen} from '@testing-library/react' import userEvent from '@testing-library/user-event' +import {createRef} from 'react' import {Banner} from '../Banner' import {implementsClassName} from '../utils/testing' import classes from './Banner.module.css' @@ -8,9 +9,12 @@ import classes from './Banner.module.css' describe('Banner', () => { implementsClassName(props => , classes.Banner) + /** + * @see ./SPEC.md#default + */ it('should render as a region element', () => { render() - expect(screen.getByRole('region', {name: 'test'})).toBeInTheDocument() + expect(screen.getByRole('region', {name: 'test'})).toHaveAttribute('tabindex', '-1') expect(screen.getByRole('heading', {name: 'test'})).toBeInTheDocument() }) @@ -44,6 +48,9 @@ describe('Banner', () => { expect(dismissButton).toBeInTheDocument() }) + /** + * @see ./SPEC.md#default + */ it('should label the landmark element with the title by default', () => { render() const region = screen.getByRole('region', {name: 'My Banner Title'}) @@ -51,41 +58,62 @@ describe('Banner', () => { expect(region).not.toHaveAttribute('aria-label') }) + /** + * @see ./SPEC.md#variants-and-leading-visuals + */ it('should use aria-labelledby to reference the title for the critical variant', () => { render() const region = screen.getByRole('region', {name: 'Critical Issue'}) expect(region).toHaveAttribute('aria-labelledby') }) + /** + * @see ./SPEC.md#variants-and-leading-visuals + */ it('should use aria-labelledby to reference the title for the info variant', () => { render() const region = screen.getByRole('region', {name: 'Information'}) expect(region).toHaveAttribute('aria-labelledby') }) + /** + * @see ./SPEC.md#variants-and-leading-visuals + */ it('should use aria-labelledby to reference the title for the success variant', () => { render() const region = screen.getByRole('region', {name: 'Success Message'}) expect(region).toHaveAttribute('aria-labelledby') }) + /** + * @see ./SPEC.md#variants-and-leading-visuals + */ it('should use aria-labelledby to reference the title for the upsell variant', () => { render() const region = screen.getByRole('region', {name: 'Recommendation'}) expect(region).toHaveAttribute('aria-labelledby') }) + /** + * @see ./SPEC.md#variants-and-leading-visuals + */ it('should use aria-labelledby to reference the title for the warning variant', () => { render() const region = screen.getByRole('region', {name: 'Warning'}) expect(region).toHaveAttribute('aria-labelledby') }) + /** + * @see ./SPEC.md#default + */ it('should support the `aria-label` prop to override the default label for the landmark', () => { render() expect(screen.getByRole('region')).toHaveAttribute('aria-label', 'Test') }) + /** + * @see ./SPEC.md#default + */ it('should prefer aria-labelledby over aria-label and not set both', () => { render( @@ -98,6 +126,9 @@ describe('Banner', () => { expect(region).not.toHaveAttribute('aria-label') }) + /** + * @see ./SPEC.md#default + */ it('should only set aria-label when aria-labelledby is not provided', () => { render() const region = screen.getByRole('region') @@ -105,6 +136,9 @@ describe('Banner', () => { expect(region).not.toHaveAttribute('aria-labelledby') }) + /** + * @see ./SPEC.md#default + */ it('should use aria-labelledby to reference Banner.Title when provided as a child', () => { render( @@ -118,6 +152,9 @@ describe('Banner', () => { expect(region).not.toHaveAttribute('aria-label') }) + /** + * @see ./SPEC.md#default + */ it('should use aria-labelledby to reference Banner.Title with custom id', () => { render( @@ -129,12 +166,28 @@ describe('Banner', () => { expect(screen.getByRole('heading')).toHaveAttribute('id', 'custom-title-id') }) + /** + * @see ./SPEC.md#default + */ it('should default the title to a h2', () => { render() expect(screen.getByRole('heading', {level: 2})).toBeInTheDocument() expect(screen.getByRole('heading', {level: 2})).toEqual(screen.getByText('test')) }) + /** + * @see ./SPEC.md#default + */ + it('should keep a visually hidden title in the accessibility tree', () => { + render() + + expect(screen.getByRole('region', {name: 'Hidden title'})).toBeInTheDocument() + expect(screen.getByRole('heading', {name: 'Hidden title'})).toBeInTheDocument() + }) + + /** + * @see ./SPEC.md#default + */ it('should throw an error if no title is provided', () => { const spy = vi.spyOn(console, 'error').mockImplementation(() => {}) expect(() => { @@ -143,12 +196,47 @@ describe('Banner', () => { spy.mockRestore() }) + /** + * @see ./SPEC.md#default + */ + it('should throw an error if more than one title is provided', () => { + const spy = vi.spyOn(console, 'error').mockImplementation(() => {}) + expect(() => { + render( + + Title child + , + ) + }).toThrow( + 'Expected exactly one title to be provided to the component with either the `title` prop or through `` but multiple titles were found', + ) + spy.mockRestore() + }) + + /** + * @see ./SPEC.md#default + */ + it('should not automatically announce itself', () => { + render() + const banner = screen.getByRole('region', {name: 'test'}) + + expect(banner).not.toHaveAttribute('aria-live') + expect(screen.queryByRole('alert')).not.toBeInTheDocument() + expect(screen.queryByRole('status')).not.toBeInTheDocument() + }) + + /** + * @see ./SPEC.md#default + */ it('should rendering a description with the `description` prop', () => { render() expect(screen.getByText('test-description')).toBeInTheDocument() expect(screen.getByRole('region', {name: 'test'})).toContainElement(screen.getByText('test-description')) }) + /** + * @see ./SPEC.md#actions + */ it('should support a primary action', async () => { const user = userEvent.setup() const onClick = vi.fn() @@ -161,11 +249,15 @@ describe('Banner', () => { ) expect(screen.queryAllByRole('button', {name: 'test primary action', hidden: true}).length).toBe(2) + expect(screen.getAllByRole('button', {name: 'test primary action'})).toHaveLength(1) await user.click(screen.queryAllByText('test primary action')[0]) expect(onClick).toHaveBeenCalledTimes(1) }) + /** + * @see ./SPEC.md#actions + */ it('should support a secondary action', async () => { const user = userEvent.setup() const onClick = vi.fn() @@ -178,11 +270,15 @@ describe('Banner', () => { ) expect(screen.queryAllByRole('button', {name: 'test secondary action', hidden: true}).length).toBe(2) + expect(screen.getAllByRole('button', {name: 'test secondary action'})).toHaveLength(1) await user.click(screen.queryAllByText('test secondary action')[0]) expect(onClick).toHaveBeenCalledTimes(1) }) + /** + * @see ./SPEC.md#actions + */ it('should support primary action and secondary action', () => { render( { expect(screen.queryAllByRole('button', {name: 'test primary action', hidden: true}).length).toBe(2) expect(screen.queryAllByRole('button', {name: 'test secondary action', hidden: true}).length).toBe(2) + expect(screen.getAllByRole('button', {name: 'test primary action'})).toHaveLength(1) + expect(screen.getAllByRole('button', {name: 'test secondary action'})).toHaveLength(1) + }) + + /** + * @see ./SPEC.md#actions + */ + it.each(['default', 'inline', 'stacked'] as const)( + 'should expose one operable instance of each action in the %s layout', + actionsLayout => { + render( + test primary action} + secondaryAction={test secondary action} + />, + ) + + expect(screen.getAllByRole('button', {name: 'test primary action'})).toHaveLength(1) + expect(screen.getAllByRole('button', {name: 'test secondary action'})).toHaveLength(1) + }, + ) + + /** + * @see ./SPEC.md#actions + */ + it('should support rendering actions as links', () => { + render( + + test primary action + + } + secondaryAction={ + + test secondary action + + } + />, + ) + + expect(screen.getAllByRole('link', {name: 'test primary action'})).toHaveLength(1) + expect(screen.getAllByRole('link', {name: 'test secondary action'})).toHaveLength(1) + }) + + /** + * @see ./SPEC.md#actions + */ + it('should not dismiss the Banner when an action is activated', async () => { + const user = userEvent.setup() + const onAction = vi.fn() + const onDismiss = vi.fn() + render( + test primary action} + />, + ) + + await user.click(screen.getByRole('button', {name: 'test primary action'})) + + expect(onAction).toHaveBeenCalledOnce() + expect(onDismiss).not.toHaveBeenCalled() }) + /** + * @see ./SPEC.md#dismissal + */ it('should call `onDismiss` when the dismiss button is activated', async () => { const user = userEvent.setup() const onDismiss = vi.fn() @@ -214,6 +380,9 @@ describe('Banner', () => { expect(onDismiss).toHaveBeenCalledTimes(3) }) + /** + * @see ./SPEC.md#dismissal + */ it.each(['critical', 'info', 'success', 'upsell', 'warning'] as const)( 'should support onDismiss for the %s variant', variant => { @@ -223,11 +392,43 @@ describe('Banner', () => { }, ) + /** + * @see ./SPEC.md#default + */ it('should pass extra props onto the container element', () => { const {container} = render() expect(container.firstChild).toHaveAttribute('data-testid', 'test') }) + /** + * @see ./SPEC.md#default + */ + it('should forward the ref to the root section', () => { + const ref = createRef() + render() + + expect(ref.current).toBeInstanceOf(HTMLElement) + expect(ref.current?.tagName).toBe('SECTION') + expect(ref.current).toHaveAttribute('data-component', 'Banner') + }) + + /** + * @see ./SPEC.md#variants-and-leading-visuals + */ + it.each(['critical', 'info', 'success', 'upsell', 'warning'] as const)( + 'should render a decorative built-in leading visual for the %s variant', + variant => { + const {container} = render() + const visual = container.querySelector('[data-component="Banner.Icon"] svg') + + expect(visual).toBeInTheDocument() + expect(visual).toHaveAttribute('aria-hidden', 'true') + }, + ) + + /** + * @see ./SPEC.md#variants-and-leading-visuals + */ it('should support a custom icon for info and upsell variants', () => { const CustomIcon = vi.fn(() => ) const {rerender} = render( @@ -248,12 +449,16 @@ describe('Banner', () => { expect(screen.queryByTestId('icon')).toBe(null) }) + /** + * @see ./SPEC.md#variants-and-leading-visuals + */ it('should support a custom leadingVisual for info and upsell variants', () => { const CustomIcon = vi.fn(() => ) const {rerender} = render( } />, ) expect(screen.getByTestId('leading-visual')).toBeInTheDocument() + expect(screen.getByTestId('leading-visual')).toHaveAttribute('aria-hidden', 'true') rerender(} />) expect(screen.getByTestId('leading-visual')).toBeInTheDocument() @@ -268,6 +473,9 @@ describe('Banner', () => { expect(screen.queryByTestId('leading-visual')).toBe(null) }) + /** + * @see ./SPEC.md#variants-and-leading-visuals + */ it('should prefer leadingVisual over icon when both are provided', () => { const LeadingVisualIcon = () => const DeprecatedIcon = () => @@ -284,32 +492,58 @@ describe('Banner', () => { expect(screen.queryByTestId('deprecated-icon')).toBe(null) }) + /** + * @see ./SPEC.md#layout + */ it('should render data-actions-layout attribute with inline value', () => { const {container} = render() expect(container.firstChild).toHaveAttribute('data-actions-layout', 'inline') }) + /** + * @see ./SPEC.md#layout + */ it('should render data-actions-layout attribute with stacked value', () => { const {container} = render() expect(container.firstChild).toHaveAttribute('data-actions-layout', 'stacked') }) + /** + * @see ./SPEC.md#layout + */ it('should render data-actions-layout attribute with default value when not specified', () => { const {container} = render() expect(container.firstChild).toHaveAttribute('data-actions-layout', 'default') }) + /** + * @see ./SPEC.md#layout + */ it('should render data-flush attribute when flush is true', () => { const {container} = render() expect(container.firstChild).toHaveAttribute('data-flush') }) + /** + * @see ./SPEC.md#layout + */ it('should not render data-flush attribute when flush is false', () => { const {container} = render() expect(container.firstChild).not.toHaveAttribute('data-flush') }) + /** + * @see ./SPEC.md#layout + */ + it('should render the compact layout', () => { + const {container} = render() + expect(container.firstChild).toHaveAttribute('data-layout', 'compact') + }) + describe('Banner.Title', () => { + /** + * @see ./SPEC.md#default + */ it('should render as a h2 element by default', () => { render( @@ -319,6 +553,9 @@ describe('Banner', () => { expect(screen.getByRole('heading', {level: 2, name: 'test'})).toBeInTheDocument() }) + /** + * @see ./SPEC.md#default + */ it('should support rendering as any heading element above level 2', () => { const levels = [2, 3, 4, 5, 6] as const diff --git a/packages/react/src/Banner/Banner.tsx b/packages/react/src/Banner/Banner.tsx index 0fb19589f75..a382519eb13 100644 --- a/packages/react/src/Banner/Banner.tsx +++ b/packages/react/src/Banner/Banner.tsx @@ -134,22 +134,24 @@ export const Banner = React.forwardRef(function Banner const visual = leadingVisual ?? icon useDevOnlyEffect(() => { - if (title) { - return - } - const {current: banner} = bannerRef if (!banner) { return } - const hasTitle = banner.querySelector('[data-banner-title]') - if (!hasTitle) { + const titles = banner.querySelectorAll('[data-banner-title]') + if (titles.length === 0) { throw new Error( 'Expected a title to be provided to the component with the `title` prop or through `` but no title was found', ) } - }, [title]) + + if (titles.length > 1) { + throw new Error( + 'Expected exactly one title to be provided to the component with either the `title` prop or through `` but multiple titles were found', + ) + } + }, [children, title]) return ( diff --git a/packages/react/src/Banner/SPEC.md b/packages/react/src/Banner/SPEC.md new file mode 100644 index 00000000000..e55e7899506 --- /dev/null +++ b/packages/react/src/Banner/SPEC.md @@ -0,0 +1,145 @@ +# Banner spec + +Banner highlights important information or provides feedback after a user action. + +## Accessibility + +- A Banner MUST have a title that identifies its purpose and communicates the type of message it contains. +- A Banner MUST be discoverable as both a named landmark and part of the page's heading hierarchy. +- A Banner's variant MUST be communicated by more than color alone. +- Interactive controls within a Banner MUST have descriptive accessible names that include their visible labels. +- When a Banner appears dynamically, consumers MUST either move focus to the Banner when its content is essential or announce its content through a persistent live region that existed before the Banner appeared when moving focus would be disruptive. +- When Banner content changes after it is rendered, consumers MUST announce the update through a persistent live region within the Banner or an external live region that existed before the content changed. +- When a dismissed Banner is removed, consumers MUST move focus to a useful element if removal would otherwise cause focus loss. +- Banner content MUST remain readable and operable without horizontal scrolling at 200% zoom and at a viewport size of 320 by 256 CSS pixels. +- Text MUST have a contrast ratio of at least 4.5:1. Icons and other meaningful non-text content MUST have a contrast ratio of at least 3:1. +- Interactive controls MUST have a target size of at least 24 by 24 CSS pixels. + +## Features + +### Default + +Banner provides a title, an optional description, and a visual that communicates the selected variant. + +#### Default markup + +```html +
+ + +

Description text

+
+``` + +- The root MUST render as a `
` landmark. +- The root MUST be programmatically focusable without placing it in the sequential focus order. +- The Banner title MUST render as a heading. It MUST render as an `

` by default and MAY render as an `

`, `

`, `

`, or `
` when consumers provide the appropriate heading level. +- The landmark MUST be named by the Banner title by default. +- When `aria-label` is provided without `aria-labelledby`, the landmark MUST use `aria-label` instead of the Banner title for its accessible name. +- When `aria-labelledby` is provided, it MUST take precedence over `aria-label`. +- A visually hidden title MUST remain in the accessibility tree and continue to name the landmark. +- The built-in leading visual MUST be hidden from the accessibility tree. + +#### Default behavior + +- Consumers MUST provide exactly one title, either through the `title` prop or a `Banner.Title` child. +- The component MUST report a development error when no title is provided. +- Hiding the title MUST affect only its visual presentation. +- The component MUST NOT automatically announce itself when it is rendered. + +#### Default public API + +- When a `Banner.Title` child provides a custom `id`, consumers MUST provide a matching `aria-labelledby` value on Banner. +- The forwarded ref MUST target the root `
`. +- Additional root element props MUST be forwarded to the root `
`. + +### Variants and leading visuals + +Variants communicate the type of message and provide a corresponding visual treatment. + +#### Variants and leading visuals behavior + +- Banner MUST support critical, info, success, upsell, and warning variants. +- The `critical` variant SHOULD be used when a user-blocking error or destructive condition requires immediate attention. +- The `info` variant SHOULD be used for neutral or informational messages that do not require an immediate action. +- The `success` variant SHOULD be used to confirm that an action has completed successfully. +- The `warning` variant SHOULD be used to surface a potential issue or condition that may require attention but is not yet an error. +- The `upsell` variant SHOULD be used for promotional content, feature suggestions, or upgrade prompts. +- Each variant MUST render a leading visual that distinguishes it without relying on color alone. +- Consumers MUST NOT be able to remove the leading visual. +- Custom leading visuals MUST be supported only for info and upsell variants. +- When both `leadingVisual` and `icon` are provided, `leadingVisual` MUST take precedence. +- A custom leading visual SHOULD be decorative and hidden from the accessibility tree. It MUST NOT be the only way the Banner's purpose is communicated. +- Changing the variant MUST NOT change the landmark role or accessible naming behavior. + +### Actions + +Banner may provide one primary action, one secondary action, or both. + +#### Actions markup + +```html + + + + +Action label + + +Action label +``` + +- Primary and secondary actions MUST render as interactive controls. +- Primary and secondary actions MAY render as buttons for actions or links for navigation. +- Responsive layout changes MUST expose only one operable instance of each action at a time. + +#### Actions behavior + +- When both actions are present, the primary action MUST represent the recommended action. +- In the default actions layout, actions MUST remain inline when sufficient container space is available and stack when the available container width is less than 500 CSS pixels. +- In the inline actions layout, actions SHOULD remain inline with the content but MUST stack at narrow viewport widths. +- In the stacked actions layout, actions MUST remain in a separate stacked row. +- When actions stack, the primary action MUST precede the secondary action. +- Activating an action MUST invoke the handler provided to that action without invoking Banner dismissal. + +#### Actions accessibility + +- When a Banner appears dynamically and contains an action that is required to continue, consumers MUST move focus to the Banner when it appears. + +### Dismissal + +A Banner may provide a dismiss control when the message can be safely removed. + +#### Dismissal markup + +```html + +``` + +- A dismissible Banner MUST render a button with the accessible name `Dismiss banner`. + +#### Dismissal behavior + +- Activating the dismiss button MUST invoke `onDismiss` once for each activation. +- Banner MUST NOT remove itself after dismissal. Consumers are responsible for updating visibility in response to `onDismiss`. +- Banner MUST NOT move focus after dismissal. Consumers are responsible for restoring focus when removing the Banner would otherwise cause focus loss. + +#### Dismissal accessibility + +- The dismiss button MUST be reachable by keyboard focus and MUST be activated by Space or Enter. + +### Layout + +Banner supports default and compact spacing, responsive action placement, and a flush presentation for confined surfaces. + +#### Layout behavior + +- The default layout MUST use the standard Banner spacing. +- The compact layout MUST reduce the Banner's padding without changing its semantic structure. +- The default actions layout MUST respond to the Banner's available container width rather than only the viewport width. +- A flush Banner MUST span the available width without rounded side borders. +- Flush presentation SHOULD be used only within confined surfaces such as dialogs, tables, cards, or boxes. + +## Log + + diff --git a/packages/react/src/Blankslate/Blankslate.docs.json b/packages/react/src/Blankslate/Blankslate.docs.json index 98babaf6230..8d67c44987d 100644 --- a/packages/react/src/Blankslate/Blankslate.docs.json +++ b/packages/react/src/Blankslate/Blankslate.docs.json @@ -27,6 +27,12 @@ }, { "id": "experimental-components-blankslate-features--spacious" + }, + { + "id": "experimental-components-blankslate-features--size-small" + }, + { + "id": "experimental-components-blankslate-features--size-large" } ], "importPath": "@primer/react/experimental", @@ -39,12 +45,12 @@ { "name": "narrow", "type": "boolean", - "description": "" + "description": "Constrain the maximum width of the component" }, { "name": "spacious", "type": "boolean", - "description": "" + "description": "Increase the component padding" }, { "name": "className", @@ -55,7 +61,7 @@ { "name": "size", "type": "'small' | 'medium' | 'large'", - "description": "The size of the componeont", + "description": "The size of the component", "defaultValue": "'medium'" } ], @@ -69,7 +75,8 @@ "props": [ { "name": "as", - "type": "'h1' | 'h2' | 'h3' | 'h4' | 'h5' | 'h6'" + "type": "'h1' | 'h2' | 'h3' | 'h4' | 'h5' | 'h6'", + "defaultValue": "'h2'" } ] }, diff --git a/packages/react/src/Blankslate/Blankslate.features.stories.tsx b/packages/react/src/Blankslate/Blankslate.features.stories.tsx index 74224196eab..6ed1ef8f4ca 100644 --- a/packages/react/src/Blankslate/Blankslate.features.stories.tsx +++ b/packages/react/src/Blankslate/Blankslate.features.stories.tsx @@ -25,6 +25,10 @@ export const WithVisual = () => ( ) +WithVisual.parameters = { + spec: ['./SPEC.md#visual'], +} + export const WithPrimaryActionAsLink = () => ( @@ -36,6 +40,10 @@ export const WithPrimaryActionAsLink = () => ( ) +WithPrimaryActionAsLink.parameters = { + spec: ['./SPEC.md#actions'], +} + export const WithPrimaryActionAsButton = () => { const [isOpen, setIsOpen] = React.useState(false) const onDialogClose = React.useCallback(() => setIsOpen(false), []) @@ -65,6 +73,10 @@ export const WithPrimaryActionAsButton = () => { ) } +WithPrimaryActionAsButton.parameters = { + spec: ['./SPEC.md#actions'], +} + export const WithSecondaryAction = () => ( @@ -76,6 +88,10 @@ export const WithSecondaryAction = () => ( ) +WithSecondaryAction.parameters = { + spec: ['./SPEC.md#actions'], +} + export const WithBorder = () => ( @@ -118,6 +134,10 @@ export const SizeSmall = () => ( ) +SizeSmall.parameters = { + spec: ['./SPEC.md#actions'], +} + export const SizeLarge = () => ( @@ -129,3 +149,7 @@ export const SizeLarge = () => ( Secondary action ) + +SizeLarge.parameters = { + spec: ['./SPEC.md#actions'], +} diff --git a/packages/react/src/Blankslate/Blankslate.stories.tsx b/packages/react/src/Blankslate/Blankslate.stories.tsx index 571a68a5c53..a5cc0a6c31c 100644 --- a/packages/react/src/Blankslate/Blankslate.stories.tsx +++ b/packages/react/src/Blankslate/Blankslate.stories.tsx @@ -30,6 +30,8 @@ export const Default = () => ( ) +Default.parameters = {} + export const Playground: StoryFn< ComponentProps & {primaryAction: boolean; secondaryAction: boolean} > = ({primaryAction, secondaryAction, ...rest}) => ( @@ -47,6 +49,8 @@ export const Playground: StoryFn< ) +Playground.parameters = {} + Playground.args = { border: false, narrow: false, diff --git a/packages/react/src/Blankslate/Blankslate.test.tsx b/packages/react/src/Blankslate/Blankslate.test.tsx index 912d0e02daa..951c4fba97c 100644 --- a/packages/react/src/Blankslate/Blankslate.test.tsx +++ b/packages/react/src/Blankslate/Blankslate.test.tsx @@ -38,6 +38,22 @@ function getSizePadding(size: 'small' | 'medium' | 'large') { describe('Blankslate', () => { implementsClassName(Blankslate, classes.Blankslate) + /** + * @see ./SPEC.md#default + */ + it('forwards additional props to the outer container and className to the inner container', () => { + const {container} = render( + + Test content + , + ) + const outerContainer = screen.getByTestId('outer-container') + + expect(outerContainer).toBe(container.firstChild) + expect(outerContainer).not.toHaveClass('custom-class') + expect(outerContainer.firstChild).toHaveClass('custom-class') + }) + it('should render with border when border is true', () => { const {container} = render(Test content) expect(container.firstChild!.firstChild).toHaveAttribute('data-border', '') @@ -92,19 +108,25 @@ describe('Blankslate', () => { }) describe('Blankslate.Visual', () => { - it('should render a visual element', () => { + /** + * @see ./SPEC.md#visual + */ + it('hides the visual from the accessibility tree', () => { render( - + , ) - expect(screen.getByTestId('test-icon')).toBeInTheDocument() + expect(screen.getByTestId('test-icon').parentElement).toHaveAttribute('aria-hidden', 'true') }) }) describe('Blankslate.Heading', () => { + /** + * @see ./SPEC.md#default + */ it('should render a heading with h2 by default', () => { render( @@ -115,6 +137,9 @@ describe('Blankslate', () => { expect(screen.getByRole('heading', {level: 2})).toHaveClass('Blankslate-Heading') }) + /** + * @see ./SPEC.md#default + */ it('should render with a custom heading level', () => { render( @@ -126,6 +151,9 @@ describe('Blankslate', () => { }) describe('Blankslate.Description', () => { + /** + * @see ./SPEC.md#default + */ it('should render a description', () => { render( @@ -137,6 +165,9 @@ describe('Blankslate', () => { }) describe('Blankslate.PrimaryAction', () => { + /** + * @see ./SPEC.md#actions + */ it('should render a primary action button', () => { render( @@ -146,6 +177,17 @@ describe('Blankslate', () => { expect(screen.getByRole('button', {name: 'Primary action'})).toBeInTheDocument() }) + /** + * @see ./SPEC.md#actions + */ + it('should render independently from Blankslate', () => { + render(Primary action) + expect(screen.getByRole('button', {name: 'Primary action'})).toBeInTheDocument() + }) + + /** + * @see ./SPEC.md#actions + */ it('should handle click events on the button', async () => { const user = userEvent.setup() const onClick = vi.fn() @@ -159,6 +201,9 @@ describe('Blankslate', () => { expect(onClick).toHaveBeenCalledTimes(1) }) + /** + * @see ./SPEC.md#actions + */ it('should render as an anchor when href is provided', () => { render( @@ -169,9 +214,27 @@ describe('Blankslate', () => { expect(link).toBeInTheDocument() expect(link).toHaveAttribute('href', 'https://example.com') }) + + /** + * @see ./SPEC.md#actions + */ + it('should render as an anchor when href is an empty string', () => { + const {container} = render( + + Primary action + , + ) + const link = container.querySelector('a') + + expect(link).toHaveAttribute('href', '') + expect(link).toHaveTextContent('Primary action') + }) }) describe('Blankslate.SecondaryAction', () => { + /** + * @see ./SPEC.md#actions + */ it('should render a secondary action link', () => { render( diff --git a/packages/react/src/Blankslate/Blankslate.tsx b/packages/react/src/Blankslate/Blankslate.tsx index fa785e9991b..7d507c7f0e1 100644 --- a/packages/react/src/Blankslate/Blankslate.tsx +++ b/packages/react/src/Blankslate/Blankslate.tsx @@ -62,7 +62,12 @@ type BlankslateVisualProps = React.HTMLAttributes function Visual({children, className, ...rest}: BlankslateVisualProps) { return ( - + ) @@ -113,7 +118,7 @@ function PrimaryAction({children, href, ...props}: BlankslatePrimaryActionProps)
+ + +Create the first page + + +Learn more about wikis +``` + +- A primary action without a navigation target MUST render as a button. +- A primary action with a navigation target MUST render as a link. +- A secondary action MUST render as a link. + +#### Actions behavior + +- Consumers SHOULD use the primary action for the recommended next step. +- Consumers SHOULD use the secondary action for supplementary navigation, such as documentation or more information. + +#### Actions public API + +- A button-form primary action MUST forward additional button props to its rendered button. +- A primary action MAY be rendered independently from Blankslate. + +## Log + + diff --git a/packages/react/src/DataTable/DataTable.docs.json b/packages/react/src/DataTable/DataTable.docs.json index 3b0f5c7006e..4757c38e450 100644 --- a/packages/react/src/DataTable/DataTable.docs.json +++ b/packages/react/src/DataTable/DataTable.docs.json @@ -106,6 +106,13 @@ "description": "Indicate whether the sorting of the table is managed externally. When `true`, the table will not sort the data internally when a column header is clicked. The `onToggleSort` callback will still be invoked when a column header is clicked.", "defaultValue": "false" }, + { + "name": "getRowId", + "type": "(rowData: Data) => string | number", + "required": false, + "description": "Provide a function to determine the unique identifier for each row. By default, the table uses the `id` field from the data.", + "defaultValue": "" + }, { "name": "onToggleSort", "type": "(columnId: ObjectPaths | string | number, direction: 'ASC' | 'DESC') => void", @@ -292,7 +299,7 @@ }, { "name": "defaultPageIndex", - "type": "string", + "type": "number", "description": "Provide an optional index to specify the default selected page" }, { @@ -354,33 +361,13 @@ ] }, { - "name": "Table.SortHeader", + "name": "Column options", "props": [ { - "name": "direction", - "type": "'ASC' | 'DESC' | 'NONE'", - "required": true, - "description": "Specify the sort direction for the TableHeader", - "defaultValue": "" - }, - { - "name": "onToggleSort", - "type": "() => void", - "required": true, - "description": "Provide a handler that is called when the sortable TableHeader is\ninteracted with via a click or keyboard interaction", - "defaultValue": "" + "name": "id", + "type": "string", + "description": "Provide an explicit identifier for the column" }, - { - "name": "align", - "type": "'end' | 'start'", - "description": "The horizontal alignment of the cell's content", - "defaultValue": "" - } - ] - }, - { - "name": "Column options", - "props": [ { "name": "align", "type": "'start' | 'end'", diff --git a/packages/react/src/DataTable/DataTable.features.stories.tsx b/packages/react/src/DataTable/DataTable.features.stories.tsx index 6563a3054bd..3c712e725a6 100644 --- a/packages/react/src/DataTable/DataTable.features.stories.tsx +++ b/packages/react/src/DataTable/DataTable.features.stories.tsx @@ -142,7 +142,6 @@ export const WithTitle = () => ( ( ) +WithTitle.parameters = { + spec: ['./spec/header-and-actions.md#header-and-actions'], +} + export const WithTitleAndSubtitle = () => ( @@ -330,6 +333,10 @@ export const WithSorting = () => { ) } +WithSorting.parameters = { + spec: ['./spec/sorting.md#sorting'], +} + export const WithCustomSorting = () => { const rows = Array.from(data).sort((a, b) => { return b.updatedAt - a.updatedAt @@ -755,6 +762,10 @@ export const WithRowAction = () => ( ) +WithRowAction.parameters = { + spec: ['./spec/row-actions.md#row-actions'], +} + export const WithRowActions = () => ( @@ -1215,6 +1226,10 @@ export const WithOverflow = () => (
) +WithOverflow.parameters = { + spec: ['./spec/default.md#default'], +} + const columnHelper = createColumnHelper() const columns = [ columnHelper.column({ @@ -1293,6 +1308,10 @@ export const WithLoading = () => { ) } +WithLoading.parameters = { + spec: ['./spec/loading-and-errors.md#loading-and-errors'], +} + export const WithPlaceholderCells = () => ( @@ -1584,9 +1603,13 @@ export const WithPagination = () => { ) } +WithPagination.parameters = { + spec: ['./spec/pagination.md#pagination'], +} + export const WithPaginationUsingDefaultPageIndex = () => { const pageSize = 10 - const [pageIndex, setPageIndex] = React.useState(0) + const [pageIndex, setPageIndex] = React.useState(49) const start = pageIndex * pageSize const end = start + pageSize const rows = repos.slice(start, end) @@ -1664,6 +1687,10 @@ export const WithPaginationUsingDefaultPageIndex = () => { ) } +WithPaginationUsingDefaultPageIndex.parameters = { + spec: ['./spec/pagination.md#pagination'], +} + export const WithNetworkError = () => { const pageSize = 10 const [pageIndex, setPageIndex] = React.useState(0) @@ -1685,7 +1712,9 @@ export const WithNetworkError = () => { A subtitle could appear here to give extra context to the data. - {loading || error ? : null} + {loading || error ? ( + + ) : null} {error ? ( { @@ -1694,7 +1723,9 @@ export const WithNetworkError = () => { onRetry={() => { action('onRetry') }} - /> + > + Repositories could not be loaded. + ) : null} {data ? ( { ) } + +WithNetworkError.parameters = { + spec: ['./spec/loading-and-errors.md#loading-and-errors', './spec/pagination.md#pagination'], +} diff --git a/packages/react/src/DataTable/DataTable.stories.tsx b/packages/react/src/DataTable/DataTable.stories.tsx index 2a930a3468c..0f2c44c36b5 100644 --- a/packages/react/src/DataTable/DataTable.stories.tsx +++ b/packages/react/src/DataTable/DataTable.stories.tsx @@ -184,6 +184,8 @@ export const Default = () => ( ) +Default.parameters = {} + export const Playground: StoryObj & ColWidthArgTypes> = { render: (args: DataTableProps & ColWidthArgTypes) => { const getColWidth = (colIndex: number) => { @@ -357,4 +359,5 @@ export const Playground: StoryObj & ColWidthArgTypes> }, ...getColumnWidthArgTypes(5), }, + parameters: {}, } diff --git a/packages/react/src/DataTable/__tests__/DataTable.test.tsx b/packages/react/src/DataTable/__tests__/DataTable.test.tsx index 14e5479529d..5735397f636 100644 --- a/packages/react/src/DataTable/__tests__/DataTable.test.tsx +++ b/packages/react/src/DataTable/__tests__/DataTable.test.tsx @@ -6,6 +6,9 @@ import type {Column} from '../column' import {createColumnHelper} from '../column' import {getGridTemplateFromColumns, useTable} from '../useTable' +/** + * @see ../spec/default.md#default + */ describe('DataTable', () => { it('should render a semantic through `data` and `columns`', () => { const columnHelper = createColumnHelper<{id: number; name: string}>() @@ -275,6 +278,9 @@ describe('DataTable', () => { } }) + /** + * @see ../spec/sorting.md#sorting + */ describe('sorting', () => { describe('initial state', () => { it('should set the default sort state of a sortable table', () => { diff --git a/packages/react/src/DataTable/__tests__/ErrorDialog.test.tsx b/packages/react/src/DataTable/__tests__/ErrorDialog.test.tsx index 0278ec8ad65..1c0f6128479 100644 --- a/packages/react/src/DataTable/__tests__/ErrorDialog.test.tsx +++ b/packages/react/src/DataTable/__tests__/ErrorDialog.test.tsx @@ -3,6 +3,9 @@ import userEvent from '@testing-library/user-event' import {render, screen} from '@testing-library/react' import {ErrorDialog} from '../ErrorDialog' +/** + * @see ../spec/loading-and-errors.md#loading-and-errors + */ describe('Table.ErrorDialog', () => { it('should use a default title of "Error" if `title` is not provided', () => { render() diff --git a/packages/react/src/DataTable/__tests__/Pagination.test.tsx b/packages/react/src/DataTable/__tests__/Pagination.test.tsx index 8a82381e118..95313447b18 100644 --- a/packages/react/src/DataTable/__tests__/Pagination.test.tsx +++ b/packages/react/src/DataTable/__tests__/Pagination.test.tsx @@ -4,6 +4,9 @@ import {Pagination} from '../Pagination' import {act, render, screen} from '@testing-library/react' import userEvent from '@testing-library/user-event' +/** + * @see ../spec/pagination.md#pagination + */ describe('Table.Pagination', () => { beforeEach(async () => { await page.viewport(1400, 728) diff --git a/packages/react/src/DataTable/__tests__/Table.test.tsx b/packages/react/src/DataTable/__tests__/Table.test.tsx index 99e76bb43e2..9e41e7fc2d1 100644 --- a/packages/react/src/DataTable/__tests__/Table.test.tsx +++ b/packages/react/src/DataTable/__tests__/Table.test.tsx @@ -159,6 +159,9 @@ describe('Table', () => { }) }) + /** + * @see ../spec/header-and-actions.md#header-and-actions + */ describe('Table.Title', () => { it('should default to rendering a level 2 heading', () => { render(test) @@ -269,6 +272,9 @@ describe('Table', () => { }) }) + /** + * @see ../spec/loading-and-errors.md#loading-and-errors + */ describe('Table.Skeleton', () => { implementsClassName(props => ) diff --git a/packages/react/src/DataTable/spec/README.md b/packages/react/src/DataTable/spec/README.md new file mode 100644 index 00000000000..61e5b9ed0bd --- /dev/null +++ b/packages/react/src/DataTable/spec/README.md @@ -0,0 +1,29 @@ +# DataTable spec + +DataTable presents a collection of records as rows and the fields for each +record as columns. It supports sortable columns and may be composed with +pagination controls, actions, and loading or error states. + +## Accessibility + +- DataTable MUST provide native table semantics and the relationships between + column headers, row headers, and data cells. +- Consumers MUST provide an accessible name by referencing a title with + `aria-labelledby`. The referenced title MAY be rendered with `Table.Title`, + visually hidden, or provided elsewhere on the page. +- Consumers MAY reference supplementary text with `aria-describedby`. +- When horizontal overflow is detected, DataTable MUST automatically make its + scroll container a focusable region named by the table title. + +## Features + +- [Default](./default.md) +- [Header and actions](./header-and-actions.md) +- [Sorting](./sorting.md) +- [Pagination](./pagination.md) +- [Row actions](./row-actions.md) +- [Loading and errors](./loading-and-errors.md) + +## Log + + diff --git a/packages/react/src/DataTable/spec/default.md b/packages/react/src/DataTable/spec/default.md new file mode 100644 index 00000000000..c8daf4fb2b3 --- /dev/null +++ b/packages/react/src/DataTable/spec/default.md @@ -0,0 +1,61 @@ +# Default + +DataTable renders structured row data from a collection of column definitions. + +## Markup + +```html +
+ + + + + + + + + + + + + + +
Column headerRow identifier
Cell valueRow identifier value
+``` + +- DataTable MUST render a semantic `table` containing a header row and a body + row for each item in the data collection. +- DataTable MUST render one column header row. +- Each column MUST render a `th` with `scope="col"` in the header row. +- Each data value MUST render as a `td` unless its column is identified as a row + header. +- A row header MUST render as a `th` with `scope="row"`. +- The header and body MUST render as separate row groups. + +## Behavior + +- Columns MUST render in the order in which they are provided. +- Rows MUST render in the order in which they are provided unless the + [sorting](./sorting.md) feature changes that order. +- A column with a field MUST read the corresponding value from each row, + including values referenced by a nested field path. +- A custom cell renderer MUST receive the complete row and its rendered result + MUST replace the field value for that cell. +- The row identifier returned for each row MUST be used as its stable identity. +- An empty data collection MUST render the column header row with no body rows. + DataTable MUST NOT substitute a loading or empty-state presentation. + +## Public API + +- Every column MUST define either a field or an explicit identifier. +- A column without a field MUST provide a custom cell renderer. +- When both a field and custom cell renderer are provided, the custom renderer + MUST determine the rendered cell content. +- Column definitions represent one header and one cell per row. Grouped headers + and cells that span rows or columns are not supported. +- `Table` MUST forward its ref to the semantic `table` element. + +## Accessibility + +- The table, header row group, body row group, rows, column headers, row + headers, and cells MUST expose their corresponding table roles. diff --git a/packages/react/src/DataTable/spec/header-and-actions.md b/packages/react/src/DataTable/spec/header-and-actions.md new file mode 100644 index 00000000000..dd66c6f6ee0 --- /dev/null +++ b/packages/react/src/DataTable/spec/header-and-actions.md @@ -0,0 +1,46 @@ +# Header and actions + +The DataTable header provides the table's title, optional supplementary +description, and optional actions that apply to the table as a whole. + +## Markup + +```html +

Table title

+

Supplementary description

+ +
+ + +
+``` + +- `Table.Title` MUST render an `h2` by default and MUST place its identifier on + that heading. +- `Table.Subtitle` MUST place its identifier on its rendered element. +- Consumers MUST place `Table.Actions` outside of the semantic `table`. +- A divider between header content and the table MUST be presentational. + +## Behavior + +- `Table.Title` MUST allow consumers to select a heading level appropriate for + the surrounding page. +- `Table.Subtitle` MUST allow consumers to select the semantic element used for + supplementary content. + +## Public API + +- `Table.Title` and `Table.Subtitle` MUST NOT automatically label or describe + DataTable. Consumers MUST reference their identifiers with + `aria-labelledby` and `aria-describedby`, respectively. +- A heading rendered elsewhere on the page MAY be referenced instead of using + `Table.Title`. +- `Table.Title` MUST forward its ref to the rendered title element. + +## Accessibility + +- Consumers MUST ensure that the referenced title is available to assistive + technologies. +- `Table.Actions` does not assign group semantics or accessible names. + Consumers MUST provide accessible names for action controls and any grouping + required by the surrounding interface. diff --git a/packages/react/src/DataTable/spec/loading-and-errors.md b/packages/react/src/DataTable/spec/loading-and-errors.md new file mode 100644 index 00000000000..b4f35df2387 --- /dev/null +++ b/packages/react/src/DataTable/spec/loading-and-errors.md @@ -0,0 +1,61 @@ +# Loading and errors + +Loading and error states communicate that table content is unavailable while it +is being retrieved or could not be retrieved. + +## Markup + +```html + + + + + + + + + + + + + +
Column header
Loading...
+ + +
+

Error title

+

Error description

+ + +
+``` + +- `Table.Skeleton` MUST render a semantic table with the provided column + headers. +- Each skeleton cell MUST expose loading text to assistive technologies. +- `Table.ErrorDialog` MUST render an alert dialog with Retry and Dismiss + controls. + +## Behavior + +- `Table.Skeleton` SHOULD be rendered only while data is actively being fetched; consumers MUST replace it with `DataTable` once data is available. +- `Table.ErrorDialog` SHOULD be rendered only when a data fetch or operation fails; consumers MUST remove it once the error is resolved or the user dismisses it. +- The configured row count MUST determine the number of visual placeholders in + each skeleton column; it MUST NOT create that number of semantic table rows. +- Retrying an error MUST invoke the retry handler. Dismissing an error MUST + invoke the dismiss handler. + +## Public API + +- DataTable MUST NOT infer loading or error state from its data. Consumers MUST + compose `Table.Skeleton` or `Table.ErrorDialog` around DataTable when needed. + +## Accessibility + +- Consumers MUST provide the same accessible naming and description + relationships to a skeleton that they provide to the loaded DataTable. +- Loading animation MUST respect the user's reduced-motion preference. +- An error dialog MUST preserve the keyboard and focus behavior of the dialog + primitive. +- When an error dialog opens, focus MUST move to the dialog. When the dialog is + dismissed, focus MUST return to a logical element in the page. diff --git a/packages/react/src/DataTable/spec/pagination.md b/packages/react/src/DataTable/spec/pagination.md new file mode 100644 index 00000000000..fb32001ccbd --- /dev/null +++ b/packages/react/src/DataTable/spec/pagination.md @@ -0,0 +1,80 @@ +# Pagination + +Pagination provides controls for navigating a DataTable whose records are +divided into pages. + +## Markup + +```html + +``` + +- Pagination MUST render a navigation landmark with an accessible name that + identifies the related DataTable. +- Pagination MUST display the range of records represented by the selected + page and the total record count. +- Previous-page, next-page, and visible page options MUST render as native + buttons. +- When page options are rendered, the selected page button MUST expose + `aria-current`. +- Unavailable previous-page and next-page controls MUST expose + `aria-disabled="true"`. +- A visual truncation indicator for omitted page options MUST be hidden from + assistive technologies. +- The page button immediately before a truncated range MUST expose an ellipsis + as a continuation cue to assistive technologies. + +## Behavior + +- Page indexes MUST be zero-based. +- The initial selected page MUST be the first page unless a valid default page + index is provided. +- The number of pages MUST be derived from the total record count and page + size. +- Activating a different page, the previous-page control, or the next-page + control MUST update the selected page and invoke the change callback with the + new page index. +- Activating the selected page or an unavailable previous-page or next-page + control MUST NOT invoke the change callback. +- Long page ranges MAY replace omitted page options with truncation indicators. +- Page-number buttons MAY be shown or hidden per viewport width range. When + hidden at a given viewport width, only the Previous and Next controls and the + record-range summary remain visible. + +## Public API + +- Pagination MUST manage page selection without changing DataTable rows. + Consumers MUST use the page index reported by the change callback to provide + the records for the selected page. +- The default page index MUST establish uncontrolled initial state. Consumers + MUST initialize the displayed records from that index because the change + callback MUST NOT be invoked for the initial render. +- The default page index MUST be greater than or equal to zero and less than the + number of pages. An invalid initial value MUST produce a warning and fall back + to the first page. +- Changing to a different valid default page index after the initial render + MUST update the selected page and invoke the change callback. +- Changing to an invalid default page index after the initial render MUST NOT + change the selected page or invoke the change callback. +- Consumers MUST compose Pagination with the DataTable it controls and place it + outside of the semantic `table` element. + +## Accessibility + +- Each page button MUST have an accessible name that identifies its page. +- Previous-page and next-page controls MUST have accessible names that describe + their destination. +- A page change MUST announce the displayed record range and total record count. diff --git a/packages/react/src/DataTable/spec/row-actions.md b/packages/react/src/DataTable/spec/row-actions.md new file mode 100644 index 00000000000..62bcea1e1ba --- /dev/null +++ b/packages/react/src/DataTable/spec/row-actions.md @@ -0,0 +1,50 @@ +# Row actions + +Row actions allow consumers to perform operations whose scope is one DataTable +row by rendering controls in a custom cell. + +## Markup + +```html + + + + + + + + + + + + + + + +
NameActions
Repository name + +
+``` + +- DataTable MUST render row-action controls inside the cell produced by the + column's custom cell renderer. +- The row-action column MUST have a column header like every other column. + +## Behavior + +- DataTable MUST pass the complete row to the custom cell renderer. +- DataTable MUST NOT handle row-action activation or menu state. + +## Public API + +- A fieldless row-action column MUST define an explicit identifier and a custom + cell renderer. + +## Accessibility + +- Consumers SHOULD place row actions in the final column and provide a visible + or visually hidden heading that identifies the column as actions. +- Consumers MUST give each repeated row action an accessible name containing + the operation and enough row-specific context to identify the affected row. +- Consumers are responsible for the keyboard and focus behavior of controls + rendered inside a custom cell. diff --git a/packages/react/src/DataTable/spec/sorting.md b/packages/react/src/DataTable/spec/sorting.md new file mode 100644 index 00000000000..303ca1df7cf --- /dev/null +++ b/packages/react/src/DataTable/spec/sorting.md @@ -0,0 +1,83 @@ +# Sorting + +Sorting allows consumers to identify sortable columns and order rows by one +column at a time. + +## Markup + +```html + + + + + + + + + + + + +
+ + + + Type
+``` + +- A sortable column header MUST contain a native button that includes the + column heading. +- The active sorted column header MUST expose `aria-sort="ascending"` or + `aria-sort="descending"` to match its current direction. +- Column headers that are not actively sorted MUST NOT expose `aria-sort`. + +## Behavior + +- Activating an unsorted column MUST sort it in ascending order. +- Activating the currently sorted column MUST alternate between ascending and + descending order. Sorting MUST NOT cycle through an unsorted state. +- Activating a different sortable column MUST make it the active column and + start it in ascending order. +- Built-in sorting MUST support basic value comparison, natural alphanumeric + comparison, and date-time comparison. +- A custom sort function MUST receive complete row values rather than only the + column field values when sorting a field-backed column. +- For built-in field-based sorting, blank values (`null`, `undefined`, and the + empty string) MUST appear after populated values in both sort directions. +- When external sorting is enabled, activating a sortable column MUST update + the displayed sort direction and invoke the sort callback without reordering + the provided rows. +- DataTable MUST maintain sort state for at most one column at a time. +- Internal sorting MUST reorder only the rows in the provided data collection. +- When the columns change and no longer include the active sort column, + DataTable MUST clear the sort state. +- When the data collection changes, internal sorting MUST apply the current sort + state to the new rows. + +## Public API + +- An initial sort column MUST correspond to the identifier or field of a + sortable column. +- When an initial sort column is provided without an initial direction, the + direction MUST default to ascending. +- When only an initial direction is provided, it MUST apply to the first + sortable column. +- Initial sort options describe the order of the provided data; consumers MUST + provide the data in that initial order. +- Initial sort options MUST establish uncontrolled initial state. Changes to + those options after the initial render MUST NOT control the active sort + column or direction. +- The sort callback MUST receive the active column identifier and its new + ascending or descending direction after an interaction. It MUST NOT be called + for the initial render. +- When external sorting is enabled, consumers MUST use the sort callback to + provide the reordered data. + +## Accessibility + +- Sort controls MUST be operable with pointer input and with the Enter and Space + keys. +- An unsorted sortable column MUST communicate that activating its control will + sort the column in ascending order. +- The active sort direction MUST be programmatically determinable from the + column header. diff --git a/packages/spec/README.md b/packages/spec/README.md new file mode 100644 index 00000000000..8249371936c --- /dev/null +++ b/packages/spec/README.md @@ -0,0 +1,57 @@ +# `@primer/spec` + +Utilities for parsing and validating Primer component specifications. + +The package supports both component spec layouts: + +```text +Component/ +└── SPEC.md +``` + +```text +Component/ +└── spec/ + ├── README.md + ├── default.md + └── feature.md +``` + +## Usage + +Parse and validate one component: + +```ts +import {loadComponentSpec} from '@primer/spec' + +const result = await loadComponentSpec('packages/react/src/Component') + +if (!result.valid) { + for (const diagnostic of result.diagnostics) { + console.error( + `${diagnostic.location.path}:${diagnostic.location.line}:${diagnostic.location.column} ${diagnostic.message}`, + ) + } +} + +console.log(result.value) +``` + +Find and validate every component spec below a directory: + +```ts +import {validateComponentSpecs} from '@primer/spec' + +const result = await validateComponentSpecs('packages/react/src') +``` + +Pure Markdown parsers are also exported for tooling that already owns file +loading: + +```ts +import {parseComponentSpec, parseFeatureSpec, parseSpecIndex} from '@primer/spec' +``` + +Parsing always returns a normalized value when enough structure is available, +along with source-located diagnostics. `valid` is `false` when any error +diagnostic is present. diff --git a/packages/spec/package.json b/packages/spec/package.json new file mode 100644 index 00000000000..27a2af5d9da --- /dev/null +++ b/packages/spec/package.json @@ -0,0 +1,21 @@ +{ + "name": "@primer/spec", + "type": "module", + "version": "0.0.0", + "private": true, + "exports": "./src/index.ts", + "scripts": { + "test": "vitest --run", + "type-check": "tsc --noEmit" + }, + "dependencies": { + "mdast-util-from-markdown": "2.0.1", + "mdast-util-to-string": "4.0.0", + "unist-util-visit": "5.0.0" + }, + "devDependencies": { + "@primer/vitest-config": "^0.0.0", + "typescript": "^6.0.3" + }, + "sideEffects": false +} diff --git a/packages/spec/src/__fixtures__/ambiguous/SPEC.md b/packages/spec/src/__fixtures__/ambiguous/SPEC.md new file mode 100644 index 00000000000..60b72fe3cfc --- /dev/null +++ b/packages/spec/src/__fixtures__/ambiguous/SPEC.md @@ -0,0 +1,13 @@ +# Ambiguous spec + +Ambiguous has both supported component spec layouts. + +## Features + +### Default + +The default feature exists in the single-file spec. + +#### Behavior + +The feature MUST be declared once. diff --git a/packages/spec/src/__fixtures__/ambiguous/spec/README.md b/packages/spec/src/__fixtures__/ambiguous/spec/README.md new file mode 100644 index 00000000000..9c56e2d7e00 --- /dev/null +++ b/packages/spec/src/__fixtures__/ambiguous/spec/README.md @@ -0,0 +1,7 @@ +# Ambiguous spec + +Ambiguous has both supported component spec layouts. + +## Features + +- [Default](./default.md) diff --git a/packages/spec/src/__fixtures__/ambiguous/spec/default.md b/packages/spec/src/__fixtures__/ambiguous/spec/default.md new file mode 100644 index 00000000000..6434d204c39 --- /dev/null +++ b/packages/spec/src/__fixtures__/ambiguous/spec/default.md @@ -0,0 +1,7 @@ +# Default + +The default feature exists in the split spec. + +## Behavior + +The feature MUST be declared once. diff --git a/packages/spec/src/__fixtures__/invalid-split/spec/README.md b/packages/spec/src/__fixtures__/invalid-split/spec/README.md new file mode 100644 index 00000000000..71750c4f022 --- /dev/null +++ b/packages/spec/src/__fixtures__/invalid-split/spec/README.md @@ -0,0 +1,10 @@ +# InvalidExample spec + +InvalidExample is used to verify split-spec diagnostics. + +## Features + +- [Default](./default.md) +- [Missing feature](./missing.md) +- [Wrong label](./orphan.md) +- [External feature](https://example.com/feature.md) diff --git a/packages/spec/src/__fixtures__/invalid-split/spec/default.md b/packages/spec/src/__fixtures__/invalid-split/spec/default.md new file mode 100644 index 00000000000..61e07a7ba07 --- /dev/null +++ b/packages/spec/src/__fixtures__/invalid-split/spec/default.md @@ -0,0 +1,7 @@ +# Default + +The default feature exists. + +## Behavior + +The feature MUST behave predictably. diff --git a/packages/spec/src/__fixtures__/invalid-split/spec/orphan.md b/packages/spec/src/__fixtures__/invalid-split/spec/orphan.md new file mode 100644 index 00000000000..a959b5e9055 --- /dev/null +++ b/packages/spec/src/__fixtures__/invalid-split/spec/orphan.md @@ -0,0 +1,7 @@ +# Orphan + +This feature is not linked from the spec index. + +## Behavior + +The feature MUST be reported as an orphan. diff --git a/packages/spec/src/__fixtures__/invalid-split/spec/unused.md b/packages/spec/src/__fixtures__/invalid-split/spec/unused.md new file mode 100644 index 00000000000..a3e8426fb54 --- /dev/null +++ b/packages/spec/src/__fixtures__/invalid-split/spec/unused.md @@ -0,0 +1,7 @@ +# Unused + +This feature is not linked from the spec index. + +## Behavior + +The feature MUST be reported as an orphan. diff --git a/packages/spec/src/__fixtures__/single/SPEC.md b/packages/spec/src/__fixtures__/single/SPEC.md new file mode 100644 index 00000000000..3420496cc7a --- /dev/null +++ b/packages/spec/src/__fixtures__/single/SPEC.md @@ -0,0 +1,39 @@ + + +# Example spec + +Example communicates a small piece of status information. + +## Accessibility + +The component MUST expose its status to assistive technologies. + +## Features + +### Default + +The default feature renders the current status. + +#### Markup + +The component MUST render a native status element. + +#### Behavior + +The component updates when its content changes. + +### Delayed appearance + +The component can avoid displaying short-lived status changes. + +#### Behavior + +The component MUST remain unmounted until the configured delay elapses. + +#### Accessibility + +Assistive text MUST appear at the same time as the visible status. + +## Glossary + +Status refers to the current state of an operation. diff --git a/packages/spec/src/__fixtures__/split/spec/README.md b/packages/spec/src/__fixtures__/split/spec/README.md new file mode 100644 index 00000000000..f4cb29bdd35 --- /dev/null +++ b/packages/spec/src/__fixtures__/split/spec/README.md @@ -0,0 +1,16 @@ +# Example spec + +Example presents a menu of actions from a trigger. + +## Accessibility + +The trigger MUST expose whether the menu is expanded. + +## Features + +- [Default](./default.md) +- [Submenus](./submenus.md) + +## Glossary + +Trigger refers to the element that opens the menu. diff --git a/packages/spec/src/__fixtures__/split/spec/default.md b/packages/spec/src/__fixtures__/split/spec/default.md new file mode 100644 index 00000000000..0264d4496ef --- /dev/null +++ b/packages/spec/src/__fixtures__/split/spec/default.md @@ -0,0 +1,11 @@ +# Default + +The default feature opens one menu from one trigger. + +## Markup + +The menu MUST be labelled by its trigger. + +## Behavior + +Activating the trigger MUST open the menu. diff --git a/packages/spec/src/__fixtures__/split/spec/submenus.md b/packages/spec/src/__fixtures__/split/spec/submenus.md new file mode 100644 index 00000000000..c5c714113a5 --- /dev/null +++ b/packages/spec/src/__fixtures__/split/spec/submenus.md @@ -0,0 +1,11 @@ +# Submenus + +Submenus reveal a related set of actions from a menu item. + +## Behavior + +Arrow Right MUST open the focused submenu. + +## Accessibility + +The submenu trigger MUST expose its expanded state. diff --git a/packages/spec/src/index.test.ts b/packages/spec/src/index.test.ts new file mode 100644 index 00000000000..503e2d4f47d --- /dev/null +++ b/packages/spec/src/index.test.ts @@ -0,0 +1,235 @@ +import {fileURLToPath} from 'node:url' +import {describe, expect, it} from 'vitest' +import {findComponentSpecs, loadComponentSpec, parseComponentSpec, parseFeatureSpec, validateComponentSpecs} from '.' + +const fixturesPath = fileURLToPath(new URL('__fixtures__', import.meta.url)) + +describe('parseComponentSpec', () => { + it('parses a feature-first component spec', () => { + const result = parseComponentSpec(` +# Example spec + +Example communicates status. + +## Accessibility + +The component MUST expose its status. + +## Features + +### Default + +The default feature renders the status. + +#### Markup + +The component MUST render a status element. + +#### Public API + +When both values are provided, the controlled value MUST take precedence. + +### Delayed appearance + +The component can delay short-lived status. + +#### Behavior + +The component MUST remain unmounted until the delay elapses. +`) + + expect(result.valid).toBe(true) + expect(result.diagnostics).toEqual([]) + expect(result.value).toMatchObject({ + name: 'Example', + description: 'Example communicates status.', + format: 'single', + features: [ + { + name: 'Default', + sections: { + markup: { + markdown: 'The component MUST render a status element.', + }, + publicApi: { + markdown: 'When both values are provided, the controlled value MUST take precedence.', + }, + }, + }, + { + name: 'Delayed appearance', + sections: { + behavior: { + markdown: 'The component MUST remain unmounted until the delay elapses.', + }, + }, + }, + ], + }) + }) + + it('reports unsupported structure and forbidden sections', () => { + const result = parseComponentSpec(` +# Example spec + +Example communicates status. + +## Features + +### Loading + +The loading feature renders status. + +#### API + +The API is documented here. + +## Verification + +The tests are listed here. + +## SSR + +The component renders on the server. +`) + + expect(result.valid).toBe(false) + const codes = result.diagnostics.map(diagnostic => diagnostic.code) + expect(codes).toEqual( + expect.arrayContaining(['missing-default-feature', 'unexpected-heading', 'forbidden-heading']), + ) + expect(codes.filter(code => code === 'forbidden-heading')).toHaveLength(2) + expect(result.diagnostics.filter(diagnostic => diagnostic.message.includes('Verification'))).toHaveLength(1) + }) + + it('requires a component title and description', () => { + const result = parseComponentSpec(` +# Example + +## Features + +### Default + +The default feature exists. + +#### Behavior + +The feature MUST behave predictably. +`) + + expect(result.valid).toBe(false) + expect(result.diagnostics.map(diagnostic => diagnostic.code)).toEqual( + expect.arrayContaining(['invalid-title', 'empty-description']), + ) + }) +}) + +describe('parseFeatureSpec', () => { + it('parses a split feature file', () => { + const result = parseFeatureSpec(` +# Submenus + +Submenus reveal related actions. + +## Behavior + +Arrow Right MUST open the focused submenu. + +## Accessibility + +The trigger MUST expose its expanded state. +`) + + expect(result.valid).toBe(true) + expect(result.value).toMatchObject({ + name: 'Submenus', + sections: { + behavior: { + markdown: 'Arrow Right MUST open the focused submenu.', + }, + accessibility: { + markdown: 'The trigger MUST expose its expanded state.', + }, + }, + }) + }) +}) + +describe('loadComponentSpec', () => { + it('loads a single-file component spec from its component directory', async () => { + const result = await loadComponentSpec(new URL('__fixtures__/single', import.meta.url).pathname) + + expect(result.valid).toBe(true) + expect(result.value).toMatchObject({ + name: 'Example', + format: 'single', + features: [{name: 'Default'}, {name: 'Delayed appearance'}], + }) + }) + + it('loads and combines a split component spec', async () => { + const result = await loadComponentSpec(new URL('__fixtures__/split', import.meta.url).pathname) + + expect(result.valid).toBe(true) + expect(result.value).toMatchObject({ + name: 'Example', + format: 'split', + features: [{name: 'Default'}, {name: 'Submenus'}], + }) + expect(result.value?.files).toHaveLength(3) + }) + + it('reports missing and orphaned split feature files', async () => { + const result = await loadComponentSpec(new URL('__fixtures__/invalid-split', import.meta.url).pathname) + const codes = result.diagnostics.map(diagnostic => diagnostic.code) + + expect(result.valid).toBe(false) + expect(codes).toContain('missing-feature-file') + expect(codes).toContain('invalid-feature-link') + expect(codes).toContain('feature-title-mismatch') + expect(codes).toContain('orphan-feature-file') + }) + + it('rejects component directories with both spec layouts', async () => { + const result = await loadComponentSpec(new URL('__fixtures__/ambiguous', import.meta.url).pathname) + + expect(result.valid).toBe(false) + expect(result.value).toBeNull() + expect(result.diagnostics).toEqual([ + expect.objectContaining({ + code: 'ambiguous-spec', + }), + ]) + }) + + it('reports paths without a component spec', async () => { + const result = await loadComponentSpec(new URL('__fixtures__/missing', import.meta.url).pathname) + + expect(result.valid).toBe(false) + expect(result.diagnostics).toEqual([ + expect.objectContaining({ + code: 'file-not-found', + }), + ]) + }) +}) + +describe('component spec discovery', () => { + it('finds single-file and split component specs', async () => { + const entries = await findComponentSpecs(fixturesPath) + + expect(entries).toHaveLength(4) + expect(entries.some(entry => entry.endsWith('/single'))).toBe(true) + expect(entries.some(entry => entry.endsWith('/split'))).toBe(true) + expect(entries.some(entry => entry.endsWith('/ambiguous'))).toBe(true) + }) + + it('validates every discovered component spec', async () => { + const result = await validateComponentSpecs(fixturesPath) + + expect(result.valid).toBe(false) + expect(result.specs).toHaveLength(3) + expect(result.diagnostics.some(diagnostic => diagnostic.code === 'orphan-feature-file')).toBe(true) + expect(result.diagnostics.some(diagnostic => diagnostic.code === 'ambiguous-spec')).toBe(true) + }) +}) diff --git a/packages/spec/src/index.ts b/packages/spec/src/index.ts new file mode 100644 index 00000000000..d4369faca26 --- /dev/null +++ b/packages/spec/src/index.ts @@ -0,0 +1,18 @@ +export {findComponentSpecs, loadComponentSpec, validateComponentSpecs} from './loader' +export {parseComponentSpec, parseFeatureSpec, parseSpecIndex} from './parser' +export type { + ComponentSpec, + ComponentSpecFormat, + FindComponentSpecsOptions, + SpecCollectionResult, + SpecDiagnostic, + SpecDiagnosticCode, + SpecFeature, + SpecFeatureLink, + SpecFeatureSections, + SpecIndex, + SpecLocation, + SpecParseResult, + SpecSection, + SpecSeverity, +} from './types' diff --git a/packages/spec/src/loader.ts b/packages/spec/src/loader.ts new file mode 100644 index 00000000000..7aaa319df60 --- /dev/null +++ b/packages/spec/src/loader.ts @@ -0,0 +1,330 @@ +import {readdir, readFile, stat} from 'node:fs/promises' +import path from 'node:path' +import {createSplitComponentSpec, parseComponentSpec, parseFeatureSpec, parseSpecIndex} from './parser' +import type {FindComponentSpecsOptions, SpecCollectionResult, SpecDiagnostic, SpecParseResult} from './types' + +const defaultIgnoredDirectories = new Set(['.agents', '.git', 'dist', 'node_modules']) + +export async function loadComponentSpec(inputPath: string): Promise { + const resolvedInput = path.resolve(inputPath) + const entry = await resolveSpecEntry(resolvedInput) + + if (!entry.valid) { + return { + valid: false, + value: null, + diagnostics: entry.diagnostics, + } + } + + if (entry.format === 'single') { + const source = await readMarkdown(entry.path) + if (!source.valid || source.value === null) { + return { + valid: false, + value: null, + diagnostics: source.diagnostics, + } + } + return parseComponentSpec(source.value, entry.path) + } + + return loadSplitComponentSpec(entry.path) +} + +export async function findComponentSpecs( + rootPath: string, + options: FindComponentSpecsOptions = {}, +): Promise { + const root = path.resolve(rootPath) + const ignored = new Set([...defaultIgnoredDirectories, ...(options.ignore ?? [])]) + const entries: string[] = [] + + await visitDirectory(root, ignored, entries) + return entries.sort() +} + +export async function validateComponentSpecs( + rootPath: string, + options: FindComponentSpecsOptions = {}, +): Promise { + const entries = await findComponentSpecs(rootPath, options) + const results = await Promise.all(entries.map(entry => loadComponentSpec(entry))) + const specs = results.flatMap(result => (result.value === null ? [] : [result.value])) + const diagnostics = results.flatMap(result => result.diagnostics) + + return { + valid: results.every(result => result.valid), + specs, + diagnostics, + } +} + +async function loadSplitComponentSpec(indexPath: string): Promise { + const indexSource = await readMarkdown(indexPath) + if (!indexSource.valid || indexSource.value === null) { + return { + valid: false, + value: null, + diagnostics: indexSource.diagnostics, + } + } + + const indexResult = parseSpecIndex(indexSource.value, indexPath) + const diagnostics: SpecDiagnostic[] = [...indexResult.diagnostics] + if (indexResult.value === null) { + return { + valid: false, + value: null, + diagnostics, + } + } + + const indexDirectory = path.dirname(indexPath) + const features = [] + const featurePaths: string[] = [] + + for (const link of indexResult.value.features) { + const resolvedLink = resolveFeatureLink(indexDirectory, link.href) + if (resolvedLink === null) { + diagnostics.push({ + code: 'invalid-feature-link', + message: `The feature link “${link.href}” must reference a Markdown file in the spec directory.`, + severity: 'error', + location: link.location, + }) + continue + } + + const featureSource = await readMarkdown(resolvedLink) + if (!featureSource.valid || featureSource.value === null) { + diagnostics.push({ + code: 'missing-feature-file', + message: `The feature file “${link.href}” could not be read.`, + severity: 'error', + location: link.location, + }) + continue + } + + const featureResult = parseFeatureSpec(featureSource.value, resolvedLink) + diagnostics.push(...featureResult.diagnostics) + featurePaths.push(resolvedLink) + + if (featureResult.value === null) { + continue + } + + if (normalizeName(featureResult.value.name) !== normalizeName(link.name)) { + diagnostics.push({ + code: 'feature-title-mismatch', + message: `The feature link “${link.name}” does not match the title “${featureResult.value.name}”.`, + severity: 'error', + location: link.location, + }) + } + + features.push(featureResult.value) + } + + diagnostics.push(...(await findOrphanFeatureFiles(indexDirectory, featurePaths))) + + const spec = createSplitComponentSpec(indexResult.value, features, [indexPath, ...featurePaths]) + return { + valid: diagnostics.every(diagnostic => diagnostic.severity !== 'error'), + value: spec, + diagnostics, + } +} + +async function resolveSpecEntry(inputPath: string): Promise< + | { + readonly valid: true + readonly path: string + readonly format: 'single' | 'split' + readonly diagnostics: readonly SpecDiagnostic[] + } + | { + readonly valid: false + readonly path: null + readonly format: null + readonly diagnostics: readonly SpecDiagnostic[] + } +> { + const inputStat = await getStat(inputPath) + if (inputStat === null) { + return invalidEntry('file-not-found', `No file or directory exists at “${inputPath}”.`, inputPath) + } + + if (inputStat.isFile()) { + if (path.basename(inputPath) === 'SPEC.md') { + return {valid: true, path: inputPath, format: 'single', diagnostics: []} + } + if (path.basename(inputPath) === 'README.md' && path.basename(path.dirname(inputPath)) === 'spec') { + return {valid: true, path: inputPath, format: 'split', diagnostics: []} + } + return invalidEntry('invalid-entry', 'A component spec entry must be SPEC.md or spec/README.md.', inputPath) + } + + const directIndex = + path.basename(inputPath) === 'spec' ? path.join(inputPath, 'README.md') : path.join(inputPath, 'SPEC.md') + const splitIndex = path.basename(inputPath) === 'spec' ? directIndex : path.join(inputPath, 'spec', 'README.md') + const [hasDirectIndex, hasSplitIndex] = await Promise.all([isFile(directIndex), isFile(splitIndex)]) + + if (path.basename(inputPath) !== 'spec' && hasDirectIndex && hasSplitIndex) { + return invalidEntry( + 'ambiguous-spec', + 'The component directory contains both SPEC.md and spec/README.md.', + inputPath, + ) + } + + if (hasDirectIndex) { + return { + valid: true, + path: directIndex, + format: path.basename(inputPath) === 'spec' ? 'split' : 'single', + diagnostics: [], + } + } + + if (hasSplitIndex) { + return {valid: true, path: splitIndex, format: 'split', diagnostics: []} + } + + return invalidEntry( + 'file-not-found', + 'The component directory does not contain SPEC.md or spec/README.md.', + inputPath, + ) +} + +async function visitDirectory(directory: string, ignored: ReadonlySet, entries: string[]) { + const children = await readdir(directory, {withFileTypes: true}) + const hasSingleSpec = children.some(child => child.isFile() && child.name === 'SPEC.md') + const hasSplitSpec = await isFile(path.join(directory, 'spec', 'README.md')) + + if (hasSingleSpec || hasSplitSpec) { + entries.push(directory) + } + + await Promise.all( + children + .filter(child => child.isDirectory() && child.name !== 'spec' && !ignored.has(child.name)) + .map(child => visitDirectory(path.join(directory, child.name), ignored, entries)), + ) +} + +async function findOrphanFeatureFiles( + directory: string, + linkedFeaturePaths: readonly string[], +): Promise { + const linked = new Set(linkedFeaturePaths.map(featurePath => path.resolve(featurePath))) + const children = await readdir(directory, {withFileTypes: true}) + + return children + .filter(child => child.isFile() && child.name.endsWith('.md') && child.name !== 'README.md') + .map(child => path.resolve(directory, child.name)) + .filter(featurePath => !linked.has(featurePath)) + .map(featurePath => ({ + code: 'orphan-feature-file' as const, + message: `The feature file “${path.basename(featurePath)}” is not linked from the spec index.`, + severity: 'error' as const, + location: { + path: featurePath, + line: 1, + column: 1, + }, + })) +} + +function resolveFeatureLink(directory: string, href: string): string | null { + const [pathname] = href.split('#') + if (!pathname || path.isAbsolute(pathname) || /^[a-z][a-z\d+.-]*:/i.test(pathname)) { + return null + } + + const resolved = path.resolve(directory, pathname) + const relative = path.relative(directory, resolved) + if ( + relative.startsWith('..') || + path.isAbsolute(relative) || + path.dirname(relative) !== '.' || + path.extname(resolved) !== '.md' + ) { + return null + } + + return resolved +} + +async function readMarkdown(filePath: string): Promise> { + try { + return { + valid: true, + value: await readFile(filePath, 'utf8'), + diagnostics: [], + } + } catch (error) { + const message = error instanceof Error ? error.message : String(error) + return { + valid: false, + value: null, + diagnostics: [ + { + code: 'read-failed', + message: `Unable to read “${filePath}”: ${message}`, + severity: 'error', + location: { + path: filePath, + line: 1, + column: 1, + }, + }, + ], + } + } +} + +async function getStat(filePath: string) { + try { + return await stat(filePath) + } catch (error) { + if (isNodeError(error) && error.code === 'ENOENT') { + return null + } + throw error + } +} + +async function isFile(filePath: string): Promise { + return (await getStat(filePath))?.isFile() ?? false +} + +function invalidEntry(code: 'ambiguous-spec' | 'file-not-found' | 'invalid-entry', message: string, inputPath: string) { + return { + valid: false as const, + path: null, + format: null, + diagnostics: [ + { + code, + message, + severity: 'error' as const, + location: { + path: inputPath, + line: 1, + column: 1, + }, + }, + ], + } +} + +function normalizeName(value: string): string { + return value.trim().toLowerCase().replace(/\s+/g, ' ') +} + +function isNodeError(error: unknown): error is NodeJS.ErrnoException { + return error instanceof Error && 'code' in error +} diff --git a/packages/spec/src/parser.ts b/packages/spec/src/parser.ts new file mode 100644 index 00000000000..98e87d1693a --- /dev/null +++ b/packages/spec/src/parser.ts @@ -0,0 +1,698 @@ +import type {Heading, Link, Root} from 'mdast' +import {fromMarkdown} from 'mdast-util-from-markdown' +import {toString} from 'mdast-util-to-string' +import {visit} from 'unist-util-visit' +import type { + ComponentSpec, + SpecDiagnostic, + SpecDiagnosticCode, + SpecFeature, + SpecFeatureLink, + SpecFeatureSections, + SpecIndex, + SpecLocation, + SpecParseResult, + SpecSection, +} from './types' + +interface HeadingRecord { + readonly node: Heading + readonly childIndex: number + readonly text: string + readonly normalized: string +} + +interface ParsedDocument { + readonly root: Root + readonly headings: readonly HeadingRecord[] +} + +const topLevelHeadings = new Set(['accessibility', 'features', 'glossary']) +const featureSectionHeadings = new Set(['markup', 'behavior', 'public api', 'accessibility']) +const forbiddenHeadings = new Set(['verification', 'ssr', 'server rendering', 'server-side rendering']) + +export function parseComponentSpec(source: string, path = 'SPEC.md'): SpecParseResult { + const document = parseDocument(source) + const diagnostics: SpecDiagnostic[] = [] + validateForbiddenHeadings(document.headings, path, diagnostics) + + const title = validateDocumentTitle(document, source, path, diagnostics) + const componentName = title === null ? null : parseComponentName(title, path, diagnostics) + const description = title === null ? '' : getIntroMarkdown(source, document.root, title) + + if (title !== null && isEmptyMarkdown(description)) { + diagnostics.push( + createDiagnostic('empty-description', 'The component spec must include a description.', path, title), + ) + } + + const topLevel = document.headings.filter(heading => heading.node.depth === 2) + validateAllowedHeadings(topLevel, topLevelHeadings, path, diagnostics) + + const accessibilityHeading = findUniqueHeading(topLevel, 'accessibility', path, diagnostics) + const featuresHeading = findUniqueHeading(topLevel, 'features', path, diagnostics) + const glossaryHeading = findUniqueHeading(topLevel, 'glossary', path, diagnostics) + + if (featuresHeading === null) { + diagnostics.push( + createDiagnostic( + 'missing-features', + 'The component spec must include a level-two Features section.', + path, + title, + ), + ) + } + + const features = + featuresHeading === null + ? [] + : parseFeatures(source, document.root, document.headings, featuresHeading, path, diagnostics) + + validateDefaultFeature(features, path, featuresHeading ?? title, diagnostics) + + const spec = + componentName === null + ? null + : { + name: componentName, + description, + accessibility: createOptionalSection(source, document.root, accessibilityHeading, path, diagnostics), + features, + glossary: createOptionalSection(source, document.root, glossaryHeading, path, diagnostics), + format: 'single' as const, + entryPath: path, + files: [path], + } + + return createResult(spec, diagnostics) +} + +export function parseSpecIndex(source: string, path = 'spec/README.md'): SpecParseResult { + const document = parseDocument(source) + const diagnostics: SpecDiagnostic[] = [] + validateForbiddenHeadings(document.headings, path, diagnostics) + + const title = validateDocumentTitle(document, source, path, diagnostics) + const componentName = title === null ? null : parseComponentName(title, path, diagnostics) + const description = title === null ? '' : getIntroMarkdown(source, document.root, title) + + if (title !== null && isEmptyMarkdown(description)) { + diagnostics.push(createDiagnostic('empty-description', 'The spec index must include a description.', path, title)) + } + + const topLevel = document.headings.filter(heading => heading.node.depth === 2) + validateAllowedHeadings(topLevel, topLevelHeadings, path, diagnostics) + + const accessibilityHeading = findUniqueHeading(topLevel, 'accessibility', path, diagnostics) + const featuresHeading = findUniqueHeading(topLevel, 'features', path, diagnostics) + const glossaryHeading = findUniqueHeading(topLevel, 'glossary', path, diagnostics) + + if (featuresHeading === null) { + diagnostics.push( + createDiagnostic('missing-features', 'The spec index must include a level-two Features section.', path, title), + ) + } + + const featureLinks = + featuresHeading === null ? [] : parseFeatureLinks(document.root, featuresHeading, path, diagnostics) + + validateDefaultFeatureLinks(featureLinks, path, featuresHeading ?? title, diagnostics) + + const index = + componentName === null + ? null + : { + name: componentName, + description, + accessibility: createOptionalSection(source, document.root, accessibilityHeading, path, diagnostics), + features: featureLinks, + glossary: createOptionalSection(source, document.root, glossaryHeading, path, diagnostics), + path, + } + + return createResult(index, diagnostics) +} + +export function parseFeatureSpec(source: string, path = 'spec/feature.md'): SpecParseResult { + const document = parseDocument(source) + const diagnostics: SpecDiagnostic[] = [] + validateForbiddenHeadings(document.headings, path, diagnostics) + + const title = validateFeatureTitle(document, path, diagnostics) + const description = title === null ? '' : getIntroMarkdown(source, document.root, title) + + if (title !== null && isEmptyMarkdown(description)) { + diagnostics.push(createDiagnostic('empty-description', 'The feature spec must include a description.', path, title)) + } + + const sectionHeadings = document.headings.filter(heading => heading.node.depth === 2) + validateAllowedHeadings(sectionHeadings, featureSectionHeadings, path, diagnostics) + const sections = parseFeatureSections(source, document.root, sectionHeadings, path, diagnostics) + + if (sectionHeadings.length === 0) { + diagnostics.push( + createDiagnostic( + 'unexpected-heading', + 'The feature spec must include at least one Markup, Behavior, Public API, or Accessibility section.', + path, + title, + ), + ) + } + + const feature = + title === null + ? null + : { + name: title.text, + description, + sections, + location: createLocation(path, title), + path, + } + + return createResult(feature, diagnostics) +} + +export function createSplitComponentSpec( + index: SpecIndex, + features: readonly SpecFeature[], + files: readonly string[], +): ComponentSpec { + return { + name: index.name, + description: index.description, + accessibility: index.accessibility, + features, + glossary: index.glossary, + format: 'split', + entryPath: index.path, + files, + } +} + +function parseDocument(source: string): ParsedDocument { + const root = fromMarkdown(source) + const headings: HeadingRecord[] = [] + + for (const [childIndex, node] of root.children.entries()) { + if (node.type !== 'heading') { + continue + } + + const text = toString(node).trim() + headings.push({ + node, + childIndex, + text, + normalized: normalizeHeading(text), + }) + } + + return {root, headings} +} + +function validateDocumentTitle( + document: ParsedDocument, + source: string, + path: string, + diagnostics: SpecDiagnostic[], +): HeadingRecord | null { + const titles = document.headings.filter(heading => heading.node.depth === 1) + + if (titles.length === 0) { + diagnostics.push(createDiagnostic('missing-title', 'The spec must include a level-one title.', path)) + return null + } + + if (titles.length > 1) { + diagnostics.push( + createDiagnostic('duplicate-heading', 'The spec must include exactly one level-one title.', path, titles[1]), + ) + } + + const title = titles[0] + if (!/^.+ spec$/i.test(title.text)) { + diagnostics.push( + createDiagnostic('invalid-title', 'The component spec title must use “{Component} spec”.', path, title), + ) + } + + const contentBeforeTitle = source.slice(0, title.node.position?.start.offset ?? 0) + if (!isEmptyMarkdown(contentBeforeTitle)) { + diagnostics.push( + createDiagnostic('invalid-title', 'The level-one title must be the first content in the spec.', path, title), + ) + } + + return title +} + +function validateFeatureTitle( + document: ParsedDocument, + path: string, + diagnostics: SpecDiagnostic[], +): HeadingRecord | null { + const titles = document.headings.filter(heading => heading.node.depth === 1) + + if (titles.length === 0) { + diagnostics.push(createDiagnostic('missing-title', 'The feature spec must include a level-one title.', path)) + return null + } + + if (titles.length > 1) { + diagnostics.push( + createDiagnostic( + 'duplicate-heading', + 'The feature spec must include exactly one level-one title.', + path, + titles[1], + ), + ) + } + + return titles[0] +} + +function parseComponentName(title: HeadingRecord, path: string, diagnostics: SpecDiagnostic[]): string | null { + const match = /^(.+?) spec$/i.exec(title.text) + const name = match?.[1]?.trim() + + if (!name) { + diagnostics.push( + createDiagnostic('invalid-title', 'The component spec title must use “{Component} spec”.', path, title), + ) + return null + } + + return name +} + +function parseFeatures( + source: string, + root: Root, + headings: readonly HeadingRecord[], + featuresHeading: HeadingRecord, + path: string, + diagnostics: SpecDiagnostic[], +): readonly SpecFeature[] { + const endIndex = findSectionEndIndex(root, featuresHeading) + const featureHeadings = headings.filter( + heading => + heading.node.depth === 3 && heading.childIndex > featuresHeading.childIndex && heading.childIndex < endIndex, + ) + + if (featureHeadings.length === 0) { + diagnostics.push( + createDiagnostic( + 'missing-features', + 'The Features section must include at least one level-three feature.', + path, + featuresHeading, + ), + ) + return [] + } + + const seen = new Set() + return featureHeadings.map(featureHeading => { + if (seen.has(featureHeading.normalized)) { + diagnostics.push( + createDiagnostic( + 'duplicate-feature', + `The feature “${featureHeading.text}” is declared more than once.`, + path, + featureHeading, + ), + ) + } + seen.add(featureHeading.normalized) + + const featureEndIndex = findSectionEndIndex(root, featureHeading) + const sectionHeadings = headings.filter( + heading => + heading.node.depth === 4 && + heading.childIndex > featureHeading.childIndex && + heading.childIndex < featureEndIndex, + ) + validateAllowedHeadings(sectionHeadings, featureSectionHeadings, path, diagnostics) + + const description = getIntroMarkdown(source, root, featureHeading) + if (isEmptyMarkdown(description)) { + diagnostics.push( + createDiagnostic( + 'empty-description', + `The feature “${featureHeading.text}” must include a description.`, + path, + featureHeading, + ), + ) + } + + if (sectionHeadings.length === 0) { + diagnostics.push( + createDiagnostic( + 'unexpected-heading', + `The feature “${featureHeading.text}” must include at least one supported subsection.`, + path, + featureHeading, + ), + ) + } + + return { + name: featureHeading.text, + description, + sections: parseFeatureSections(source, root, sectionHeadings, path, diagnostics), + location: createLocation(path, featureHeading), + path, + } + }) +} + +function parseFeatureSections( + source: string, + root: Root, + headings: readonly HeadingRecord[], + path: string, + diagnostics: SpecDiagnostic[], +): SpecFeatureSections { + const sections: { + markup?: SpecSection + behavior?: SpecSection + publicApi?: SpecSection + accessibility?: SpecSection + } = {} + + for (const heading of headings) { + const key = featureSectionKey(heading.normalized) + if (key === null) { + continue + } + + if (sections[key] !== undefined) { + diagnostics.push( + createDiagnostic( + 'duplicate-heading', + `The section “${heading.text}” is declared more than once.`, + path, + heading, + ), + ) + continue + } + + const section = createSection(source, root, heading, path) + if (isEmptyMarkdown(section.markdown)) { + diagnostics.push( + createDiagnostic('empty-section', `The section “${heading.text}” must not be empty.`, path, heading), + ) + } + sections[key] = section + } + + return sections +} + +function parseFeatureLinks( + root: Root, + featuresHeading: HeadingRecord, + path: string, + diagnostics: SpecDiagnostic[], +): readonly SpecFeatureLink[] { + const links: SpecFeatureLink[] = [] + const endIndex = findSectionEndIndex(root, featuresHeading) + const nodes = root.children.slice(featuresHeading.childIndex + 1, endIndex) + + for (const node of nodes) { + visit(node, 'link', link => { + links.push(createFeatureLink(link, path)) + }) + } + + if (links.length === 0) { + diagnostics.push( + createDiagnostic( + 'missing-features', + 'The Features section must link to at least one feature file.', + path, + featuresHeading, + ), + ) + } + + const names = new Set() + const hrefs = new Set() + for (const link of links) { + const normalizedName = normalizeHeading(link.name) + if (names.has(normalizedName) || hrefs.has(link.href)) { + diagnostics.push({ + code: 'duplicate-feature', + message: `The feature link “${link.name}” is duplicated.`, + severity: 'error', + location: link.location, + }) + } + names.add(normalizedName) + hrefs.add(link.href) + } + + return links +} + +function createFeatureLink(link: Link, path: string): SpecFeatureLink { + return { + name: toString(link).trim(), + href: link.url, + location: { + path, + line: link.position?.start.line ?? 1, + column: link.position?.start.column ?? 1, + }, + } +} + +function validateDefaultFeature( + features: readonly SpecFeature[], + path: string, + heading: HeadingRecord | null, + diagnostics: SpecDiagnostic[], +) { + if (features.length > 0 && normalizeHeading(features[0].name) !== 'default') { + diagnostics.push( + createDiagnostic( + 'missing-default-feature', + 'The first feature in a component spec must be named “Default”.', + path, + heading, + ), + ) + } +} + +function validateDefaultFeatureLinks( + features: readonly SpecFeatureLink[], + path: string, + heading: HeadingRecord | null, + diagnostics: SpecDiagnostic[], +) { + if (features.length > 0 && normalizeHeading(features[0].name) !== 'default') { + diagnostics.push( + createDiagnostic( + 'missing-default-feature', + 'The first feature link in a spec index must be named “Default”.', + path, + heading, + ), + ) + } +} + +function validateForbiddenHeadings(headings: readonly HeadingRecord[], path: string, diagnostics: SpecDiagnostic[]) { + for (const heading of headings) { + if (forbiddenHeadings.has(heading.normalized)) { + diagnostics.push( + createDiagnostic( + 'forbidden-heading', + `The section “${heading.text}” does not belong in a component spec.`, + path, + heading, + ), + ) + } + } +} + +function validateAllowedHeadings( + headings: readonly HeadingRecord[], + allowed: ReadonlySet, + path: string, + diagnostics: SpecDiagnostic[], +) { + const seen = new Set() + + for (const heading of headings) { + if (forbiddenHeadings.has(heading.normalized)) { + continue + } + + if (!allowed.has(heading.normalized)) { + diagnostics.push( + createDiagnostic( + 'unexpected-heading', + `The section “${heading.text}” is not supported at this heading level.`, + path, + heading, + ), + ) + continue + } + + if (seen.has(heading.normalized)) { + diagnostics.push( + createDiagnostic( + 'duplicate-heading', + `The section “${heading.text}” is declared more than once.`, + path, + heading, + ), + ) + } + seen.add(heading.normalized) + } +} + +function findUniqueHeading( + headings: readonly HeadingRecord[], + name: string, + path: string, + diagnostics: SpecDiagnostic[], +): HeadingRecord | null { + const matches = headings.filter(heading => heading.normalized === name) + if (matches.length > 1) { + diagnostics.push( + createDiagnostic( + 'duplicate-heading', + `The section “${matches[1].text}” is declared more than once.`, + path, + matches[1], + ), + ) + } + return matches[0] ?? null +} + +function createOptionalSection( + source: string, + root: Root, + heading: HeadingRecord | null, + path: string, + diagnostics: SpecDiagnostic[], +): SpecSection | undefined { + if (heading === null) { + return undefined + } + + const section = createSection(source, root, heading, path) + if (isEmptyMarkdown(section.markdown)) { + diagnostics.push( + createDiagnostic('empty-section', `The section “${heading.text}” must not be empty.`, path, heading), + ) + } + return section +} + +function createSection(source: string, root: Root, heading: HeadingRecord, path: string): SpecSection { + return { + heading: heading.text, + markdown: getSectionMarkdown(source, root, heading), + location: createLocation(path, heading), + } +} + +function getIntroMarkdown(source: string, root: Root, heading: HeadingRecord): string { + const start = heading.node.position?.end.offset ?? 0 + const nextHeading = root.children + .slice(heading.childIndex + 1) + .find((node): node is Heading => node.type === 'heading') + const end = nextHeading?.position?.start.offset ?? source.length + return source.slice(start, end).trim() +} + +function getSectionMarkdown(source: string, root: Root, heading: HeadingRecord): string { + const start = heading.node.position?.end.offset ?? 0 + const endIndex = findSectionEndIndex(root, heading) + const end = + endIndex < root.children.length ? (root.children[endIndex].position?.start.offset ?? source.length) : source.length + return source.slice(start, end).trim() +} + +function findSectionEndIndex(root: Root, heading: HeadingRecord): number { + for (let index = heading.childIndex + 1; index < root.children.length; index++) { + const node = root.children[index] + if (node.type === 'heading' && node.depth <= heading.node.depth) { + return index + } + } + return root.children.length +} + +function featureSectionKey(heading: string): 'markup' | 'behavior' | 'publicApi' | 'accessibility' | null { + switch (heading) { + case 'markup': + return 'markup' + case 'behavior': + return 'behavior' + case 'public api': + return 'publicApi' + case 'accessibility': + return 'accessibility' + default: + return null + } +} + +function normalizeHeading(value: string): string { + return value.trim().toLowerCase().replace(/\s+/g, ' ') +} + +function isEmptyMarkdown(value: string): boolean { + let sanitized = value + let previous: string + do { + previous = sanitized + sanitized = sanitized.replace(//g, '') + } while (sanitized !== previous) + return sanitized.trim().length === 0 +} + +function createLocation(path: string, heading: HeadingRecord | null): SpecLocation { + return { + path, + line: heading?.node.position?.start.line ?? 1, + column: heading?.node.position?.start.column ?? 1, + } +} + +function createDiagnostic( + code: SpecDiagnosticCode, + message: string, + path: string, + heading: HeadingRecord | null = null, +): SpecDiagnostic { + return { + code, + message, + severity: 'error', + location: createLocation(path, heading), + } +} + +function createResult(value: T | null, diagnostics: readonly SpecDiagnostic[]): SpecParseResult { + return { + valid: value !== null && diagnostics.every(diagnostic => diagnostic.severity !== 'error'), + value, + diagnostics, + } +} diff --git a/packages/spec/src/types.ts b/packages/spec/src/types.ts new file mode 100644 index 00000000000..d257d51d268 --- /dev/null +++ b/packages/spec/src/types.ts @@ -0,0 +1,99 @@ +export type ComponentSpecFormat = 'single' | 'split' + +export type SpecSeverity = 'error' | 'warning' + +export type SpecDiagnosticCode = + | 'ambiguous-spec' + | 'duplicate-feature' + | 'duplicate-heading' + | 'empty-description' + | 'empty-section' + | 'feature-title-mismatch' + | 'file-not-found' + | 'forbidden-heading' + | 'invalid-entry' + | 'invalid-feature-link' + | 'invalid-title' + | 'missing-default-feature' + | 'missing-feature-file' + | 'missing-features' + | 'missing-title' + | 'orphan-feature-file' + | 'read-failed' + | 'unexpected-heading' + +export interface SpecLocation { + readonly path: string + readonly line: number + readonly column: number +} + +export interface SpecDiagnostic { + readonly code: SpecDiagnosticCode + readonly message: string + readonly severity: SpecSeverity + readonly location: SpecLocation +} + +export interface SpecSection { + readonly heading: string + readonly markdown: string + readonly location: SpecLocation +} + +export interface SpecFeatureSections { + readonly markup?: SpecSection + readonly behavior?: SpecSection + readonly publicApi?: SpecSection + readonly accessibility?: SpecSection +} + +export interface SpecFeature { + readonly name: string + readonly description: string + readonly sections: SpecFeatureSections + readonly location: SpecLocation + readonly path: string +} + +export interface ComponentSpec { + readonly name: string + readonly description: string + readonly accessibility?: SpecSection + readonly features: readonly SpecFeature[] + readonly glossary?: SpecSection + readonly format: ComponentSpecFormat + readonly entryPath: string + readonly files: readonly string[] +} + +export interface SpecFeatureLink { + readonly name: string + readonly href: string + readonly location: SpecLocation +} + +export interface SpecIndex { + readonly name: string + readonly description: string + readonly accessibility?: SpecSection + readonly features: readonly SpecFeatureLink[] + readonly glossary?: SpecSection + readonly path: string +} + +export interface SpecParseResult { + readonly valid: boolean + readonly value: T | null + readonly diagnostics: readonly SpecDiagnostic[] +} + +export interface SpecCollectionResult { + readonly valid: boolean + readonly specs: readonly ComponentSpec[] + readonly diagnostics: readonly SpecDiagnostic[] +} + +export interface FindComponentSpecsOptions { + readonly ignore?: readonly string[] +} diff --git a/packages/spec/tsconfig.json b/packages/spec/tsconfig.json new file mode 100644 index 00000000000..1da8cb9e28d --- /dev/null +++ b/packages/spec/tsconfig.json @@ -0,0 +1,4 @@ +{ + "extends": "../../tsconfig.base.json", + "include": ["src", "vitest.config.ts"] +} diff --git a/packages/spec/vitest.config.ts b/packages/spec/vitest.config.ts new file mode 100644 index 00000000000..6c7353fc4e0 --- /dev/null +++ b/packages/spec/vitest.config.ts @@ -0,0 +1,7 @@ +import {defineConfig} from '@primer/vitest-config/config' + +export default defineConfig({ + test: { + environment: 'node', + }, +})