Skip to content

Commit 0d6d406

Browse files
fix(studio): explain each control the field inspector locks in display-patch mode (#29081)
<!-- claude-projects-attribution: {"github_login":"zhuangjianguo"} --> _Requested by **Steedos** · [project thread](https://claude.ai/code/project/chan_018ETKaNwMZCNRYD7D8Q9Ubb?thread=cmsg_018ETKaNwMZCNRYD7D8Q9UbbSoAeFWE57GxG3dEVs9Yx86)_ Fixes #28986 Clause-②: no Metadata-protocol: none ## 维护者速读 - **改了什么**:Studio 字段检查器在「只能改显示属性」模式(只读基础包的字段,经配对的 `.custom` 包定制)下,每个被锁住的控件(API 名、类型、必填/唯一、默认值、选项值与颜色、跟踪历史、三个条件规则编辑器、上移/下移、删除等)外面多了一层可 Tab 聚焦的包装:悬停或键盘聚焦时弹出提示,读屏通过 aria-describedby 读到顶部那句「此字段属于已安装的包:你只能修改它的……」。可编辑的标签、描述、帮助文本、占位符、选项标签不加包装、不带原因。 - **为什么改**:#28860 让基础包字段可以写显示补丁后,整栏的只读原因包装(objectui#12303)不再绘制,被锁控件变成灰着却说不出原因,`ux-conventions.disabled-control-reason` 的回归行由 PASS 变 FAIL;这是 18.3.0 发版前的回归。 - **风险与回滚**:低。只改检查器的渲染:新增的包装只在显示补丁模式下出现,普通可写字段和整栏只读两种情形 DOM 不变(各有对照测试钉住);不新增文案、不碰保存逻辑。代价是该模式下 Tab 序列里多了若干个停靠点(每个被锁控件或一行一组一个)。回滚即还原本 PR。 - **先看**:`ui/packages/app-shell/src/views/metadata-admin/inspectors/ObjectFieldInspector.tsx` 的 `DisplayPatchLock` 与 `DisplayPatchReasonContext`,然后同文件 `OptionsEditor` 里选项值与选项工具条的包装;测试 `ObjectFieldInspector.displayPatchReason-28986.test.tsx`。 - **两分钟验证**:`pnpm --dir ui exec vitest run packages/app-shell/src/views/metadata-admin/inspectors/ObjectFieldInspector.displayPatchReason-28986.test.tsx`,预期 1 个文件、16 个测试全绿。 - **需要你做的**:无,席位落地。 ## What changes - **One lock, one reason.** `ObjectFieldInspector` gains a module-private `DisplayPatchLock`. In display-patch mode (`BaseFieldDisplayPatchContext` on, `readOnly` off) it wraps a locked control, or one row of them, in a `role="group"` tab stop with `data-disabled-reason`. `aria-describedby` on that wrapper points at the existing whitelist sentence at the top of BASIC (`field-base-patch-only`, which now carries an id), and a Radix tooltip shows the same sentence on hover and on focus. This is the shape `DeclaredActionsBar` (objectui#11811) and the read-only rail (objectui#12303) already use. In any other mode the lock renders its children unchanged. - **What is wrapped:** API name, Type, the Required/Unique row, Default value, the picklist source, the related object and the lookup and related-list blocks, the formula editor, the summary block, the numeric and text-length grids, currency, value domain, the Read-only/Hidden/Track history row, the conditional-rules block, Field group, the header's Move up/down pair and the footer's Remove. Inside the options editor, each option's value input and its colour/kind/reorder/remove strip are wrapped, and a malformed row's controls too. Each option's label input is not. - **The reason is provided above the shell.** `InspectorShell` draws the header and footer itself, so the inspector hands the reason through a context provided around the shell, and the options editor reads it from there too. - **No new text.** The reason is the sentence the inspector already shows ("This field belongs to the installed package: you can change only its …"), which names the condition, so the checklist's specificity clause reads it as specific. - **`StudioDesignSurface.tsx`:** a comment only, at `fieldRailReadOnly`, saying where the reason for a base field's locked controls now lives. The rail's read-only wrapper is unchanged. ## Tests - New `ObjectFieldInspector.displayPatchReason-28986.test.tsx` mounts the real inspector under the display-patch context, over six fields (text, select with options, lookup, number, formula, currency): - every control rendered disabled (attribute, `aria-disabled` or Radix `data-disabled`) sits in a `[data-disabled-reason]` wrapper with `tabindex="0"` whose `aria-describedby` resolves to the whitelist sentence; - every wrapper contains at least one disabled control (no empty tab stop), and Label, Description, Help text, Placeholder and each option label are enabled with no wrapper and no reason; - the card's named controls (API name, Required, Track history, a rule editor, every Move up, Remove, an option value) are each described by the sentence's id; - Tab reaches the API name lock and its tooltip shows the sentence; hovering the Required lock shows it too; - CONTROL: a writable field draws no lock and nothing disabled; a read-only rail (patch context on or off) draws no inner lock, since the host describes the whole rail. - `DataPillar.baseObjectExtension-28575.test.tsx` (the real Data pillar): the `required` case now also asserts the Required box's wrapper is a tab stop described by the sentence, and that the Label input has no wrapper. ## Acceptance notes - **Live browser not measured.** The checklist row's oracle is the showcase boot in a real browser. This PR is verified in jsdom only. The wrapper is the same tab-stop-plus-tooltip shape the read-only rail uses, which the checklist's QA run measured PASS for hover and keyboard in Chromium; the next QA pass of `ux-conventions.disabled-control-reason` re-measures it here. - **Tab stops.** One stop per locked control or row, and per option two (value, then the strip). That is the cost of keeping the editable controls undescribed between them. - **Scope.** The external-object locks (`ddlLocked` from an external binding) keep their own badge and are unchanged. ## Verification <details><summary>New pin · 16/16, and 15 red against main's inspector</summary> `pnpm --dir ui exec vitest run --maxWorkers=2 packages/app-shell/src/views/metadata-admin/inspectors/ObjectFieldInspector.displayPatchReason-28986.test.tsx` under `os-verify-lock`: Test Files 1 passed (1), Tests 16 passed (16). Reverse check at `97144b76`: `ObjectFieldInspector.tsx` restored from `origin/main` (`ab085a04`, `DisplayPatchLock` count 0 on disk), the new file plus `DataPillar.baseObjectExtension-28575.test.tsx` run: Tests 15 failed, 11 passed. The 14 display-patch cases and the 28575 `required` case failed; both CONTROL cases and the rest of 28575 passed. Restored with `git checkout HEAD --`, blob hash back to `4509f157`, `git diff HEAD` empty. </details> <details><summary>Every test file naming the field inspector · 61 files</summary> `git grep -l -E "ObjectFieldInspector|field-base-patch|BaseFieldDisplayPatchContext|fieldRailReadOnly|data-field-inspector"` over ui test files: 61 files in two locked batches at `--maxWorkers=2`. Test Files 31 passed, Tests 215 passed; Test Files 30 passed, Tests 400 passed. This is a declared narrowing; CI's Test UI runs the rest of app-shell. </details> <details><summary>Type check and lint</summary> - `turbo run type-check --filter=@object-ui/app-shell` (65 tasks, includes `tsc -p tsconfig.test.json`): exit 0. - `eslint` on the three changed `.tsx` files: 0 errors, 15 warnings, all on lines that predate this diff (re-indented `any` casts in the rule editors, and two existing hook warnings). - `pnpm --dir ui run check:new-line-citations`: 0 new citations. </details> <details><summary>Derived gates</summary> `node scripts/pm/dispatch-gates.mjs --commands` derived 34 commands against merge base `ab085a048` at head `97144b76`. All 34 ran and exited 0, including `check-sdui-manifest.mjs` and its `--self-test`, `check:type-check-debt`, `check:nul-bytes`, `check:cross-package-test-inputs`, `check:test-source-alias` and `pnpm --dir ui exec vitest run scripts/` (Test Files 169 passed, 2 skipped; Tests 4900 passed, 2 skipped). `dispatch-gates --ran` reconciled 34 derived, 34 run, 0 NOT-MEASURED, 0 UNRUN. `origin/main` moved 9 commits since the base, none touching the three changed source files. </details> --- _Generated by [Claude Code](https://claude.ai/code/session_01QsR2jVM5iRVAwVwYSkX5u7)_ --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent c503b95 commit 0d6d406

5 files changed

Lines changed: 672 additions & 335 deletions

File tree

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
---
2+
'@object-ui/app-shell': patch
3+
---
4+
5+
Clause-②: no
6+
Metadata-protocol: none
7+
8+
Studio field inspector: when a field of a read-only base package opens in display-patch mode, every control it locks (API name, Type, Required and Unique, Default value, option values and colours, Track history, the rule editors, Move up/down, Remove and the rest) now explains itself. Each sits in a focusable wrapper described by the "you can change only …" sentence, which also shows as a tooltip on hover and keyboard focus. The editable display properties (Label, Description, Help text, Placeholder, option labels) carry no reason.
Lines changed: 211 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,211 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
/**
4+
* objectstack#28986 — in display-patch mode (a read-only base's own field,
5+
* edited through its pair) every control the inspector locks explains itself:
6+
* on hover, on keyboard focus and to assistive tech, as the read-only rail's
7+
* described wrapper does (objectui#12303). The whitelisted controls stay
8+
* editable and carry no reason.
9+
*
10+
* The oracle is the checklist row `ux-conventions.disabled-control-reason`:
11+
* a disabled control, or its focusable wrapper, is in the tab order and
12+
* carries an accessible description resolving to non-empty text.
13+
*/
14+
15+
import '@testing-library/jest-dom/vitest';
16+
import * as React from 'react';
17+
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
18+
import { cleanup, render, screen, within } from '@testing-library/react';
19+
import userEvent from '@testing-library/user-event';
20+
import { t, tFormat } from '../i18n';
21+
22+
type Row = Record<string, unknown>;
23+
24+
vi.mock('../useMetadata', async (importOriginal) => {
25+
const mod = await importOriginal<typeof import('../useMetadata')>();
26+
const client = { list: async () => [], listDrafts: async () => [] };
27+
return { ...mod, useMetadataClient: () => client };
28+
});
29+
30+
vi.mock('../previews/useObjectFields', async (importOriginal) => {
31+
const mod = await importOriginal<typeof import('../previews/useObjectFields')>();
32+
return { ...mod, useObjectFields: () => ({ fields: [], loading: false, error: null }) };
33+
});
34+
35+
import { ObjectFieldInspector } from './ObjectFieldInspector';
36+
import { BaseFieldDisplayPatchContext } from '../../studio-design/StudioCustomizeControls';
37+
38+
beforeEach(() => {
39+
// Radix reads pointer capture, which the DOM double does not implement.
40+
for (const m of ['hasPointerCapture', 'setPointerCapture', 'releasePointerCapture'] as const) {
41+
if (!(m in Element.prototype)) {
42+
(Element.prototype as unknown as Record<string, unknown>)[m] = m === 'hasPointerCapture' ? () => false : () => {};
43+
}
44+
}
45+
if (!('scrollIntoView' in Element.prototype)) {
46+
(Element.prototype as unknown as Record<string, unknown>).scrollIntoView = () => {};
47+
}
48+
});
49+
50+
afterEach(cleanup);
51+
52+
const LOCALE = 'en-US';
53+
54+
/** One field per family of locked control the inspector draws. */
55+
const OBJECT: Row = {
56+
name: 'crm_account',
57+
label: 'Account',
58+
fields: {
59+
name: { type: 'text', label: 'Account Name', required: true, placeholder: 'Acme' },
60+
industry: {
61+
type: 'select',
62+
label: 'Industry',
63+
options: [
64+
{ value: 'technology', label: 'Technology', color: '#22aa22' },
65+
{ value: 'retail', label: 'Retail' },
66+
],
67+
},
68+
parent: { type: 'lookup', label: 'Parent', reference: 'crm_account' },
69+
employees: { type: 'number', label: 'Employees' },
70+
score: { type: 'formula', label: 'Score', expression: 'record.employees * 2' },
71+
revenue: { type: 'currency', label: 'Revenue' },
72+
},
73+
};
74+
const FIELDS = Object.keys(OBJECT.fields as Row);
75+
76+
function mount(selected: string, { patchOnly = true, readOnly = false } = {}) {
77+
render(
78+
<BaseFieldDisplayPatchContext.Provider value={patchOnly}>
79+
<div data-testid="rail">
80+
<ObjectFieldInspector
81+
type="object"
82+
name={String(OBJECT.name)}
83+
draft={OBJECT}
84+
selection={{ kind: 'field', id: selected }}
85+
onPatch={() => {}}
86+
onClearSelection={() => {}}
87+
onSelectionChange={() => {}}
88+
readOnly={readOnly}
89+
locale={LOCALE}
90+
/>
91+
</div>
92+
</BaseFieldDisplayPatchContext.Provider>,
93+
);
94+
return screen.getByTestId('rail');
95+
}
96+
97+
const CONTROLS = 'input, button, textarea, select, [role="checkbox"], [role="combobox"], [role="switch"]';
98+
99+
/** Every control rendered disabled: the attribute, `aria-disabled`, or Radix's `data-disabled`. */
100+
function disabledControls(root: HTMLElement): HTMLElement[] {
101+
return Array.from(root.querySelectorAll<HTMLElement>(CONTROLS)).filter(
102+
(el) => el.matches(':disabled') || el.getAttribute('aria-disabled') === 'true' || el.hasAttribute('data-disabled'),
103+
);
104+
}
105+
106+
/** The text an element's `aria-describedby` resolves to, ids joined as assistive tech reads them. */
107+
function description(el: Element): string {
108+
const ids = (el.getAttribute('aria-describedby') ?? '').split(/\s+/).filter(Boolean);
109+
return ids
110+
.map((id) => document.getElementById(id)?.textContent ?? '')
111+
.join(' ')
112+
.trim();
113+
}
114+
115+
/** The whitelisted inputs: label, description, help text, placeholder, and each option's label. */
116+
function editableControls(root: HTMLElement): HTMLElement[] {
117+
const named = ['designer.field.description', 'designer.field.helpText', 'designer.field.placeholder'].map(
118+
(key) => within(root).getByLabelText(t(key, LOCALE)),
119+
);
120+
return [within(root).getByTestId('field-label-input'), ...named, ...within(root).queryAllByTestId('option-label')];
121+
}
122+
123+
describe('ObjectFieldInspector — display-patch mode explains every locked control (objectstack#28986)', () => {
124+
it.each(FIELDS)('field %s: each disabled control sits in a focusable wrapper described by the whitelist sentence', (field) => {
125+
const rail = mount(field);
126+
const sentence = within(rail).getByTestId('field-base-patch-only');
127+
expect(sentence.textContent?.trim()).not.toBe('');
128+
const locked = disabledControls(rail);
129+
expect(locked.length).toBeGreaterThan(0);
130+
for (const control of locked) {
131+
const wrapper = control.closest('[data-disabled-reason]');
132+
expect(wrapper, `${control.outerHTML.slice(0, 120)} has no described wrapper`).not.toBeNull();
133+
expect(wrapper).toHaveAttribute('tabindex', '0');
134+
expect(description(wrapper!)).toBe(sentence.textContent?.trim());
135+
}
136+
});
137+
138+
it.each(FIELDS)('field %s: no wrapper is a tab stop over nothing locked, nor over an editable control', (field) => {
139+
const rail = mount(field);
140+
const wrappers = within(rail).queryAllByTestId('field-base-patch-locked');
141+
expect(wrappers.length).toBeGreaterThan(0);
142+
for (const wrapper of wrappers) {
143+
expect(disabledControls(wrapper).length, wrapper.outerHTML.slice(0, 160)).toBeGreaterThan(0);
144+
}
145+
for (const control of editableControls(rail)) {
146+
expect(control).toBeEnabled();
147+
expect(control.closest('[data-disabled-reason]')).toBeNull();
148+
expect(control).not.toHaveAttribute('aria-describedby', expect.stringContaining(within(rail).getByTestId('field-base-patch-only').id));
149+
}
150+
});
151+
152+
it('the named locks of the card: API name, Type, Required, Track history, a rule editor, Move up/down and Remove', () => {
153+
const rail = mount('industry');
154+
const sentence = within(rail).getByTestId('field-base-patch-only');
155+
const named = [
156+
within(rail).getByTestId('field-apiname-input'),
157+
within(rail).getByRole('checkbox', { name: t('designer.field.required', LOCALE) }),
158+
within(rail).getByRole('checkbox', { name: t('designer.field.trackHistory', LOCALE) }),
159+
within(rail).getByLabelText(t('designer.field.visibleWhen', LOCALE)),
160+
// The header's pair and each option row's.
161+
...within(rail).getAllByRole('button', { name: t('engine.inspector.reorder.up', LOCALE) }),
162+
within(rail).getByRole('button', { name: tFormat('designer.field.remove', LOCALE, { label: 'Industry' }) }),
163+
within(rail).getAllByTestId('option-value')[0]!,
164+
];
165+
for (const control of named) {
166+
expect(control).toBeDisabled();
167+
const wrapper = control.closest('[data-disabled-reason]');
168+
expect(wrapper).not.toBeNull();
169+
expect(wrapper).toHaveAttribute('aria-describedby', sentence.id);
170+
}
171+
});
172+
173+
it('Tab reaches a lock, and focus or hover shows the reason as a tooltip', async () => {
174+
const rail = mount('name');
175+
const sentence = within(rail).getByTestId('field-base-patch-only').textContent!.trim();
176+
const apiNameLock = within(rail).getByTestId('field-apiname-input').closest<HTMLElement>('[data-disabled-reason]')!;
177+
let reached = false;
178+
for (let i = 0; i < 12 && !reached; i++) {
179+
await userEvent.tab();
180+
reached = document.activeElement === apiNameLock;
181+
}
182+
expect(reached).toBe(true);
183+
expect(await screen.findByRole('tooltip')).toHaveTextContent(sentence);
184+
185+
cleanup();
186+
const again = mount('name');
187+
const requiredLock = within(again)
188+
.getByRole('checkbox', { name: t('designer.field.required', LOCALE) })
189+
.closest<HTMLElement>('[data-disabled-reason]')!;
190+
await userEvent.hover(requiredLock);
191+
expect(await screen.findByRole('tooltip', {}, { timeout: 2000 })).toHaveTextContent(sentence);
192+
});
193+
});
194+
195+
describe('CONTROL: outside display-patch mode the inspector draws no lock of its own (objectstack#28986)', () => {
196+
it('a writable field: nothing disabled, nothing wrapped', () => {
197+
const rail = mount('name', { patchOnly: false });
198+
expect(within(rail).queryByTestId('field-base-patch-only')).toBeNull();
199+
expect(rail.querySelector('[data-disabled-reason]')).toBeNull();
200+
expect(within(rail).getByTestId('field-apiname-input')).toBeEnabled();
201+
});
202+
203+
it('a read-only rail: the host describes the whole rail, so no inner tab stops are added', () => {
204+
for (const patchOnly of [false, true]) {
205+
const rail = mount('industry', { patchOnly, readOnly: true });
206+
expect(disabledControls(rail).length).toBeGreaterThan(0);
207+
expect(rail.querySelector('[data-disabled-reason]')).toBeNull();
208+
cleanup();
209+
}
210+
});
211+
});

0 commit comments

Comments
 (0)