Skip to content

Commit 7858c1d

Browse files
fix(app-shell): name the page block inspector's JSON editor by its label (#27254)
<!-- claude-projects-attribution: {"github_login":"os-warren"} --> _Requested by **Warren** · [project thread](https://claude.ai/code/project/chan_01U5RdnAAmkSQmZbqh6FtqZ8?thread=cmsg_01U5RdnAAmkSQmZbqh6FtqZ8D1GfomwtVv1LMaZwNA9aHS)_ Fixes #27178 Clause-②: no Metadata-protocol: none ## 维护者速读 - 改了什么:Studio 页面设计器的块检查器里,JSON 编辑框(`InspectorJsonField`)的标签现在用 `htmlFor` 指向它的 textarea(id 由 `React.useId()` 生成)。 - 为什么改:原来标签和输入框没有关联,读屏器只读出一个没有名字的文本框,测试也无法按标签找到这个字段。 - 风险与回滚:只加了 label 与 textarea 的 id 配对,不改写入的数据和提交逻辑;回滚即 revert 本 PR。 - 先看:`PageBlockInspector.tsx` 中的 `InspectorJsonField`;新测试 `PageBlockInspector.jsonFieldLabel-27178.test.tsx`。 - 两分钟验证:`pnpm --dir ui exec vitest run packages/app-shell/src/views/metadata-admin/inspectors/PageBlockInspector.jsonFieldLabel-27178.test.tsx`,预期 2/2 通过。 - 需要你做的:无,席位落地。 ## What changes `InspectorJsonField` in `PageBlockInspector.tsx` mints an id with `React.useId()` and pairs it: `Label htmlFor={id}` and the textarea's `id={id}`. This is the same scheme `FilterRulesField` and `ColorPropField` in the same file already use. The id is minted inside the component, not derived from the label or prop name, because `renderField` also draws this editor inside array rows, where a caller-derived id would repeat. Every door that draws the editor gets the pairing, because they all go through the one component: a curated `kind: 'json'` prop (for example `record:related_list.add`, `element:button.action`), the JSON fallback for a stored value in another union arm (`sort` stored as a string, `columns` as `ListColumn` objects), and the generic "Advanced" editor for object or array props. There was no `aria-describedby` or error wiring on the field to keep; the invalid-JSON message is unchanged. ## Tests New `PageBlockInspector.jsonFieldLabel-27178.test.tsx`, on the real inspector inside a host that applies every patch: - `getByLabelText('Add existing records (picker)')` returns the `add` textarea, its accessible name is that label, and typing JSON and blurring writes `add` in the stored shape. - With `sort: 'subject'` stored, `getByLabelText('Sort')` returns the JSON fallback textarea showing `"subject"`, and the `Sort` and `add` editors resolve to two different textareas with two different ids. At `bc69c4ad`: every `PageBlockInspector*` test file in the directory, 13 files, 86 tests passed. `pnpm --filter @object-ui/app-shell run type-check` (both the source and the test tsconfig): exit 0. `eslint` on the two changed files: exit 0, no new warning. <details><summary>Ablation (drop the htmlFor / id pair)</summary> Through `scripts/ablation-replace.mjs`, on the committed fix: anchor hits 1 to 0, blob `e552a33e` to `1da05b77`. Both new tests turn red with the card's own wording: `Found a label with the text of: Add existing records (picker), however no form control was found associated to that label` and the same for `Sort`. Restore: blob back to the HEAD blob `e552a33e`, `git diff HEAD` empty. </details> ## Acceptance notes - Changeset: none. `@object-ui/app-shell` is `private: true`, so the PR carries `skip-changeset`. - The `jsonEditorUnder` helper in `PageBlockInspector.relatedListProps-27054.test.tsx` says the label "is a sibling, not a `for`". That comment is now out of date; the helper still works because the label and the textarea share a parent. Left as is (outside this card's file surface). - Three other labels in the same file still name no control: `FieldListField`'s label, and the `string-list` and `array` labels in `renderField`. Each labels a group of rows, so the fix there is a group name (`role="group"` with `aria-labelledby`), not this `for` pairing. Reported to the seat as a finding; not changed here. ## Verification Derived gates at `e781cf8f` (`node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack`, 24 families), every one run locally, exit codes captured before any pipe: all 24 exit 0, including `node scripts/check-sdui-manifest.mjs` and its `--self-test`, and `pnpm --dir ui exec vitest run scripts/` under the verify lock. `dispatch-gates --ran`: 24 derived, 24 run, 0 NOT-MEASURED. `dispatch-gates --tier`: no path-derived mandate. 109 changed lines, under the 3000 threshold; no governed path. Every family CI runs is read from this PR's check-runs. ## Human Minimum - [x] 1. Schemas start as Zod; `packages/spec` holds no runtime logic. - [x] 2. Every name I added uses the casing for its kind. - [x] 3. No formatter ran; `pnpm lint` passes. - [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. - [x] 6. `packages/spec` touched: its generated artifacts are regenerated and committed. - [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. - [x] The *Before Submitting* list in CONTRIBUTING.md is done. 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01JQwxbW6EicqdfE9onssvrp --- _Generated by [Claude Code](https://claude.ai/code/session_01JQwxbW6EicqdfE9onssvrp)_ --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent c13ef04 commit 7858c1d

2 files changed

Lines changed: 108 additions & 1 deletion

File tree

Lines changed: 102 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,102 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
/**
4+
* The page block inspector's JSON editor (`InspectorJsonField`) is named by the
5+
* label drawn above it: `Label htmlFor` ↔ textarea `id`, the pairing the
6+
* inspector's other rows already carry. Without it every curated JSON prop
7+
* exposed an unnamed text box to a screen reader, and `getByLabelText` found
8+
* the label but "no form control … associated to that label".
9+
*
10+
* Both doors that draw the editor are read through their label here:
11+
* - a curated `kind: 'json'` prop (`record:related_list.add`), unset, which is
12+
* the only door that can ADD the key;
13+
* - the JSON fallback for a stored value in another arm of a union
14+
* (`record:related_list.sort` stored as a string).
15+
* Two editors in one panel each resolve to their own textarea.
16+
*/
17+
18+
import '@testing-library/jest-dom/vitest';
19+
import * as React from 'react';
20+
import { describe, it, expect, vi, afterEach } from 'vitest';
21+
import { render, screen, fireEvent, cleanup } from '@testing-library/react';
22+
23+
// The filter builder lives inside a Radix popover, which does not open in
24+
// jsdom; the popover parts render inline (this directory's existing way).
25+
vi.mock('@object-ui/components', async () => {
26+
const actual = await vi.importActual<Record<string, unknown>>('@object-ui/components');
27+
const Pass = ({ children }: { children?: unknown }) => children as never;
28+
return { ...actual, Popover: Pass, PopoverTrigger: Pass, PopoverContent: Pass };
29+
});
30+
vi.mock('../previews/useObjectFields', () => ({
31+
useObjectFields: () => ({
32+
fields: [{ name: 'subject', label: 'Subject', type: 'text', hidden: false }],
33+
loading: false,
34+
error: null,
35+
}),
36+
}));
37+
vi.mock('../useMetadata', () => ({
38+
useMetadataClient: () => ({ get: vi.fn(async () => undefined), list: vi.fn(async () => [] as unknown[]) }),
39+
}));
40+
41+
import { PageBlockInspector } from './PageBlockInspector';
42+
43+
afterEach(() => cleanup());
44+
45+
type Row = Record<string, unknown>;
46+
47+
function Host({ props, writes }: { props: Row; writes: Row[] }) {
48+
const [draft, setDraft] = React.useState<Row>({
49+
name: 'lead_page',
50+
type: 'record',
51+
object: 'lead',
52+
kind: 'full',
53+
regions: [{ name: 'main', components: [{ id: 'tasks', type: 'record:related_list', properties: props }] }],
54+
});
55+
return (
56+
<PageBlockInspector
57+
type="page"
58+
name="lead_page"
59+
draft={draft}
60+
selection={{ kind: 'block', id: 'regions[0].components[0]' }}
61+
onPatch={(patch: Row) => setDraft((d) => {
62+
const next = { ...d, ...patch };
63+
const regions = next.regions as Array<{ components: Array<{ properties: Row }> }>;
64+
writes.push(regions[0].components[0].properties);
65+
return next;
66+
})}
67+
onClearSelection={() => {}}
68+
readOnly={false}
69+
locale="en-US"
70+
/>
71+
);
72+
}
73+
74+
const BASE = { objectName: 'task', relationshipField: 'lead' };
75+
const ADD_LABEL = 'Add existing records (picker)';
76+
77+
describe('page block inspector — the JSON editor is named by its label', () => {
78+
it('finds a curated JSON prop by its label, and the textarea it finds is the editor', () => {
79+
const writes: Row[] = [];
80+
render(<Host props={BASE} writes={writes} />);
81+
const box = screen.getByLabelText(ADD_LABEL);
82+
expect(box.tagName).toBe('TEXTAREA');
83+
expect(box).toHaveAccessibleName(ADD_LABEL);
84+
85+
const add = { picker: { object: 'sys_user' }, linkField: 'user_id' };
86+
fireEvent.change(box, { target: { value: JSON.stringify(add) } });
87+
fireEvent.blur(box);
88+
expect(writes.at(-1)!.add).toEqual(add);
89+
});
90+
91+
it('names the JSON fallback of a stored other-arm value, and each editor keeps its own id', () => {
92+
render(<Host props={{ ...BASE, sort: 'subject' }} writes={[]} />);
93+
const sort = screen.getByLabelText('Sort');
94+
expect(sort.tagName).toBe('TEXTAREA');
95+
expect((sort as HTMLTextAreaElement).value).toBe('"subject"');
96+
97+
const add = screen.getByLabelText(ADD_LABEL);
98+
expect(add).not.toBe(sort);
99+
expect(sort.id).not.toBe('');
100+
expect(add.id).not.toBe(sort.id);
101+
});
102+
});

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

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -180,6 +180,10 @@ function InspectorJsonField({ label, value, onCommit, disabled, placeholder, loc
180180
// message is resolved at render and follows a locale switch instead of
181181
// freezing whatever language was active when the parse failed.
182182
const [invalid, setInvalid] = React.useState(false);
183+
// Minted here, not derived from the label or the prop name: `renderField`
184+
// draws this editor inside array rows too, where a caller-derived id would
185+
// repeat and `for` would resolve to the first row's textarea.
186+
const id = React.useId();
183187
React.useEffect(() => { setText(initial); setInvalid(false); }, [initial]);
184188
const commit = () => {
185189
if (disabled) return;
@@ -195,8 +199,9 @@ function InspectorJsonField({ label, value, onCommit, disabled, placeholder, loc
195199
};
196200
return (
197201
<div className="space-y-1">
198-
<Label className="text-xs text-muted-foreground">{label}</Label>
202+
<Label htmlFor={id} className="text-xs text-muted-foreground">{label}</Label>
199203
<textarea
204+
id={id}
200205
value={text}
201206
onChange={(e) => setText(e.target.value)}
202207
onBlur={commit}

0 commit comments

Comments
 (0)