diff --git a/.agents/papercuts.md b/.agents/papercuts.md index 1a746c0c..d4f67027 100644 --- a/.agents/papercuts.md +++ b/.agents/papercuts.md @@ -58,3 +58,26 @@ evidence shows it does not help. Silence in this log is not evidence. - Problem: The documented contract is `.x`, and the regex manager is supposed to keep that form with `rangeStrategy: 'replace'`. The non-major group still rewrote the field to an exact version, so the PR had to be corrected by hand. + +## The token board's visual capture renders two different ways on the same code + +- Seen: 2026-10-13 (#797). The Visual regression run for commit `42cf474f` (38333714238) and the run + for commit `c818adf3` (38338181845) produced byte-identical `theme/token-board-*` renders that + differ from the baseline. The run for commit `a2f12ba8` (38336322056) matched the baseline, + although that commit contains the same code as `42cf474f`. The token board imports no field + components. +- Where: `packages/@luke-ui/react/src/theme/token-board.browser.test.tsx`. Governing rule: + docs/TESTING.md, "Remove nondeterminism. Do not add an allowance." +- Problem: about 290k mismatched pixels across the whole board (anti-aliasing, not a shift) appear + on unrelated PRs, so reviewers must judge whether a token-board diff is real. + +## The font-family browser assertion disagrees with Chromium's computed serialization + +- Seen: 2026-10-13 (necessity-markers-dom cleanup). `TURBO_FORCE=true pnpm run test` failed at + `src/theme/font-family.browser.test.ts:76`: the assertion accepted + `-apple-system, BlinkMacSystemFont, Noto Sans CJK JP, sans-serif`, but Chromium returned + `-apple-system, "system-ui", "Noto Sans CJK JP", sans-serif`. +- Where: `packages/@luke-ui/react/src/theme/font-family.browser.test.ts:76`. Governing rule: + docs/TESTING.md, "Component tests". +- Problem: The assertion compares different font-family serializations, and the browser test fails + on Chromium's returned value. diff --git a/apps/docs/content/docs/components/forms/checkbox-field.mdx b/apps/docs/content/docs/components/forms/checkbox-field.mdx index 743adc17..8f4bfd0a 100644 --- a/apps/docs/content/docs/components/forms/checkbox-field.mdx +++ b/apps/docs/content/docs/components/forms/checkbox-field.mdx @@ -50,9 +50,8 @@ library or your server. A non-empty message marks the checkbox invalid. Read ## Required marker -A required checkbox shows a marker after its `label`. Read -[Required fields](/components/forms/text-input-field#required-fields) for both `necessityIndicator` -values. +By default, a required checkbox shows a marker after its `label`. Read +[Required fields](/docs/forms#required-fields) for the `necessityIndicator` values. ## Labels with Text diff --git a/apps/docs/content/docs/components/forms/combobox-field.mdx b/apps/docs/content/docs/components/forms/combobox-field.mdx index 3f79fc8a..0df54d5d 100644 --- a/apps/docs/content/docs/components/forms/combobox-field.mdx +++ b/apps/docs/content/docs/components/forms/combobox-field.mdx @@ -32,7 +32,7 @@ fields. ## Required fields Set `isRequired` to make a selection mandatory. Use `necessityIndicator` to choose how it appears -beside the label. +beside the label. Read [Required fields](/docs/forms#required-fields) for the indicator values. diff --git a/apps/docs/content/docs/components/forms/select-field.mdx b/apps/docs/content/docs/components/forms/select-field.mdx index e0ae9afe..a59c35b5 100644 --- a/apps/docs/content/docs/components/forms/select-field.mdx +++ b/apps/docs/content/docs/components/forms/select-field.mdx @@ -37,7 +37,10 @@ selection yourself. `onChange` receives the selected key, or `null` when nothing ## Required fields Set `isRequired` to make a selection mandatory. Use `necessityIndicator` to choose how it appears -beside the label. +beside the label. Read [Required fields](/docs/forms#required-fields) for the indicator values. + +A required Select also announces “required” from a hidden cue on the trigger, including when +`necessityIndicator` is `'optional-label'`. diff --git a/apps/docs/content/docs/components/forms/switch-field.mdx b/apps/docs/content/docs/components/forms/switch-field.mdx index a0fcde0b..6941cab2 100644 --- a/apps/docs/content/docs/components/forms/switch-field.mdx +++ b/apps/docs/content/docs/components/forms/switch-field.mdx @@ -48,9 +48,8 @@ marks the switch invalid. Set `isRequired` only when the switch must be on befor and pass `validate` for a custom rule. Read [Validation](/docs/validation) for where messages come from and how to write them. -A required switch shows a marker after its `label`. Read -[Required fields](/components/forms/text-input-field#required-fields) for both `necessityIndicator` -values. +By default, a required switch shows a marker after its `label`. Read +[Required fields](/docs/forms#required-fields) for the `necessityIndicator` values. ## IDs and refs diff --git a/apps/docs/content/docs/components/forms/text-input-field.mdx b/apps/docs/content/docs/components/forms/text-input-field.mdx index be9e7712..c1218692 100644 --- a/apps/docs/content/docs/components/forms/text-input-field.mdx +++ b/apps/docs/content/docs/components/forms/text-input-field.mdx @@ -26,8 +26,9 @@ instead. ## Required fields -Set `isRequired` for required fields. `necessityIndicator="icon"` adds a visual marker. -`necessityIndicator="label"` appends “(required)” to the label. +Set `isRequired` when the field must have a value. By default, a required text input shows a marker +after its `label`. Read [Required fields](/docs/forms#required-fields) for the `necessityIndicator` +values. diff --git a/apps/docs/content/docs/components/primitives/checkbox.mdx b/apps/docs/content/docs/components/primitives/checkbox.mdx index 04f31218..48012699 100644 --- a/apps/docs/content/docs/components/primitives/checkbox.mdx +++ b/apps/docs/content/docs/components/primitives/checkbox.mdx @@ -44,8 +44,8 @@ Put the label text in either `CheckboxLabel` or `FieldLabel`, not both. The chec the text from each. Pass `aria-label` when no visible text names the checkbox. `CheckboxLabel` draws no required marker. With `isRequired` on `CheckboxRoot`, a `FieldLabel` does, -as [Required fields](/components/forms/text-input-field#required-fields) describes. Show the -required state in your own content when you use `CheckboxLabel` for the text. +as [Required fields](/docs/forms#required-fields) describes. Show the required state in your own +content when you use `CheckboxLabel` for the text. ## Text in the label diff --git a/apps/docs/content/docs/components/primitives/field.mdx b/apps/docs/content/docs/components/primitives/field.mdx index 73eabbb6..3eefd4ca 100644 --- a/apps/docs/content/docs/components/primitives/field.mdx +++ b/apps/docs/content/docs/components/primitives/field.mdx @@ -38,8 +38,27 @@ the control when clicked. ## Required fields -`necessityIndicator` changes the visible marker on `FieldLabel`. It does not make the control -required, so set the required state on the control root separately. +`necessityIndicator` changes the visible marker on `FieldLabel` and `InlineLabelText`. It does not +make the control required. Set `isRequired` on the control root, or `required` on a native control +next to the label part. + +`'required-asterisk'` and `'required-label'` mark required labels. `'optional-label'` marks +definitely optional labels and includes “(optional)” in the accessible name. A composed `Field` +named only through `aria-label` or `aria-labelledby` has no `label` to render, so it shows no +marker. + +`isRequired` on `FieldLabel` or `InlineLabelText` describes the control's state for the marker. It +does not set the control's required semantics. Place it on the label part next to a native control +with `required`. Inside a Luke UI control root, the root's required state wins. + +```tsx + +``` + +Read [Required fields](/docs/forms#required-fields) for the three indicator values. ## Validation messages @@ -69,6 +88,13 @@ icon, while the surrounding control root is invalid. name="FieldLabelProps" /> +### InlineLabelTextProps + + + ### FieldDescriptionProps { isRequired label="Work location" name="workLocation" - necessityIndicator="icon" + necessityIndicator="required-asterisk" placeholder="Choose a country" > {(item) => {item.label}} diff --git a/apps/docs/src/examples/select-field/required.tsx b/apps/docs/src/examples/select-field/required.tsx index 1011eed8..d5967e25 100644 --- a/apps/docs/src/examples/select-field/required.tsx +++ b/apps/docs/src/examples/select-field/required.tsx @@ -14,7 +14,7 @@ export default () => { items={countries} label="Work location" name="workLocation" - necessityIndicator="icon" + necessityIndicator="required-asterisk" placeholder="Choose a country" > {(item) => {item.label}} diff --git a/apps/docs/src/examples/text-input-field/required.tsx b/apps/docs/src/examples/text-input-field/required.tsx index 8b6e9884..b3c94814 100644 --- a/apps/docs/src/examples/text-input-field/required.tsx +++ b/apps/docs/src/examples/text-input-field/required.tsx @@ -4,8 +4,19 @@ import { TextInputField } from '@luke-ui/react/text-input-field'; export default () => { return ( - - + + + ); }; diff --git a/apps/docs/src/lib/guide-prop-audit-data.ts b/apps/docs/src/lib/guide-prop-audit-data.ts index 0f8bc1fd..5b50fa0b 100644 --- a/apps/docs/src/lib/guide-prop-audit-data.ts +++ b/apps/docs/src/lib/guide-prop-audit-data.ts @@ -263,8 +263,10 @@ export const GUIDE_TAUGHT_PROPS: Readonly>> ], 'primitives/field.mdx::packages/@luke-ui/react/src/core/primitives/field/field.tsx::InlineFieldProps': ['description', 'errorMessage'], + 'primitives/field.mdx::packages/@luke-ui/react/src/core/primitives/field/inline-label-text.tsx::InlineLabelTextProps': + ['isRequired', 'necessityIndicator'], 'primitives/field.mdx::packages/@luke-ui/react/src/core/primitives/field/label.tsx::FieldLabelProps': - ['htmlFor', 'necessityIndicator'], + ['htmlFor', 'isRequired', 'necessityIndicator'], 'primitives/select.mdx::packages/@luke-ui/react/src/core/primitives/select/select.tsx::SelectIndicatorProps': ['children'], 'primitives/select.mdx::packages/@luke-ui/react/src/core/primitives/select/select.tsx::SelectItemProps': diff --git a/docs/CONVENTIONS.md b/docs/CONVENTIONS.md index 14dcd979..c4346aef 100644 --- a/docs/CONVENTIONS.md +++ b/docs/CONVENTIONS.md @@ -16,6 +16,11 @@ double quotes in JSX. `// oxlint-disable-next-line /` directly above the offending line — the directive applies to the literal next line, so it cannot sit inside a multi-line comment. +## React Compiler + +The package is built with the React Compiler, so do not add `useMemo`, `useCallback` or `memo` for +performance. Pass plain objects as context values, because the compiler memoises them. + ## Reuse Prefer utilities the repo already ships over hand-rolling: `cx` from `@luke-ui/react/utils` for diff --git a/packages/@luke-ui/react/package.json b/packages/@luke-ui/react/package.json index 11867b68..2feb696e 100644 --- a/packages/@luke-ui/react/package.json +++ b/packages/@luke-ui/react/package.json @@ -131,6 +131,7 @@ "@vitest/coverage-v8": "catalog:", "@vitest/ui": "catalog:", "axe-core": "catalog:", + "dom-accessibility-api": "catalog:", "lightningcss": "catalog:", "node-html-parser": "catalog:", "oxc-transform-react": "catalog:", diff --git a/packages/@luke-ui/react/src/core/checkbox-field/checkbox-field.browser.test.tsx b/packages/@luke-ui/react/src/core/checkbox-field/checkbox-field.browser.test.tsx index 1f7eec22..b1d6e2ab 100644 --- a/packages/@luke-ui/react/src/core/checkbox-field/checkbox-field.browser.test.tsx +++ b/packages/@luke-ui/react/src/core/checkbox-field/checkbox-field.browser.test.tsx @@ -2,6 +2,7 @@ import { CheckboxField } from '@luke-ui/react/checkbox-field'; import { Strong } from '@luke-ui/react/strong'; import { Text } from '@luke-ui/react/text'; import { createRef } from 'react'; +import { CheckboxFieldContext } from 'react-aria-components'; import { expect, test } from 'vite-plus/test'; import { page, userEvent } from 'vite-plus/test/context'; import { expectNoAxeViolations } from '../test-utils/axe.js'; @@ -120,6 +121,14 @@ function controlFor(input: HTMLInputElement): HTMLElement { return control; } +/** A range from `start` to the end of a text node. */ +function textRange(text: Text, start: number): Range { + const range = document.createRange(); + range.setStart(text, start); + range.setEnd(text, text.data.length); + return range; +} + test('CheckboxField resolves ref to the root, resolves object and callback inputRef to the control, participates in a form, and fires onBlur', () => { const ref = createRef(); const inputRef = createRef(); @@ -241,31 +250,98 @@ test('the control renders before the label', () => { expect(control.nextSibling?.textContent).toBe('Terms'); }); -// The marker is CSS content with no DOM node, so the accessible name is how the tests observe it. -// It follows the last word with no space before it. +// The marker is hidden from assistive technology, so the label's text shows it and the name does not. test('a required CheckboxField marks its label with its necessityIndicator', () => { render( <> - + , ); + const asterisk = checkbox('Checkbox asterisk'); + const words = checkbox('Checkbox words'); - expect(checkbox('Checkbox icon*')).toBeInTheDocument(); - expect(checkbox('Checkbox words(required)')).toBeInTheDocument(); + expect(asterisk).toBeRequired(); + expect(labelFor(asterisk).textContent).toBe('Checkbox asterisk\u00A0*'); + expect(words).toBeRequired(); + expect(labelFor(words).textContent).toBe('Checkbox words\u00A0(required)'); }); test('an optional CheckboxField shows no marker', () => { render(); + const input = checkbox('Optional'); + + expect(input).not.toBeRequired(); + expect(labelFor(input).textContent).toBe('Optional'); +}); + +const requiredCheckboxFieldContext = { isRequired: true }; - expect(checkbox('Optional')).toHaveAccessibleName('Optional'); +test('a CheckboxField without isRequired keeps isRequired from React Aria context', () => { + render( + + + + , + ); + const optionalLabel = checkbox('Terms'); + const asterisk = checkbox('Required asterisk'); + + expect(optionalLabel).toBeRequired(); + expect(labelFor(optionalLabel).textContent).toBe('Terms'); + expect(asterisk).toBeRequired(); + expect(labelFor(asterisk).textContent).toBe('Required asterisk\u00A0*'); }); +// Layout is the contract: in a container that only fits the label text, the label wraps before its +// last word instead of leaving the marker alone on the next line. +for (const { accessibleName, label, necessityIndicator, ...fieldProps } of [ + { accessibleName: 'Accept the terms', isRequired: true, label: 'Accept the terms' }, + { + accessibleName: 'Accept the terms (optional)', + label: 'Accept the terms', + necessityIndicator: 'optional-label' as const, + }, +]) { + test(`the ${necessityIndicator ?? 'required-asterisk'} marker stays on the line of the last label word`, () => { + render( +
+ +
, + ); + const frame = page.getByTestId('frame').element(); + if (!(frame instanceof HTMLElement)) throw new Error('Expected an HTML frame.'); + const labelText = controlFor(checkbox(accessibleName)).nextElementSibling; + const text = labelText?.firstChild; + const marker = labelText?.lastElementChild; + // `Text` here is the Luke UI component, so check against the DOM's `Text` node type. + if (!(text instanceof window.Text) || marker == null) { + throw new Error('Expected text, then the marker.'); + } + + const textEnd = textRange(text, 0).getBoundingClientRect().right; + frame.style.inlineSize = `${Math.ceil(textEnd - frame.getBoundingClientRect().left)}px`; + const firstLine = textRange(text, 0).getClientRects()[0]; + const lastWord = textRange(text, text.data.lastIndexOf(' ') + 1).getBoundingClientRect(); + const markerBox = marker.getBoundingClientRect(); + if (firstLine == null) throw new Error('Expected the label text to render.'); + + expect(lastWord.top).toBeGreaterThan(firstLine.top); + expect(markerBox.top).toBe(lastWord.top); + expect(markerBox.left).toBeGreaterThanOrEqual(lastWord.right - 1); + }); +} + test('an errorMessage marks the checkbox invalid and shows the error under the label', () => { render(); const input = checkbox('Terms'); @@ -287,7 +363,7 @@ test('a required CheckboxField shows its native validation message after a faile , ); - const input = checkbox('Terms*'); + const input = checkbox('Terms'); expect(input).not.toHaveAttribute('aria-invalid', 'true'); @@ -307,7 +383,7 @@ test('the CheckboxField scene has no axe violations', async () => { isRequired label="Required" /> - + , ); @@ -395,7 +471,12 @@ test('necessity markers', { tags: ['visual'] }, async () => { const { locator } = render( - + { isRequired label="This required label wraps onto a second line so the marker follows." name="wrapping-words" - necessityIndicator="label" + necessityIndicator="required-label" /> , ); diff --git a/packages/@luke-ui/react/src/core/checkbox-field/checkbox-field.tsx b/packages/@luke-ui/react/src/core/checkbox-field/checkbox-field.tsx index d9ac7004..ab0c19f0 100644 --- a/packages/@luke-ui/react/src/core/checkbox-field/checkbox-field.tsx +++ b/packages/@luke-ui/react/src/core/checkbox-field/checkbox-field.tsx @@ -6,7 +6,7 @@ import { CheckboxLabel, CheckboxRoot, } from '../primitives/checkbox/checkbox.js'; -import type { FieldNecessityIndicator } from '../primitives/field/field.js'; +import type { NecessityIndicator } from '../primitives/field/field.js'; import { InlineField, isInvalidFromErrorMessage, @@ -32,10 +32,10 @@ interface _CheckboxFieldProps extends _CheckboxFieldOmit { */ label: Exclude; /** - * How a required checkbox is marked. - * @default 'icon' + * Controls the visible marker for required or optional controls. + * @default 'required-asterisk' */ - necessityIndicator?: FieldNecessityIndicator; + necessityIndicator?: NecessityIndicator; } /** Props for `CheckboxField`. */ diff --git a/packages/@luke-ui/react/src/core/combobox-field/combobox-field.browser.test.tsx b/packages/@luke-ui/react/src/core/combobox-field/combobox-field.browser.test.tsx index d51087cd..83d4af8a 100644 --- a/packages/@luke-ui/react/src/core/combobox-field/combobox-field.browser.test.tsx +++ b/packages/@luke-ui/react/src/core/combobox-field/combobox-field.browser.test.tsx @@ -47,6 +47,12 @@ const wrappingErrorMessage = const renderCountryItem = (item: CountryItem) => {item.label}; +function combobox(name: string): HTMLInputElement { + const element = page.getByRole('combobox', { name }).element(); + if (!(element instanceof HTMLInputElement)) throw new Error(`Expected a combobox "${name}".`); + return element; +} + // Luke UI widens RAC's `inputRef` to accept React Hook Form's callback ref. test('ComboboxField resolves object and callback inputRefs, submits its value, and fires onBlur', () => { const inputRef = createRef(); @@ -321,6 +327,38 @@ test('ComboboxField uses a mobile modal to search and select an option', async ( await expect.poll(() => new FormData(form).get('country')).toBe('ca'); }); +// Required markers are hidden from assistive technology, so the name does not include them. +test('a required ComboboxField marks its label with its necessityIndicator', () => { + render( + <> + + {renderCountryItem} + + + {renderCountryItem} + + + {renderCountryItem} + + , + ); + const asterisk = combobox('Required asterisk'); + const words = combobox('Required words'); + const optional = combobox('Optional'); + + expect(asterisk).toBeRequired(); + expect(asterisk.labels?.[0]?.textContent).toBe('Required asterisk\u00A0*'); + expect(words).toBeRequired(); + expect(words.labels?.[0]?.textContent).toBe('Required words\u00A0(required)'); + expect(optional).not.toBeRequired(); + expect(optional.labels?.[0]?.textContent).toBe('Optional'); +}); + test('an unlabeled ComboboxField exposes aria-label on the combobox', () => { const { locator } = render( diff --git a/packages/@luke-ui/react/src/core/packed-consumer/react-fixture.ts b/packages/@luke-ui/react/src/core/packed-consumer/react-fixture.ts index b0d184fa..1bb4a586 100644 --- a/packages/@luke-ui/react/src/core/packed-consumer/react-fixture.ts +++ b/packages/@luke-ui/react/src/core/packed-consumer/react-fixture.ts @@ -85,6 +85,7 @@ import { Button } from '@luke-ui/react/button'; import { CheckboxField } from '@luke-ui/react/checkbox-field'; import { Icon } from '@luke-ui/react/icon'; import { Provider } from '@luke-ui/react/provider'; +import { SelectField, SelectItem } from '@luke-ui/react/select-field'; import spritesheetHref from '@luke-ui/react/spritesheet.svg?url&no-inline'; import { SwitchField } from '@luke-ui/react/switch-field'; import { TextInputField } from '@luke-ui/react/text-input-field'; @@ -106,6 +107,12 @@ export function App({ onHydrated }) { h(TextInputField, { label: 'Name' }), h(CheckboxField, { label: 'Example checkbox' }), h(SwitchField, { label: 'Example switch' }), + h( + SelectField, + { isRequired: true, label: 'Country' }, + h(SelectItem, { id: 'au' }, 'Australia'), + h(SelectItem, { id: 'nz' }, 'New Zealand'), + ), h(Button, null, 'Save'), ), ); diff --git a/packages/@luke-ui/react/src/core/packed-consumer/react-package.test.ts b/packages/@luke-ui/react/src/core/packed-consumer/react-package.test.ts index dd3c79f6..127ffaae 100644 --- a/packages/@luke-ui/react/src/core/packed-consumer/react-package.test.ts +++ b/packages/@luke-ui/react/src/core/packed-consumer/react-package.test.ts @@ -162,6 +162,7 @@ for (const consumer of consumers) { expect(markup).toContain(' element === serverElements[index]), recoverableErrors: hydration?.recoverableErrors, + selectName: { + ids: selectNameIds, + text: selectNameIds.map((id) => document.getElementById(id)?.textContent ?? null), + unresolved: selectNameIds.filter((id) => document.getElementById(id) == null), + }, textAfterHydration: root?.textContent, serverText: serverSnapshot?.text, }; @@ -201,13 +212,17 @@ for (const consumer of consumers) { expect(result.recoverableErrors).toEqual([]); expect(consoleErrors).toEqual([]); - expect(result.controls).toEqual({ buttons: 1, inputs: 3 }); + expect(result.controls).toEqual({ buttons: 2, inputs: 3 }); // React Aria effects adjust attributes after hydration, so compare nodes, not markup. // Every server element must survive in place: none replaced, added, or removed. expect(result.keptServerElements).toBe(true); expect(result.textAfterHydration).toBe(result.serverText); // Each label resolves through server-generated ids, so it proves those ids hydrated intact. expect(result.inputLabels).toEqual(['Name', 'Example checkbox', 'Example switch']); + // The Select's required cue is hidden text the trigger references by a server-generated id. + expect(result.serverText).toContain('(required)'); + expect(result.selectName.unresolved).toEqual([]); + expect(result.selectName.text).toContain('(required)'); // Clicking the label text proves hydration attached each field's handlers. await page.getByText('Example checkbox').click(); diff --git a/packages/@luke-ui/react/src/core/primitives/checkbox/checkbox.browser.test.tsx b/packages/@luke-ui/react/src/core/primitives/checkbox/checkbox.browser.test.tsx index fb3f702c..75f4a8c1 100644 --- a/packages/@luke-ui/react/src/core/primitives/checkbox/checkbox.browser.test.tsx +++ b/packages/@luke-ui/react/src/core/primitives/checkbox/checkbox.browser.test.tsx @@ -213,8 +213,31 @@ test('CheckboxLabel draws no required marker', () => { , ); + const input = checkbox('Required'); - expect(checkbox('Required')).toHaveAccessibleName('Required'); + expect(input).toBeRequired(); + expect(input.closest('label')?.textContent).toBe('Required'); +}); + +// The marker is hidden from assistive technology, so the label's text shows it and the name does not. +test('a FieldLabel marks a required CheckboxRoot', () => { + render( + <> + + Required + + + + Optional + + + , + ); + + expect(checkbox('Required')).toBeRequired(); + expect(page.getByTestId('required-label').element().textContent).toBe('Required\u00A0*'); + expect(checkbox('Optional')).not.toBeRequired(); + expect(page.getByTestId('optional-label').element().textContent).toBe('Optional'); }); test('a composed checkbox primitive has no axe violations', async () => { diff --git a/packages/@luke-ui/react/src/core/primitives/checkbox/checkbox.test.ts b/packages/@luke-ui/react/src/core/primitives/checkbox/checkbox.test.ts index 10333116..cfa229bd 100644 --- a/packages/@luke-ui/react/src/core/primitives/checkbox/checkbox.test.ts +++ b/packages/@luke-ui/react/src/core/primitives/checkbox/checkbox.test.ts @@ -16,5 +16,5 @@ test('CheckboxRoot owns ids through id and inputId, not render', () => { test('CheckboxLabel has no necessity indicator', () => { // @ts-expect-error — CheckboxField owns the required marker - assertType({ children: 'Terms', necessityIndicator: 'label' }); + assertType({ children: 'Terms', necessityIndicator: 'required-label' }); }); diff --git a/packages/@luke-ui/react/src/core/primitives/checkbox/checkbox.tsx b/packages/@luke-ui/react/src/core/primitives/checkbox/checkbox.tsx index ae4dcd61..72d45d09 100644 --- a/packages/@luke-ui/react/src/core/primitives/checkbox/checkbox.tsx +++ b/packages/@luke-ui/react/src/core/primitives/checkbox/checkbox.tsx @@ -1,5 +1,6 @@ import type { ComponentProps, JSX, Ref } from 'react'; -import { useId, useMemo } from 'react'; +import { useId } from 'react'; +import { CheckboxFieldContext } from 'react-aria-components'; import type { CheckboxButtonProps as RacCheckboxButtonProps, CheckboxFieldProps as RacCheckboxFieldProps, @@ -10,8 +11,10 @@ import { } from 'react-aria-components/Checkbox'; import { LabelContext } from 'react-aria-components/Label'; import { composeRenderProps } from 'react-aria-components/composeRenderProps'; +import { useSlottedContext } from 'react-aria-components/slots'; import type { DistributiveOmit } from '../../types/distributive-omit.js'; import type { Prettify } from '../../types/prettify.js'; +import { FieldStateContext } from '../field/label.js'; import { fieldRecipe } from '../field/recipe.css.js'; import { rootIdProps } from '../root-id.js'; import type { CheckboxRecipeVariants } from './styles.css.js'; @@ -113,20 +116,26 @@ export type CheckboxIndicatorProps = Prettify<_CheckboxIndicatorProps>; * does not. */ export function CheckboxRoot(props: CheckboxRootProps): JSX.Element { - const { className, id, inputId, size, ...restProps } = props; + const { className, id, inputId, isRequired, size, ...restProps } = props; const generatedInputId = useId(); const resolvedInputId = inputId ?? generatedInputId; - const labelContext = useMemo(() => ({ htmlFor: resolvedInputId }), [resolvedInputId]); + const labelContext = { htmlFor: resolvedInputId }; + const checkboxFieldContext = useSlottedContext(CheckboxFieldContext, props.slot); + const resolvedIsRequired = isRequired ?? checkboxFieldContext?.isRequired ?? false; + const fieldState = { isRequired: resolvedIsRequired }; return ( - { - return checkboxRecipe({ size }).root({ className }); - })} - /> + + { + return checkboxRecipe({ size }).root({ className }); + })} + isRequired={isRequired} + /> + ); } diff --git a/packages/@luke-ui/react/src/core/primitives/combobox/input.tsx b/packages/@luke-ui/react/src/core/primitives/combobox/input.tsx index 1eb122aa..047c4071 100644 --- a/packages/@luke-ui/react/src/core/primitives/combobox/input.tsx +++ b/packages/@luke-ui/react/src/core/primitives/combobox/input.tsx @@ -80,6 +80,5 @@ export function ComboboxInput(props: ComboboxInputProps): JSX.Element { if (!isTraySearch) return input; - // oxlint-disable-next-line react/jsx-no-constructed-context-values return {input}; } diff --git a/packages/@luke-ui/react/src/core/primitives/combobox/root.tsx b/packages/@luke-ui/react/src/core/primitives/combobox/root.tsx index d42d5f35..7dcf1cc8 100644 --- a/packages/@luke-ui/react/src/core/primitives/combobox/root.tsx +++ b/packages/@luke-ui/react/src/core/primitives/combobox/root.tsx @@ -1,10 +1,12 @@ import type { JSX, Ref } from 'react'; import { useRef } from 'react'; import type { Key, ComboBoxProps as RacComboBoxProps } from 'react-aria-components/ComboBox'; -import { ComboBox as RacComboBox } from 'react-aria-components/ComboBox'; +import { ComboBoxContext, ComboBox as RacComboBox } from 'react-aria-components/ComboBox'; import { composeRenderProps } from 'react-aria-components/composeRenderProps'; +import { useSlottedContext } from 'react-aria-components/slots'; import type { DistributiveOmit } from '../../types/distributive-omit.js'; import type { Prettify } from '../../types/prettify.js'; +import { FieldStateContext } from '../field/label.js'; import { rootIdProps } from '../root-id.js'; import { ComboboxInputGroupContext } from './input-group-context.js'; import { ComboboxSizeProvider } from './size-context.js'; @@ -106,22 +108,25 @@ export function ComboboxRoot(props: ComboboxRootProps): JSX className, id, inputId, + isRequired, menuTrigger = 'focus', ref, size = 'medium', ...comboboxProps } = props; + const comboBoxContext = useSlottedContext(ComboBoxContext, props.slot); + const resolvedIsRequired = isRequired ?? comboBoxContext?.isRequired ?? false; + const fieldState = { isRequired: resolvedIsRequired }; // Published for the tray validation and submission inputs. `ComboBoxStateContext` does not // carry these props. - // oxlint-disable-next-line react/jsx-no-constructed-context-values const validationContextValue = { allowsCustomValue: comboboxProps.allowsCustomValue ?? false, form: comboboxProps.form, formValue: comboboxProps.formValue, isDisabled: comboboxProps.isDisabled ?? false, isReadOnly: comboboxProps.isReadOnly ?? false, - isRequired: comboboxProps.isRequired ?? false, + isRequired: resolvedIsRequired, name: comboboxProps.name, validationBehavior: comboboxProps.validationBehavior, }; @@ -132,16 +137,19 @@ export function ComboboxRoot(props: ComboboxRootProps): JSX - { - return comboboxRecipe().root({ className: renderedClassName }); - })} - menuTrigger={menuTrigger} - ref={ref} - /> + + { + return comboboxRecipe().root({ className: renderedClassName }); + })} + isRequired={isRequired} + menuTrigger={menuTrigger} + ref={ref} + /> + diff --git a/packages/@luke-ui/react/src/core/primitives/combobox/tray.browser.test.tsx b/packages/@luke-ui/react/src/core/primitives/combobox/tray.browser.test.tsx index b152b7ad..84793af5 100644 --- a/packages/@luke-ui/react/src/core/primitives/combobox/tray.browser.test.tsx +++ b/packages/@luke-ui/react/src/core/primitives/combobox/tray.browser.test.tsx @@ -283,7 +283,7 @@ test('ComboboxTrayTrigger blocks submission of a required, unselected combobox w expect(new FormData(form).getAll('country')).toHaveLength(1); await userEvent.click( - page.getByRole('button', { name: 'Country* Select a country...' }).element(), + page.getByRole('button', { name: 'Country Select a country...' }).element(), ); await userEvent.click(page.getByRole('option', { name: 'Australia' }).element()); await expect.element(page.getByRole('dialog')).not.toBeInTheDocument(); @@ -308,16 +308,16 @@ test('ComboboxTrayTrigger allows a required combobox with allowsCustomValue to s , ); - await openTray('Country* Select a country...', 'Country*'); - await enterTraySearch('Freedonia', 'Country*'); + await openTray(); + await enterTraySearch('Freedonia'); await userEvent.keyboard('{Escape}'); await expect.element(page.getByRole('dialog')).not.toBeInTheDocument(); await userEvent.click(page.getByRole('button', { name: 'Submit' }).element()); expect(submitCount).toBe(1); - await openTray('Country* Select a country...', 'Country*'); - await userEvent.clear(page.getByRole('searchbox', { name: 'Country*' }).element()); + await openTray(); + await userEvent.clear(page.getByRole('searchbox', { name: 'Country' }).element()); await userEvent.keyboard('{Escape}'); await expect.element(page.getByRole('dialog')).not.toBeInTheDocument(); @@ -438,7 +438,7 @@ test('ComboboxTrayTrigger associates with an external form via the root form pro expect(submitCount).toBe(0); await userEvent.click( - page.getByRole('button', { name: 'Country* Select a country...' }).element(), + page.getByRole('button', { name: 'Country Select a country...' }).element(), ); await userEvent.click(page.getByRole('option', { name: 'Australia' }).element()); await expect.element(page.getByRole('dialog')).not.toBeInTheDocument(); diff --git a/packages/@luke-ui/react/src/core/primitives/field/field.tsx b/packages/@luke-ui/react/src/core/primitives/field/field.tsx index 2906b521..ae6e566f 100644 --- a/packages/@luke-ui/react/src/core/primitives/field/field.tsx +++ b/packages/@luke-ui/react/src/core/primitives/field/field.tsx @@ -6,20 +6,29 @@ import type { FieldDescriptionProps } from './description.js'; import { FieldDescription } from './description.js'; import type { FieldErrorProps } from './error.js'; import { FieldError } from './error.js'; +import type { InlineLabelTextProps } from './inline-label-text.js'; +import { InlineLabelText } from './inline-label-text.js'; import { isInvalidFromErrorMessage, normalizeErrorMessage, } from './is-invalid-from-error-message.js'; -import type { FieldLabelProps, FieldNecessityIndicator } from './label.js'; +import type { FieldLabelProps, NecessityIndicator } from './label.js'; import { FieldLabel } from './label.js'; import { fieldRecipe } from './recipe.css.js'; import { Field as PrimitiveField } from './root.js'; -export type { FieldDescriptionProps, FieldErrorProps, FieldLabelProps, FieldNecessityIndicator }; +export type { + FieldDescriptionProps, + FieldErrorProps, + FieldLabelProps, + InlineLabelTextProps, + NecessityIndicator, +}; export { FieldDescription, FieldError, FieldLabel, + InlineLabelText, isInvalidFromErrorMessage, normalizeErrorMessage, }; @@ -28,8 +37,11 @@ export { interface FieldSlotContentProps { /** Optional helper text shown below the control. */ description?: ReactNode; - /** Label necessity style. @default 'icon' */ - necessityIndicator?: FieldNecessityIndicator; + /** + * Controls the visible marker for required or optional controls. + * @default 'required-asterisk' + */ + necessityIndicator?: NecessityIndicator; } /** Label, description, and error props for the `Field` primitive. */ @@ -84,14 +96,7 @@ export type FieldProps = Prettify<_FieldProps>; * alternatives, so do not render those parts inside a `Field`. */ export function Field(props: FieldProps): JSX.Element { - const { - children, - description, - errorMessage, - label, - necessityIndicator = 'icon', - ...restProps - } = props; + const { children, description, errorMessage, label, necessityIndicator, ...restProps } = props; return ( diff --git a/packages/@luke-ui/react/src/core/primitives/field/inline-label-text.tsx b/packages/@luke-ui/react/src/core/primitives/field/inline-label-text.tsx index 0f319f74..814d0235 100644 --- a/packages/@luke-ui/react/src/core/primitives/field/inline-label-text.tsx +++ b/packages/@luke-ui/react/src/core/primitives/field/inline-label-text.tsx @@ -1,23 +1,49 @@ -import type { JSX, ReactNode } from 'react'; -import type { FieldNecessityIndicator } from './recipe.css.js'; +import type { ComponentProps, JSX } from 'react'; +import type { Prettify } from '../../types/prettify.js'; +import type { NecessityIndicator } from './label.js'; +import { NecessityMarker, useNecessityIndicator } from './label.js'; import { fieldRecipe } from './recipe.css.js'; -interface InlineLabelTextProps { - /** The composed field's `label` content. */ - children: ReactNode; - /** How a required control is marked. */ - necessityIndicator: FieldNecessityIndicator | undefined; +interface _InlineLabelTextProps extends ComponentProps<'span'> { + /** + * Describes whether the associated control is required. It does not make the control required. + * Inside a Luke UI control root, the root's required state wins. + */ + isRequired?: boolean; + /** + * Controls the visible marker for required or optional controls. + * @default 'required-asterisk' + */ + necessityIndicator?: NecessityIndicator; } +/** Props for `InlineLabelText`. */ +export type InlineLabelTextProps = Prettify<_InlineLabelTextProps>; + /** - * The text of a composed inline field's label, such as `CheckboxField` or `SwitchField`. It is - * private to the composed fields: primitive consumers own their label content. + * Label text for a custom checkbox or switch layout. Place it beside the control when the composed + * field's label order does not fit. * - * It draws the required marker after the label's last inline content, so the marker follows the - * final word when the label wraps. + * It draws a necessity marker from the surrounding control root, or from its own `isRequired` when + * there is no root. */ export function InlineLabelText(props: InlineLabelTextProps): JSX.Element { - const { children, necessityIndicator = 'icon' } = props; + const { + children, + className, + isRequired, + necessityIndicator: necessityIndicatorProp, + ...restProps + } = props; + const necessityIndicator = useNecessityIndicator(necessityIndicatorProp); - return {children}; + return ( + + {children} + + + ); } diff --git a/packages/@luke-ui/react/src/core/primitives/field/label.tsx b/packages/@luke-ui/react/src/core/primitives/field/label.tsx index 081b3885..bd374307 100644 --- a/packages/@luke-ui/react/src/core/primitives/field/label.tsx +++ b/packages/@luke-ui/react/src/core/primitives/field/label.tsx @@ -1,17 +1,32 @@ import type { JSX } from 'react'; +import { createContext, use } from 'react'; import type { LabelProps as RacLabelProps } from 'react-aria-components/Label'; import { Label as RacLabel } from 'react-aria-components/Label'; import type { DistributiveOmit } from '../../types/distributive-omit.js'; import type { Prettify } from '../../types/prettify.js'; -import type { FieldNecessityIndicator } from './recipe.css.js'; import { fieldRecipe } from './recipe.css.js'; -/** Allowed `necessityIndicator` values for `FieldLabel`. */ -export type { FieldNecessityIndicator }; +/** How a field shows whether its control is required or optional. */ +export type NecessityIndicator = 'required-asterisk' | 'required-label' | 'optional-label'; + +interface FieldState { + isRequired: boolean; +} + +/** Required state from the nearest semantic control root, or `null` when no root is present. */ +export const FieldStateContext = createContext(null); interface FieldLabelStyleProps { - /** Shows how required fields are marked. */ - necessityIndicator?: FieldNecessityIndicator; + /** + * Describes whether the associated control is required. It does not make the control required. + * Inside a Luke UI control root, the root's required state wins. + */ + isRequired?: boolean; + /** + * Controls the visible marker for required or optional controls. + * @default 'required-asterisk' + */ + necessityIndicator?: NecessityIndicator; } type _FieldLabelOmit = DistributiveOmit; @@ -31,12 +46,84 @@ export type FieldLabelProps = Prettify<_FieldLabelProps>; * control automatically. In a switch or checkbox root, clicking it toggles the control. Do not also * put label text in `SwitchLabel` or `CheckboxLabel`, which draw no required marker. * - * It draws the required marker when the control root is required. + * It draws the necessity marker when the control root is required or optional, depending on + * `necessityIndicator`. */ export function FieldLabel(props: FieldLabelProps): JSX.Element { - const { className, necessityIndicator = 'icon', ...restProps } = props; + const { + children, + className, + isRequired, + necessityIndicator: necessityIndicatorProp, + ...restProps + } = props; + const necessityIndicator = useNecessityIndicator(necessityIndicatorProp); + + return ( + + {children} + + + ); +} + +/** Resolves the marker mode for a label part. */ +export function useNecessityIndicator(prop: NecessityIndicator | undefined): NecessityIndicator { + return prop ?? 'required-asterisk'; +} + +const NECESSITY_MARKER_TEXT = { + 'required-asterisk': '*', + 'required-label': '(required)', + 'optional-label': '(optional)', +} as const satisfies Record; + +/** Resolves requiredness, with the nearest control root taking precedence. */ +function useResolvedIsRequired(propIsRequired: boolean | undefined): boolean | undefined { + const context = use(FieldStateContext); + + return context?.isRequired ?? propIsRequired; +} + +// Joins the marker to the label's last word without allowing a line break. +const NON_BREAKING_SPACE = '\u00A0'; + +/** + * Adds the configured required or optional marker after the label text. Required markers are hidden + * from assistive technology. Optional markers are included in the accessible name. + */ +export function NecessityMarker(props: { + isRequired?: boolean; + necessityIndicator: NecessityIndicator; +}): JSX.Element | null { + const { isRequired: propIsRequired, necessityIndicator } = props; + const isRequired = useResolvedIsRequired(propIsRequired); + + const shouldHideFromAssistiveTechnology = + necessityIndicator === 'required-asterisk' || necessityIndicator === 'required-label'; + const shouldRender = + (shouldHideFromAssistiveTechnology && isRequired === true) || + (necessityIndicator === 'optional-label' && isRequired === false); + + if (!shouldRender) return null; + + const marker = NECESSITY_MARKER_TEXT[necessityIndicator]; + const className = fieldRecipe({ necessityIndicator }).necessityMarker(); + + if (necessityIndicator === 'optional-label') { + // Keep the optional space outside the span because dom-accessibility-api trims element text. + return ( + <> + {NON_BREAKING_SPACE} + {marker} + + ); + } return ( - + + {NON_BREAKING_SPACE} + {marker} + ); } diff --git a/packages/@luke-ui/react/src/core/primitives/field/recipe.css.ts b/packages/@luke-ui/react/src/core/primitives/field/recipe.css.ts index cdb139f3..2b0d6235 100644 --- a/packages/@luke-ui/react/src/core/primitives/field/recipe.css.ts +++ b/packages/@luke-ui/react/src/core/primitives/field/recipe.css.ts @@ -7,7 +7,6 @@ import type { SlottedConfigInput } from '../../styles/recipe.js'; import { recipe } from '../../styles/recipe.js'; const dataDisabledSelector = '[data-disabled="true"]'; -const dataRequiredSelector = '[data-required="true"]'; /** * Optional indentation for error messages that sit beneath a control+label row. @@ -30,29 +29,6 @@ export const inlineControlGap = vars.space.sp8; /** Gap between the error icon and the message text that follows it. */ const errorIconGap = vars.space.sp8; -/** Required marker drawn after a label's last inline content: an asterisk. */ -const iconNecessityMarker = { - selectors: { - [`${dataRequiredSelector} &::after`]: { - color: vars.color.foreground.danger.rest, - content: '"*"', - marginInlineStart: vars.space.sp4, - }, - }, -} as const; - -/** Required marker drawn after a label's last inline content: the word “(required)”. */ -const labelNecessityMarker = { - selectors: { - [`${dataRequiredSelector} &::after`]: { - color: vars.color.text.secondary, - content: '"(required)"', - fontWeight: vars.font.weight.body, - marginInlineStart: vars.space.sp4, - }, - }, -} as const; - /** * Inline size of the error icon's rail. It is `fieldMessageIndent` less the gap, so the rail plus * the gap spans the indent exactly and the message text starts at the label's inline edge. `max()` @@ -65,8 +41,8 @@ const errorIconRailInlineSize = `max(calc(${fallbackVar(fieldMessageIndent, '0px * * Slots: `root` (stacked layout), `inline` (inline-control layout), `label`, `inlineLabel` (the * clickable label of an inline control such as `CheckboxLabel`), `inlineLabelText` (the text of a - * composed inline control's label, which carries the required marker), `message` (description/error - * text), and `icon` (the error message's leading icon). + * composed inline control's label), `necessityMarker` (the required marker after a label's text), + * `message` (description/error text), and `icon` (the error message's leading icon). * * `FieldError` lays the error `message` out with `trackRecipe`'s `firstLine` rail alignment, which * centres the `icon` rail on the message's first line and keeps wrapped lines aligned with the @@ -118,6 +94,7 @@ const fieldConfig = { }, }, inlineLabelText: {}, + necessityMarker: {}, message: { ...vars.font.label, fontWeight: vars.font.weight.body, @@ -126,18 +103,27 @@ const fieldConfig = { icon: {}, }, defaultVariants: { - necessityIndicator: 'icon', + necessityIndicator: 'required-asterisk', tone: 'description', }, variants: { necessityIndicator: { - icon: { - inlineLabelText: iconNecessityMarker, - label: iconNecessityMarker, + 'required-asterisk': { + necessityMarker: { + color: vars.color.foreground.danger.rest, + }, }, - label: { - inlineLabelText: labelNecessityMarker, - label: labelNecessityMarker, + 'required-label': { + necessityMarker: { + color: vars.color.text.secondary, + fontWeight: vars.font.weight.body, + }, + }, + 'optional-label': { + necessityMarker: { + color: vars.color.text.secondary, + fontWeight: vars.font.weight.body, + }, }, }, tone: { @@ -172,9 +158,6 @@ const fieldConfig = { * Slotted recipe for the `Field` primitive. * * `fieldRecipe({ necessityIndicator, tone }).root() / .inline() / .label() / .inlineLabel() / - * .inlineLabelText() / .message() / .icon()`. + * .inlineLabelText() / .necessityMarker() / .message() / .icon()`. */ export const fieldRecipe = recipe(fieldConfig); - -/** Allowed `necessityIndicator` values for the field label. */ -export type FieldNecessityIndicator = keyof typeof fieldConfig.variants.necessityIndicator; diff --git a/packages/@luke-ui/react/src/core/primitives/select/select.browser.test.tsx b/packages/@luke-ui/react/src/core/primitives/select/select.browser.test.tsx index 062e1bd8..0ab71008 100644 --- a/packages/@luke-ui/react/src/core/primitives/select/select.browser.test.tsx +++ b/packages/@luke-ui/react/src/core/primitives/select/select.browser.test.tsx @@ -88,6 +88,24 @@ function trigger(name: RegExp | string): HTMLButtonElement { return element; } +/** The label that names a trigger, outside the trigger itself. */ +function labelFor(element: HTMLButtonElement): HTMLElement { + const label = element + .getAttribute('aria-labelledby') + ?.split(' ') + .map((id) => document.getElementById(id)) + .find((candidate) => candidate != null && !element.contains(candidate)); + if (label == null) throw new Error('Expected a label for the trigger.'); + return label; +} + +/** The hidden native `` React Aria renders in a select root for form submission. */ +function nativeSelect(rootId: string): HTMLSelectElement { + const select = document.getElementById(rootId)?.querySelector('select'); + if (select == null) throw new Error(`Expected a native select in "${rootId}".`); + return select; +} + function SelectFieldScene() { return ( @@ -94,7 +116,7 @@ function SelectFieldScene() { items={themeItems} label="Required with a label" name="required-label" - necessityIndicator="label" + necessityIndicator="required-label" placeholder="Choose a theme" > {renderThemeItem} @@ -148,31 +170,237 @@ test('SelectField adds aria-describedby to its own description', () => { expect(getDescribedText(trigger(/Without one/))).toBe('External hint'); }); -// The marker follows the last word of the label, so the accessible name has no space before it. -test('SelectField marks a required field with an icon or a label', () => { +const indicators = ['required-asterisk', 'required-label', 'optional-label'] as const; + +type RequiredNameCase = { + /** The field, with `isRequired` and a trigger named after "Theme". */ + field: ReactElement; + /** Words the trigger's name must contain, in any order. */ + expectedName: string; + /** An id from React Aria or the consumer that the trigger's `aria-labelledby` must keep. */ + keptId: (element: HTMLButtonElement) => string; + title: string; +}; + +const visibleLabelId = () => page.getByText(VISIBLE_LABEL_PATTERN).element().id; + +const requiredNameCases: Array = [ + ...indicators.map((necessityIndicator) => ({ + expectedName: 'Choose a theme Theme (required)', + field: ( + + {renderThemeItem} + + ), + keptId: visibleLabelId, + title: `a visible label with necessityIndicator="${necessityIndicator}"`, + })), + { + expectedName: 'Choose a theme Theme (required)', + field: ( + + {renderThemeItem} + + ), + keptId: (element) => element.id, + title: 'aria-label', + }, + { + expectedName: 'Choose a theme Theme (required)', + field: ( + <> + Theme + + {renderThemeItem} + + + ), + keptId: () => 'consumer-label', + title: 'aria-labelledby', + }, + { + expectedName: 'Light Theme (required)', + field: ( + + {renderThemeItem} + + ), + keptId: visibleLabelId, + title: 'a visible label and a selected value', + }, +]; + +for (const { expectedName, field, keptId, title } of requiredNameCases) { + test(`a required SelectField with ${title} adds "(required)" to the trigger's name once`, async () => { + render(field); + const element = trigger(/Theme/); + const ids = element.getAttribute('aria-labelledby')?.split(' ') ?? []; + + await expectAccessibleNameWords(element, expectedName); + expect(ids).toContain(keptId(element)); + expect(document.getElementById(ids.at(-1) ?? '')?.textContent).toBe('(required)'); + }); +} + +test('an optional SelectField has no required cue in its name', async () => { + render( + + {renderThemeItem} + , + ); + + await expectAccessibleNameWords(trigger(/Theme/), 'Choose a theme Theme'); +}); + +// Required markers are hidden from assistive technology; the trigger's cue supplies "(required)". +test('SelectField marks a required field with its necessityIndicator', async () => { render( <> - + {renderThemeItem} {renderThemeItem} - + + {renderThemeItem} + + , + ); + + // The trigger is a button, so the hidden native select carries the required state. + expect(nativeSelect('asterisk-root')).toBeRequired(); + expect(labelFor(trigger(/Asterisk field/)).textContent).toBe('Asterisk field\u00A0*'); + await expectAccessibleNameWords(trigger(/Asterisk field/), 'Light Asterisk field (required)'); + expect(nativeSelect('text-root')).toBeRequired(); + expect(labelFor(trigger(/Text field/)).textContent).toBe('Text field\u00A0(required)'); + await expectAccessibleNameWords(trigger(/Text field/), 'Light Text field (required)'); + expect(nativeSelect('optional-root')).not.toBeRequired(); + expect(labelFor(trigger(/Optional field/)).textContent).toBe('Optional field'); + await expectAccessibleNameWords(trigger(/Optional field/), 'Light Optional field'); +}); + +test('an optional SelectField with optional-label includes "(optional)" in its name once', async () => { + render( + + {renderThemeItem} + , + ); + + expect(labelFor(trigger(/Theme/)).textContent).toBe('Theme\u00A0(optional)'); + await expectAccessibleNameWords(trigger(/Theme/), 'Light Theme (optional)'); +}); + +test('an aria-label SelectField with optional-label has no "(optional)" in its name', async () => { + render( + + {renderThemeItem} + , + ); + + await expectAccessibleNameWords(trigger(/Theme/), 'Light Theme'); +}); + +test('an aria-labelledby SelectField with optional-label has no "(optional)" in its name', async () => { + render( + <> + Theme + {renderThemeItem} , ); - expect(trigger(/Icon field/)).toHaveAccessibleName('Light Icon field*'); - expect(trigger(/Text field/)).toHaveAccessibleName('Light Text field(required)'); - expect(trigger(/Optional field/)).toHaveAccessibleName('Light Optional field'); + await expectAccessibleNameWords(trigger(/Theme/), 'Light Theme'); +}); + +const requiredSelectContext = { isRequired: true }; + +test('a SelectField without isRequired keeps isRequired from React Aria context', async () => { + render( + + + {renderThemeItem} + + , + ); + const element = trigger(/Theme/); + const ids = element.getAttribute('aria-labelledby')?.split(' ') ?? []; + + expect(nativeSelect('context-required')).toBeRequired(); + expect(document.getElementById(ids.at(-1) ?? '')?.textContent).toBe('(required)'); + await expectAccessibleNameWords(element, 'Choose a theme Theme (required)'); +}); + +test('SelectContext isRequired suppresses optional-label on the visible label', () => { + render( + + + {renderThemeItem} + + , + ); + const element = trigger(/Theme/); + const ids = element.getAttribute('aria-labelledby')?.split(' ') ?? []; + + expect(nativeSelect('context-optional-label')).toBeRequired(); + expect(labelFor(element).textContent).toBe('Theme'); + expect(document.getElementById(ids.at(-1) ?? '')?.textContent).toBe('(required)'); }); test('an errorMessage marks the field invalid and describes the trigger with the error', () => { diff --git a/packages/@luke-ui/react/src/core/switch-field/switch-field.browser.test.tsx b/packages/@luke-ui/react/src/core/switch-field/switch-field.browser.test.tsx index c33c65ab..77f6bd01 100644 --- a/packages/@luke-ui/react/src/core/switch-field/switch-field.browser.test.tsx +++ b/packages/@luke-ui/react/src/core/switch-field/switch-field.browser.test.tsx @@ -157,19 +157,25 @@ test('a disabled or read-only SwitchField keeps its selection', async () => { expect(changes).toEqual([]); }); -// The marker is CSS content with no DOM node, so the accessible name is how the tests observe it. +// Required markers are hidden from assistive technology, so the name does not include them. test('a required SwitchField marks its label with its necessityIndicator', () => { render( <> - - + + , ); - - expect(switchInput('Switch icon*')).toBeInTheDocument(); - expect(switchInput('Switch words(required)')).toBeInTheDocument(); - expect(switchInput('Optional')).toHaveAccessibleName('Optional'); + const asterisk = switchInput('Switch asterisk'); + const words = switchInput('Switch words'); + const optional = switchInput('Optional'); + + expect(asterisk).toBeRequired(); + expect(labelFor(asterisk).textContent).toBe('Switch asterisk\u00A0*'); + expect(words).toBeRequired(); + expect(labelFor(words).textContent).toBe('Switch words\u00A0(required)'); + expect(optional).not.toBeRequired(); + expect(labelFor(optional).textContent).toBe('Optional'); }); test('an errorMessage marks the switch invalid and shows the error under the label', () => { @@ -197,7 +203,7 @@ test('a required SwitchField shows its native validation message after a failed , ); - const input = switchInput('Notifications*'); + const input = switchInput('Notifications'); await userEvent.click(page.getByRole('button', { name: 'Submit' })); @@ -243,7 +249,7 @@ test('the SwitchField scene has no axe violations', async () => { const { container } = render( <> - + , ); @@ -356,7 +362,7 @@ test('necessity markers', { tags: ['visual'] }, async () => { isRequired label="Enable notifications" name="label" - necessityIndicator="label" + necessityIndicator="required-label" /> ; /** - * How a required switch is marked. - * @default 'icon' + * Controls the visible marker for required or optional controls. + * @default 'required-asterisk' */ - necessityIndicator?: FieldNecessityIndicator; + necessityIndicator?: NecessityIndicator; } /** Props for `SwitchField`. */ diff --git a/packages/@luke-ui/react/src/core/test-utils/expect-accessible-name-words.ts b/packages/@luke-ui/react/src/core/test-utils/expect-accessible-name-words.ts new file mode 100644 index 00000000..0fec8a98 --- /dev/null +++ b/packages/@luke-ui/react/src/core/test-utils/expect-accessible-name-words.ts @@ -0,0 +1,68 @@ +import { computeAccessibleName } from 'dom-accessibility-api'; +import { expect } from 'vite-plus/test'; +import { cdp } from 'vite-plus/test/context'; + +/** + * Asserts that Chromium's accessibility tree and `dom-accessibility-api` both name `element` with + * exactly the words in `expected`. Words compare in any order, because the order React Aria joins + * name parts in is not a contract. + */ +export async function expectAccessibleNameWords(element: Element, expected: string): Promise { + const expectedWords = toSortedWords(expected); + const chromiumName = await getChromiumAccessibleName(element); + const domName = computeAccessibleName(element); + expect( + toSortedWords(chromiumName), + `Chromium accessibility tree name: ${JSON.stringify(chromiumName)}`, + ).toEqual(expectedWords); + expect(toSortedWords(domName), `dom-accessibility-api name: ${JSON.stringify(domName)}`).toEqual( + expectedWords, + ); +} + +const WHITESPACE_PATTERN = /\s+/; +const PROBE_ATTRIBUTE = 'data-accessible-name-probe'; + +function toSortedWords(name: string): Array { + return name + .split(WHITESPACE_PATTERN) + .filter((word) => word !== '') + .sort(); +} + +/** Reads the element's name from Chromium's own accessibility tree. */ +async function getChromiumAccessibleName(element: Element): Promise { + const session = cdp(); + const probe = crypto.randomUUID(); + element.setAttribute(PROBE_ATTRIBUTE, probe); + + try { + // Tests run in an iframe, so pierce frames to find the element from the top document. + await session.send('DOM.getDocument', { depth: -1, pierce: true }); + const { resultCount, searchId } = await session.send('DOM.performSearch', { + query: `[${PROBE_ATTRIBUTE}="${probe}"]`, + }); + + try { + if (resultCount !== 1) { + throw new Error(`Expected one element to probe, found ${resultCount}.`); + } + const { nodeIds } = await session.send('DOM.getSearchResults', { + fromIndex: 0, + searchId, + toIndex: 1, + }); + const { nodes } = await session.send('Accessibility.getPartialAXTree', { + fetchRelatives: false, + nodeId: nodeIds[0], + }); + const name = nodes[0]?.name?.value; + if (typeof name !== 'string') throw new Error('Expected Chromium to name the element.'); + return name; + } finally { + await session.send('DOM.discardSearchResults', { searchId }); + } + } finally { + element.removeAttribute(PROBE_ATTRIBUTE); + } +} diff --git a/packages/@luke-ui/react/src/core/text-input-field/text-input-field.browser.test.tsx b/packages/@luke-ui/react/src/core/text-input-field/text-input-field.browser.test.tsx index aa716629..97cd590d 100644 --- a/packages/@luke-ui/react/src/core/text-input-field/text-input-field.browser.test.tsx +++ b/packages/@luke-ui/react/src/core/text-input-field/text-input-field.browser.test.tsx @@ -1,9 +1,11 @@ import { Icon } from '@luke-ui/react/icon'; import { TextInputField } from '@luke-ui/react/text-input-field'; import { createRef } from 'react'; +import { TextFieldContext } from 'react-aria-components/TextField'; import { expect, test } from 'vite-plus/test'; import { page, userEvent } from 'vite-plus/test/context'; import { expectNoAxeViolations } from '../test-utils/axe.js'; +import { expectAccessibleNameWords } from '../test-utils/expect-accessible-name-words.js'; import { getDescribedText } from '../test-utils/get-described-text.js'; import { measureFieldError } from '../test-utils/measure-field-error.js'; import { render, visualAppearances } from '../test-utils/render.js'; @@ -96,6 +98,12 @@ function TextInputFieldScene() { ); } +function textbox(name: string): HTMLInputElement { + const element = page.getByRole('textbox', { name }).element(); + if (!(element instanceof HTMLInputElement)) throw new Error(`Expected an input "${name}".`); + return element; +} + // Luke UI widens RAC's `inputRef` to accept React Hook Form's callback ref. test('TextInputField resolves object and callback inputRef to the input', () => { const inputRef = createRef(); @@ -139,8 +147,8 @@ test('a required field turns invalid only after a submit fails validation', asyn , ); - const email = page.getByRole('textbox', { name: 'Email*' }).element(); - const username = page.getByRole('textbox', { name: 'Username*' }).element(); + const email = page.getByRole('textbox', { name: 'Email' }).element(); + const username = page.getByRole('textbox', { name: 'Username' }).element(); expect(email).not.toHaveAttribute('aria-invalid'); expect(username).not.toHaveAttribute('aria-invalid'); @@ -152,6 +160,124 @@ test('a required field turns invalid only after a submit fails validation', asyn await expect.poll(() => getDescribedText(username).length).toBeGreaterThan(0); }); +// Required markers are hidden from assistive technology, so the name does not include them. +test('a required TextInputField marks its label with its necessityIndicator', () => { + render( + <> + + + + , + ); + const asterisk = textbox('Required asterisk'); + const words = textbox('Required words'); + const optional = textbox('Optional'); + + expect(asterisk).toBeRequired(); + expect(asterisk.labels?.[0]?.textContent).toBe('Required asterisk\u00A0*'); + expect(words).toBeRequired(); + expect(words.labels?.[0]?.textContent).toBe('Required words\u00A0(required)'); + expect(optional).not.toBeRequired(); + expect(optional.labels?.[0]?.textContent).toBe('Optional'); +}); + +test('an optional TextInputField with optional-label includes "(optional)" in its name', async () => { + render(); + + const input = textbox('Middle name (optional)'); + expect(input).not.toBeRequired(); + expect(input.labels?.[0]?.textContent).toBe('Middle name\u00A0(optional)'); + await expectAccessibleNameWords(input, 'Middle name (optional)'); +}); + +test('an aria-label TextInputField with optional-label has no "(optional)" in its name', async () => { + render(); + + const input = textbox('Middle name'); + expect(input.labels).toHaveLength(0); + await expectAccessibleNameWords(input, 'Middle name'); +}); + +for (const appearance of visualAppearances) { + test(`an optional-label TextInputField has no axe violations in ${appearance.theme} ${appearance.mode}`, async () => { + const { container } = render( + , + { appearance }, + ); + await expectNoAxeViolations(container); + }); +} + +test('optional-label', { tags: ['visual'] }, async () => { + for (const appearance of visualAppearances) { + const { locator } = render( + , + { appearance }, + ); + await captureVisualAppearance(locator, 'text-input-field/optional-label', appearance); + } +}); + +const requiredTextFieldContext = { isRequired: true }; + +test('a TextInputField without isRequired keeps isRequired from React Aria context', () => { + render( + + + , + ); + + expect(textbox('Email')).toBeRequired(); +}); + +// RAC mergeProps keeps context `isRequired` when the root omits the prop; FieldStateContext must +// match so optional-label does not show "(optional)" on a required control. +for (const { expectedLabel, necessityIndicator, title } of [ + { + expectedLabel: 'Email\u00A0*', + necessityIndicator: undefined, + title: 'default required-asterisk', + }, + { + expectedLabel: 'Email\u00A0*', + necessityIndicator: 'required-asterisk' as const, + title: 'explicit required-asterisk', + }, + { + expectedLabel: 'Email', + necessityIndicator: 'optional-label' as const, + title: 'optional-label', + }, +]) { + test(`TextFieldContext isRequired drives the ${title} marker`, () => { + render( + + + , + ); + const input = textbox('Email'); + + expect(input).toBeRequired(); + expect(input.labels?.[0]?.textContent).toBe(expectedLabel); + }); +} + +test('an aria-labelledby TextInputField with optional-label has no "(optional)" in its name', async () => { + render( + <> + Middle name + + , + ); + + const input = textbox('Middle name'); + expect(input.labels).toHaveLength(0); + await expectAccessibleNameWords(input, 'Middle name'); +}); + test('an errorMessage marks the field invalid, describes it, and renders its markup', () => { render( **Superseded by [#780](https://github.com/lukebennett88/luke-ui/issues/780):** the public type is +> now `NecessityIndicator` (`'required-asterisk' \| 'required-label' \| 'optional-label'`). There is +> no `FieldNecessityIndicator` alias. + ### `@luke-ui/react/primitives/select` Source: @@ -844,6 +848,10 @@ demonstrated use. `FieldNecessityIndicator` remains the shared public `icon | label` annotation. Accessible-name union, normalisation helpers, field recipe, slot functions and size/state context are private. +> **Superseded by [#780](https://github.com/lukebennett88/luke-ui/issues/780):** the public type is +> now `NecessityIndicator` (`'required-asterisk' \| 'required-label' \| 'optional-label'`). There is +> no `FieldNecessityIndicator` alias. + ### `primitives/text-input` | Part | Specific props, state and defaults | Root/ref, slot and anatomy ownership | diff --git a/vite.config.ts b/vite.config.ts index d750d607..5842ec79 100644 --- a/vite.config.ts +++ b/vite.config.ts @@ -192,6 +192,8 @@ export default defineConfig({ 'react/globals': 'error', 'react/immutability': 'error', 'react/incompatible-library': 'warn', + // The React Compiler memoises context values, so manual `useMemo` is redundant. + 'react/jsx-no-constructed-context-values': 'off', 'react/no-array-index-key': 'off', 'react/preserve-manual-memoization': 'error', 'react/purity': 'error', @@ -227,6 +229,7 @@ export default defineConfig({ 'captureVisual', 'captureVisualAppearance', 'expect', + 'expectAccessibleNameWords', 'expectForwardsDomProps', 'expectNoAxeViolations', 'expectTypeOf',