diff --git a/.changeset/10435-select-label-association.md b/.changeset/10435-select-label-association.md new file mode 100644 index 0000000000..2a10481901 --- /dev/null +++ b/.changeset/10435-select-label-association.md @@ -0,0 +1,20 @@ +--- +'@object-ui/components': patch +--- + +The `ui:select` form renderer's `label` now names its combobox (objectui#10435). + +Before, the renderer drew its `Label` with no `htmlFor` and the Radix +`SelectTrigger` with no `id`, and nothing set `aria-label` or +`aria-labelledby`. The trigger is a `button` with `role="combobox"`, a role +that takes no name from its content, so the placeholder and value text inside +it did not name it either: a labelled select's combobox had an empty accessible +name, and a screen reader announced an unnamed combobox. + +The fix is the shape `element:record_picker` landed for the same defect +(objectui#5771): the trigger gets the node's `id`, and the label gets `htmlFor` +to it. A labelled select authored with an `id` is now named by its label, and a +required one's name leaves out the `*`, because the required marker is an +`aria-hidden` element (objectui#10368). A select authored without an `id` +is unchanged: the renderer mints no id, so its caption stays unassociated, as +on the sibling `input` and `textarea` renderers. diff --git a/packages/components/src/__tests__/required-marker-markup.test.tsx b/packages/components/src/__tests__/required-marker-markup.test.tsx index 6b672a7f73..893f1db926 100644 --- a/packages/components/src/__tests__/required-marker-markup.test.tsx +++ b/packages/components/src/__tests__/required-marker-markup.test.tsx @@ -133,11 +133,9 @@ const SITES: Site[] = [ required, options: [{ label: 'A', value: 'a' }], }), - // This label carries no `for` today. Whether it SHOULD name the trigger is - // outside objectui#10368 and deliberately not pinned either way here; the - // marker must be an element regardless, because it would enter the name - // the moment the label is associated. - label: () => document.querySelector('label'), + // The label names the trigger through `for` since objectui#10435, whose + // own pin (`select-label-association.test.tsx`) reads the resulting name. + label: () => document.querySelector('label[for="sel-ctl"]'), // Radix writes `aria-required` on the combobox trigger from `required`. expectRequiredState: () => expect(screen.getByRole('combobox')).toHaveAttribute('aria-required', 'true'), }, diff --git a/packages/components/src/renderers/form/__tests__/select-label-association.test.tsx b/packages/components/src/renderers/form/__tests__/select-label-association.test.tsx new file mode 100644 index 0000000000..d5941deae0 --- /dev/null +++ b/packages/components/src/renderers/form/__tests__/select-label-association.test.tsx @@ -0,0 +1,128 @@ +/** + * ObjectUI + * Copyright (c) 2024-present ObjectStack Inc. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ + +/** + * `ui:select` — the authored `label` names the combobox (objectui#10435). + * + * Pre-fix, the renderer drew its `Label` with no `htmlFor` and the Radix + * `SelectTrigger` with no `id`, and nothing set `aria-label` / + * `aria-labelledby`. The trigger is a `button` with `role="combobox"`, a role + * that takes no name from its content, so neither the placeholder nor the + * value text named it: a labelled select's combobox had an EMPTY accessible + * name, and `getByRole('combobox', { name: LABEL })` could not find it. + * + * The fix is the `element:record_picker` shape (objectui#5771): the trigger + * gets `schema.id`, the label gets `htmlFor` to it. So every associated case + * below authors an `id`, and the no-`id` case pins that the renderer mints + * none, the same deliberate half that shape pins for the picker. + * + * Each case resolves the association end to end (`getByRole` by name, + * `toHaveAccessibleName`, `label[for]` through `document.getElementById`, the + * trigger's `labels` collection), never `htmlFor` and `id` read as two + * independent strings: two values that are each present and never match is the + * failure mode objectui#3341 documents. + */ + +import { describe, it, expect, afterEach } from 'vitest'; +import { render, screen, cleanup } from '@testing-library/react'; +import { SchemaRenderer } from '@object-ui/react'; +import { renderComponent } from '../../../__tests__/test-utils'; +// Registers `select` at module scope, not in a hook +// (object-ui/no-dynamic-import-in-test-hook, objectui#3010). +import '../../../renderers'; + +afterEach(cleanup); + +const OPTIONS = [ + { label: 'Mr', value: 'mr' }, + { label: 'Ms', value: 'ms' }, +]; + +describe('ui:select — label names the combobox (objectui#10435)', () => { + it('is found by getByRole combobox with the label as its name', () => { + renderComponent({ type: 'select', id: 'title', label: 'Title', options: OPTIONS }); + + const combobox = screen.getByRole('combobox', { name: 'Title' }); + expect(combobox).toHaveAccessibleName('Title'); + expect(screen.getByLabelText('Title')).toBe(combobox); + }); + + it('keeps the name `Title` when required: the aria-hidden marker stays out of it', () => { + renderComponent({ type: 'select', id: 'title', label: 'Title', required: true, options: OPTIONS }); + + // The marker is still drawn inside the label (objectui#10368) ... + const label = document.querySelector('label[for="title"]'); + expect(label).not.toBeNull(); + expect(label!.querySelector('[data-required-marker][aria-hidden="true"]')).toHaveTextContent('*'); + + // ... and the name the label now gives the combobox excludes it. + const combobox = screen.getByRole('combobox', { name: 'Title' }); + expect(combobox).toHaveAccessibleName('Title'); + expect(combobox).toHaveAttribute('aria-required', 'true'); + }); + + it('points `for` at an id that really exists on the trigger', () => { + renderComponent({ type: 'select', id: 'title', label: 'Title', options: OPTIONS }); + + const label = document.querySelector('label[for]'); + expect(label).not.toBeNull(); + const target = document.getElementById(label!.getAttribute('for')!); + expect(target).not.toBeNull(); + expect(target).toBe(screen.getByRole('combobox')); + + // The association browsers use for both the name and click-to-focus. + const trigger = target as HTMLButtonElement; + expect(Array.from(trigger.labels ?? [])).toEqual([label]); + }); + + it('names the combobox with the label, not the placeholder text inside it', () => { + renderComponent({ type: 'select', id: 'title', label: 'Title', placeholder: 'Choose…', options: OPTIONS }); + + const combobox = screen.getByRole('combobox'); + expect(combobox).toHaveTextContent('Choose…'); + expect(combobox).toHaveAccessibleName('Title'); + }); + + it('names the combobox with the label, not the selected value text inside it', () => { + renderComponent({ type: 'select', id: 'title', label: 'Title', value: 'ms', options: OPTIONS }); + + const combobox = screen.getByRole('combobox'); + expect(combobox).toHaveTextContent('Ms'); + expect(combobox).toHaveAccessibleName('Title'); + }); + + it('mints no id when `schema.id` is absent: the caption stays unassociated', () => { + // The deliberate half of the objectui#5771 shape: only the author supplies + // `schema.id`, so without one the renderer neither writes `for` nor mints + // an `id`, and the combobox stays unnamed, as before this change. Pinned so + // a later change to that answer is a visible decision, not a drift. + renderComponent({ type: 'select', label: 'Title', placeholder: 'Choose…', options: OPTIONS }); + + const combobox = screen.getByRole('combobox'); + expect(combobox).toHaveTextContent('Choose…'); + const label = document.querySelector('label'); + expect(label).not.toBeNull(); + expect(label).not.toHaveAttribute('for'); + expect(combobox).not.toHaveAttribute('id'); + expect(combobox).toHaveAccessibleName(''); + }); + + it('survives the real render path through SchemaRenderer', () => { + render( + , + ); + + // By role and name, not `getByLabelText('Title')`: that query matches the + // label's whole text content, which on a required field includes the + // aria-hidden `*` the accessible name leaves out. + const combobox = screen.getByRole('combobox', { name: 'Title' }) as HTMLButtonElement; + expect(Array.from(combobox.labels ?? [])).toEqual([document.querySelector('label[for="title"]')]); + }); +}); diff --git a/packages/components/src/renderers/form/select.tsx b/packages/components/src/renderers/form/select.tsx index 4296933c05..8fc729cbd2 100644 --- a/packages/components/src/renderers/form/select.tsx +++ b/packages/components/src/renderers/form/select.tsx @@ -47,8 +47,19 @@ const SelectRenderer = ({ schema, className, onChange, value, disabled: hostDisa data-obj-type={dataObjType} style={style} > + {/* + The label names the trigger through a plain `htmlFor` / `id` pair on + `schema.id` (objectui#10435), the shape `element:record_picker` landed + for the same defect (objectui#5771): `SelectTrigger` renders a `button` + with `role="combobox"`, a labelable element, and that role takes no + name from its content, so without the pair the placeholder or value + text inside it named nothing and the combobox's accessible name was + empty. Only the author supplies `schema.id`, so a select authored + without one keeps an unassociated caption, as on the sibling `input` / + `textarea` renderers, rather than a minted `useId()` fallback. + */} {schema.label && ( -