Skip to content

Commit 02a891e

Browse files
fix(app-shell): page block inspector list labels name their group (#27307)
<!-- claude-projects-attribution: {"github_login":"os-warren"} --> _Requested by **Warren** · [project thread](https://claude.ai/code/project/chan_01U5RdnAAmkSQmZbqh6FtqZ8?thread=cmsg_01U5RdnAAmkSQmZbqh6FtqZ8L4EbhLan2Y7fLAm2Hf41nK)_ Fixes #27256 Clause-②: no Metadata-protocol: none ## 维护者速读 - 改了什么:Studio 页面块检查器里的三个列表编辑器(字段列表如「Columns」、字符串列表、数组)现在各自是一个以其标题命名的分组;只带占位提示的文本行输入框按「标题 + 序号」命名(如「Action names 1」)。 - 为什么改:这三个标题之前不指向任何控件,读屏用户只听到一串无名的行,分不清它们属于哪个属性。 - 风险与回滚:只加 ARIA 属性和一个很小的包装组件,不改数据、写入或布局;回滚即 revert 本 PR。 - 先看:`PageBlockInspector.tsx` 里新增的 `LabelledListGroup` 及三处调用;新测试 `PageBlockInspector.listGroupLabels-27256.test.tsx`。 - 两分钟验证:`pnpm --dir ui exec vitest run packages/app-shell/src/views/metadata-admin/inspectors/PageBlockInspector.listGroupLabels-27256.test.tsx`,预期 4 passed。 - 需要你做的:无,席位落地。 ## What changes `ui/packages/app-shell/src/views/metadata-admin/inspectors/PageBlockInspector.tsx`, the three list editors only: - New `LabelledListGroup` component: the container takes `role="group"` and `aria-labelledby` pointing at the label's `id`, minted with `React.useId()` per instance. It is a component (not inline in `renderField`) because `renderField` is a function run in a loop over array items and cannot call a hook, and a caller-derived id would repeat across array rows. This is the same pattern `ColorPropField` already uses in this file. - `FieldListField` (Columns, Fields, Card fields …), the `string-list` case and the `array` case of `renderField` all render through it. - Row naming (the order asked to check the string-list rows): each string-list row input had no name but a repeated placeholder, so it now carries `aria-label` = the list label plus its 1-based position ("Action names 1"). Chosen over `aria-labelledby` plus an index element because no visible per-row index exists to point at. The field-list text fallback rows (shown when the object has no fields) had the same gap and get the same name. The field-list `Select` trigger rows are left alone: their name is the selected value's text, and an `aria-label` would hide it. `app-shell` is `private: true`, so no changeset (`skip-changeset`). ## Why Each of the three labels headed a list of rows and named nothing: `getByLabelText('Columns')` found the label and no control, and a screen reader heard the rows with no group name. A `for` pairing does not fit, since one label heads several controls. ## Tests `PageBlockInspector.listGroupLabels-27256.test.tsx` (4 cases): - `record:related_list`: `getByRole('group', { name: 'Columns' })` (field list) and `{ name: 'Sort' }` (array) each resolve; the row controls (combobox, `#1`, the remove buttons) sit inside their group; the IDREF resolves to the label element inside the group; the two ids differ. - `record:quick_actions`: the `Action names` group holds textboxes named `Action names 1` and `Action names 2`. - Field-list text fallback (object with no fields): the `Columns` group holds a textbox named `Columns 1`. - `record:details` with two sections: two `Fields` groups inside the `Sections` group, each named by its own label with distinct ids. ## Acceptance notes - Ablation (one-off, no file kept): `scripts/ablation-replace.mjs` swapped the group's `aria-labelledby={labelId}` for an inert attribute → 4 of 4 cases red (`Unable to find an accessible element with the role "group" and name …`); restore verified blob equal to HEAD and `git diff HEAD` empty. A first attempt was refused by the tool before running anything (the replacement text already occurred in the file), so it measured nothing. - The remove buttons in each list share one name ("Remove" / "Remove item") per row; left as is, out of this card's scope. ## Verification All on head `3402dbdd` (base `5b5cff15`), each exit captured before any pipe. - Closure: `pnpm --workspace-concurrency=2 --filter '@object-ui/app-shell^...' build` under the lock: exit 0. - New pin: `pnpm --dir ui exec vitest run packages/app-shell/src/views/metadata-admin/inspectors/PageBlockInspector.listGroupLabels-27256.test.tsx`: 4 passed, exit 0. - Neighbours: all 14 `PageBlockInspector*.test.tsx` files: 14 files, 90 tests passed, exit 0. - `pnpm --filter @object-ui/app-shell run type-check` (source and `tsconfig.test.json`): exit 0. - `pnpm --dir ui run check:new-line-citations`: 0 new citations, exit 0. - `node scripts/pm/dispatch-gates.mjs --commands` derived 24 commands; `--ran` reconciles 24 run, 0 NOT-MEASURED. <details><summary>Derived gates (24)</summary> ```text node scripts/check-closing-keyword-parity.mjs :: exit 0 node scripts/check-closing-keyword-parity.mjs --self-test :: exit 0 node scripts/check-comment-mask-corpus.mjs :: exit 0 node scripts/check-sdui-manifest.mjs :: exit 0 node scripts/check-sdui-manifest.mjs --self-test :: exit 0 pnpm --dir ui exec vitest run scripts/ :: exit 0 pnpm --filter @objectstack/spec run check:duration-unit-keys :: exit 0 pnpm check:cross-package-test-inputs :: exit 0 pnpm check:dispatcher-error-vocabulary :: exit 0 pnpm check:driver-memory-census :: exit 0 pnpm check:engine-double-contract :: exit 0 pnpm check:gitlink-declared :: exit 0 pnpm check:nul-bytes :: exit 0 pnpm check:published-files :: exit 0 pnpm check:query-options-erasure :: exit 0 pnpm check:refd-timer-probe :: exit 0 pnpm check:skill-identifier-liveness :: exit 0 pnpm check:test-source-alias :: exit 0 pnpm check:tier-file-adoption :: exit 0 pnpm check:type-check-coverage :: exit 0 pnpm check:type-check-debt :: exit 0 pnpm check:undeclared-dep-imports :: exit 0 pnpm check:watch-hint-literal :: exit 0 pnpm check:where-matcher :: exit 0 ``` `pnpm --dir ui exec vitest run scripts/` first exited 1 on an unlocked run beside the app-shell type-check: two tree-sweep tests (`check-i18n-dead-keys`, `check-vi-mock-inherit`) hit the 15 s test timeout. Rerun alone under the lock: 2 files, 229 tests passed; then the whole suite under the lock: 174 files passed, 2 skipped, exit 0. The recorded exit is that rerun. </details> ## Human Minimum - [x] 1. Schemas start as Zod; `packages/spec` holds no runtime logic. (not touched) - [x] 2. Every name I added uses the casing for its kind. - [x] 3. No formatter ran; `pnpm lint` passes. (CI runs the repo lint) - [x] 4. One issue, one branch off an up-to-date `main`, one pull request. - [x] 5. A change that publishes carries a changeset, and both declaration lines are filled in here and in the changeset. (publishes nothing: private package) - [x] 6. `packages/spec` touched: its generated artifacts are regenerated and committed. (not touched) - [x] 7. No hand edits to `content/docs/references/`, `packages/*/CHANGELOG.md` or `content/docs/releases/`. - [x] 8. Title, body and comments are in English. (the 维护者速读 section is the dispatch contract's Chinese summary) --- _Generated by [Claude Code](https://claude.ai/code/session_01JQwxbZEzQNxEeTKF7YTgo6)_ Co-authored-by: Claude <noreply@anthropic.com>
1 parent ff26bc4 commit 02a891e

2 files changed

Lines changed: 170 additions & 9 deletions

File tree

Lines changed: 137 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,137 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
/**
4+
* The page block inspector's three LIST editors are groups named by the label
5+
* drawn above them: the field list (`FieldListField`, e.g. Columns), the
6+
* string list and the array editor. Each label heads several rows, so it is
7+
* not a `for` pairing — the container takes `role="group"` and answers the
8+
* label's id through `aria-labelledby`, the WAI-ARIA group pattern
9+
* `ColorPropField` already uses. Before this, `getByLabelText('Columns')` found
10+
* the label and no control, and a screen reader heard the rows with no group
11+
* name.
12+
*
13+
* The id is minted per instance (`React.useId()`), because these editors also
14+
* render inside array rows: the last case draws one field list per
15+
* `record:details` section and pins that each group answers to its own label.
16+
*
17+
* Row inputs that carry only a repeated placeholder are named from the list's
18+
* label plus their position ("Action names 1").
19+
*/
20+
21+
import '@testing-library/jest-dom/vitest';
22+
import * as React from 'react';
23+
import { describe, it, expect, vi, afterEach } from 'vitest';
24+
import { render, screen, within, cleanup } from '@testing-library/react';
25+
26+
const state = vi.hoisted(() => ({
27+
fields: [{ name: 'subject', label: 'Subject', type: 'text', hidden: false }] as Array<Record<string, unknown>>,
28+
}));
29+
30+
// The filter builder lives inside a Radix popover, which does not open in
31+
// jsdom; the popover parts render inline (this directory's existing way).
32+
vi.mock('@object-ui/components', async () => {
33+
const actual = await vi.importActual<Record<string, unknown>>('@object-ui/components');
34+
const Pass = ({ children }: { children?: unknown }) => children as never;
35+
return { ...actual, Popover: Pass, PopoverTrigger: Pass, PopoverContent: Pass };
36+
});
37+
vi.mock('../previews/useObjectFields', () => ({
38+
useObjectFields: () => ({ fields: state.fields, loading: false, error: null }),
39+
}));
40+
vi.mock('../useMetadata', () => ({
41+
useMetadataClient: () => ({ get: vi.fn(async () => undefined), list: vi.fn(async () => [] as unknown[]) }),
42+
}));
43+
44+
import { PageBlockInspector } from './PageBlockInspector';
45+
46+
afterEach(() => {
47+
cleanup();
48+
state.fields = [{ name: 'subject', label: 'Subject', type: 'text', hidden: false }];
49+
});
50+
51+
type Row = Record<string, unknown>;
52+
53+
function renderBlock(type: string, properties: Row) {
54+
return render(
55+
<PageBlockInspector
56+
type="page"
57+
name="lead_page"
58+
draft={{
59+
name: 'lead_page',
60+
type: 'record',
61+
object: 'lead',
62+
kind: 'full',
63+
regions: [{ name: 'main', components: [{ id: 'b1', type, properties }] }],
64+
}}
65+
selection={{ kind: 'block', id: 'regions[0].components[0]' }}
66+
onPatch={() => {}}
67+
onClearSelection={() => {}}
68+
readOnly={false}
69+
locale="en-US"
70+
/>,
71+
);
72+
}
73+
74+
/** The group's IDREF resolves to the label element that carries the name. */
75+
function expectNamedByItsLabel(group: HTMLElement, label: string) {
76+
const labelId = group.getAttribute('aria-labelledby');
77+
expect(labelId).toBeTruthy();
78+
const labelEl = document.getElementById(labelId!);
79+
expect(labelEl).not.toBeNull();
80+
expect(labelEl!.textContent).toBe(label);
81+
expect(group).toContainElement(labelEl);
82+
}
83+
84+
describe('page block inspector — list editors are groups named by their label', () => {
85+
it('names the field list (Columns) and the array editor (Sort), with their rows inside', () => {
86+
renderBlock('record:related_list', {
87+
objectName: 'task',
88+
relationshipField: 'lead',
89+
columns: ['subject'],
90+
sort: [{ field: 'subject', order: 'asc' }],
91+
});
92+
93+
const columns = screen.getByRole('group', { name: 'Columns' });
94+
expectNamedByItsLabel(columns, 'Columns');
95+
expect(within(columns).getAllByRole('combobox')).toHaveLength(1);
96+
expect(within(columns).getByRole('button', { name: 'Remove' })).toBeInTheDocument();
97+
98+
const sort = screen.getByRole('group', { name: 'Sort' });
99+
expectNamedByItsLabel(sort, 'Sort');
100+
expect(within(sort).getByText('#1')).toBeInTheDocument();
101+
expect(within(sort).getByRole('button', { name: 'Remove item' })).toBeInTheDocument();
102+
103+
expect(columns.getAttribute('aria-labelledby')).not.toBe(sort.getAttribute('aria-labelledby'));
104+
});
105+
106+
it('names the string list, and each row input by the label and its position', () => {
107+
renderBlock('record:quick_actions', { actionNames: ['approve', 'reject'] });
108+
109+
const list = screen.getByRole('group', { name: 'Action names' });
110+
expectNamedByItsLabel(list, 'Action names');
111+
expect(within(list).getByRole('textbox', { name: 'Action names 1' })).toHaveValue('approve');
112+
expect(within(list).getByRole('textbox', { name: 'Action names 2' })).toHaveValue('reject');
113+
});
114+
115+
it('names the field list text fallback rows the same way when the object has no fields', () => {
116+
state.fields = [];
117+
renderBlock('record:related_list', { objectName: 'task', columns: ['subject'] });
118+
119+
const columns = screen.getByRole('group', { name: 'Columns' });
120+
expect(within(columns).getByRole('textbox', { name: 'Columns 1' })).toHaveValue('subject');
121+
});
122+
123+
it('mints a distinct id per instance when the same list repeats inside array rows', () => {
124+
renderBlock('record:details', {
125+
sections: [
126+
{ name: 'main', label: 'Main', fields: ['subject'] },
127+
{ name: 'more', label: 'More', fields: ['subject'] },
128+
],
129+
});
130+
131+
const sections = screen.getByRole('group', { name: 'Sections' });
132+
const fieldLists = within(sections).getAllByRole('group', { name: 'Fields' });
133+
expect(fieldLists).toHaveLength(2);
134+
fieldLists.forEach((g) => expectNamedByItsLabel(g, 'Fields'));
135+
expect(fieldLists[0].getAttribute('aria-labelledby')).not.toBe(fieldLists[1].getAttribute('aria-labelledby'));
136+
});
137+
});

‎ui/packages/app-shell/src/views/metadata-admin/inspectors/PageBlockInspector.tsx‎

Lines changed: 33 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -114,6 +114,28 @@ function FieldPickerField({ label, objectName, value, onCommit, disabled }: {
114114
return <InspectorSelectField label={label} value={value || undefined} options={options} onCommit={onCommit} disabled={disabled} />;
115115
}
116116

117+
/**
118+
* The visible label over a LIST of rows, and the group it names. A label that
119+
* heads several controls is not a `for` pairing (that names one control), so
120+
* the container takes `role="group"` and answers the label's id by IDREF —
121+
* `ColorPropField`'s pattern. Its own component for the same reason as that
122+
* one: the id is minted with `React.useId()` per instance, and `renderField`
123+
* (a function run in a loop over array items, not a component) cannot call a
124+
* hook, while a caller-derived id would repeat across two array rows carrying
125+
* the same property and resolve to the first group.
126+
*/
127+
function LabelledListGroup({ label, className, children }: {
128+
label: string; className: string; children: React.ReactNode;
129+
}) {
130+
const labelId = React.useId();
131+
return (
132+
<div role="group" aria-labelledby={labelId} className={className}>
133+
<Label id={labelId} className="text-xs text-muted-foreground">{label}</Label>
134+
{children}
135+
</div>
136+
);
137+
}
138+
117139
/** Editable list of field names — each row a field dropdown (or text fallback). */
118140
function FieldListField({ label, objectName, value, onChange, disabled, locale }: {
119141
label: string; objectName: string | undefined; value: unknown; onChange: (v: string[]) => void; disabled?: boolean;
@@ -122,8 +144,7 @@ function FieldListField({ label, objectName, value, onChange, disabled, locale }
122144
const options = useFieldOptions(objectName);
123145
const arr: string[] = Array.isArray(value) ? (value as string[]) : [];
124146
return (
125-
<div className="space-y-1.5">
126-
<Label className="text-xs text-muted-foreground">{label}</Label>
147+
<LabelledListGroup label={label} className="space-y-1.5">
127148
{arr.map((s, i) => (
128149
<div key={i} className="flex items-center gap-1.5">
129150
{options.length > 0 ? (
@@ -137,6 +158,7 @@ function FieldListField({ label, objectName, value, onChange, disabled, locale }
137158
</div>
138159
) : (
139160
<Input className="h-8 text-sm" value={String(s ?? '')} placeholder={t('engine.inspector.pageBlock.fieldPlaceholder', locale)} disabled={disabled}
161+
aria-label={`${label} ${i + 1}`}
140162
onChange={(e) => { const n = [...arr]; n[i] = e.target.value; onChange(n); }} />
141163
)}
142164
<Button type="button" variant="ghost" size="icon" className="h-8 w-8" disabled={disabled}
@@ -151,7 +173,7 @@ function FieldListField({ label, objectName, value, onChange, disabled, locale }
151173
<Plus className="mr-1 h-3.5 w-3.5" /> {t('engine.inspector.pageBlock.list.add', locale)}
152174
</Button>
153175
)}
154-
</div>
176+
</LabelledListGroup>
155177
);
156178
}
157179

@@ -625,12 +647,15 @@ export function PageBlockInspector({ selection, draft, onPatch, onClearSelection
625647
);
626648
case 'string-list': {
627649
const arr = Array.isArray(read(f.name)) ? (read(f.name) as unknown[]) : [];
650+
// Each row's input is named from the list's label plus its position:
651+
// the placeholder is the only other text it carries, and it repeats.
652+
const label = fieldLabel(f.label);
628653
return (
629-
<div key={k} className="space-y-1.5">
630-
<Label className="text-xs text-muted-foreground">{fieldLabel(f.label)}</Label>
654+
<LabelledListGroup key={k} label={label} className="space-y-1.5">
631655
{arr.map((s, i) => (
632656
<div key={i} className="flex items-center gap-1.5">
633657
<Input className="h-8 text-sm" value={String(s ?? '')} placeholder={placeholderText(f.placeholder)} disabled={readOnly}
658+
aria-label={`${label} ${i + 1}`}
634659
onChange={(e) => { const next = [...arr]; next[i] = e.target.value; write(f.name, next); }} />
635660
<Button type="button" variant="ghost" size="icon" className="h-8 w-8" disabled={readOnly}
636661
aria-label={t('engine.inspector.pageBlock.list.remove', locale)}
@@ -644,7 +669,7 @@ export function PageBlockInspector({ selection, draft, onPatch, onClearSelection
644669
<Plus className="mr-1 h-3.5 w-3.5" /> {t('engine.inspector.pageBlock.list.add', locale)}
645670
</Button>
646671
)}
647-
</div>
672+
</LabelledListGroup>
648673
);
649674
}
650675
case 'array': {
@@ -654,8 +679,7 @@ export function PageBlockInspector({ selection, draft, onPatch, onClearSelection
654679
if (read(f.name) !== undefined && !Array.isArray(read(f.name))) return jsonFallback();
655680
const arr = Array.isArray(read(f.name)) ? (read(f.name) as unknown[]) : [];
656681
return (
657-
<div key={k} className="space-y-2">
658-
<Label className="text-xs text-muted-foreground">{fieldLabel(f.label)}</Label>
682+
<LabelledListGroup key={k} label={fieldLabel(f.label)} className="space-y-2">
659683
{arr.map((item, i) => {
660684
const itemObj = item && typeof item === 'object' ? (item as Record<string, unknown>) : {};
661685
return (
@@ -684,7 +708,7 @@ export function PageBlockInspector({ selection, draft, onPatch, onClearSelection
684708
<Plus className="mr-1 h-3.5 w-3.5" /> {fieldLabel(f.addLabel)}
685709
</Button>
686710
)}
687-
</div>
711+
</LabelledListGroup>
688712
);
689713
}
690714
case 'object-picker':

0 commit comments

Comments
 (0)