Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 20 additions & 0 deletions .changeset/10435-select-label-association.md
Original file line number Diff line number Diff line change
@@ -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.
Original file line number Diff line number Diff line change
Expand Up @@ -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'),
},
Expand Down
Original file line number Diff line number Diff line change
@@ -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(
<SchemaRenderer
schema={{ type: 'select', id: 'title', label: 'Title', required: true, options: OPTIONS }}
/>,
);

// 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"]')]);
});
});
15 changes: 13 additions & 2 deletions packages/components/src/renderers/form/select.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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 && (
<Label className={cn(schema.required && "text-destructive")}>
<Label htmlFor={schema.id} className={cn(schema.required && "text-destructive")}>
{schema.label}
{schema.required && (
// A real `aria-hidden` element, not CSS generated content, the
Expand All @@ -72,7 +83,7 @@ const SelectRenderer = ({ schema, className, onChange, value, disabled: hostDisa
name={schema.name}
{...selectProps}
>
<SelectTrigger className={className}>
<SelectTrigger id={schema.id} className={className}>
<SelectValue placeholder={schema.placeholder} />
</SelectTrigger>
<SelectContent>
Expand Down
Loading