Skip to content

Commit fea2dbb

Browse files
fix(app-shell): read-only flow node inspector gives its reason on hover, focus and to assistive tech (#29073)
<!-- claude-projects-attribution: {"github_login":"zhuangjianguo"} --> _Requested by **Steedos** · [project thread](https://claude.ai/code/project/chan_018ETKaNwMZCNRYD7D8Q9Ubb?thread=cmsg_018ETKaNwMZCNRYD7D8Q9UbbGscW3SsJbBqFxAgdKikVVT)_ Fixes #28985 Clause-②: yes (widening) Metadata-protocol: none ## 维护者速读 - **改了什么**:Studio 流程画布上,只读包的节点属性栏现在会说明控件为什么是灰的:鼠标悬停任一灰色控件弹出原因提示,键盘 Tab 能停在属性栏上并读出原因,读屏把原因当作描述读出。做法与字段栏、校验规则编辑器一致。原因文字由宿主传入(新增可选属性 `readOnlyReason`),Studio 自动化页传「只读软件包」那句。 - **为什么改**:打开已安装包(如 showcase)的流程,点中节点后整栏灰掉,悬停、Tab、读屏都得不到原因;平台测试清单 `ux-conventions.disabled-control-reason` 因此判失败。 - **风险与回滚**:低。只在「只读且宿主给了原因」时多包一层可聚焦的分组;不传原因的宿主(如元数据管理页)渲染与之前完全一样。可写时不变。回滚即还原本 PR。 - **先看**:`FlowNodeInspector.tsx` 末尾新增的只读原因分组;`inspector-registry.ts` 的 `readOnlyReason`;`StudioDesignSurface.tsx` 流程宿主调用处新增的一行。 - **两分钟验证**:在 `ui/` 下运行 `pnpm exec vitest run packages/app-shell/src/views/metadata-admin/inspectors/FlowNodeInspector.readOnlyReason-28985.test.tsx packages/app-shell/src/views/studio-design/StudioDesignSurface.automationsReadOnly-11124.test.tsx`,预期 2 个文件、16 个测试全绿。 - **需要你做的**:无,席位落地。 ## What changes - **`MetadataInspectorProps.readOnlyReason?: string`** (`inspector-registry.ts`, exported from the `@object-ui/app-shell` entry). The inspector is shared by hosts whose read-only causes differ: Studio's Automations pillar is read-only because the package is, while the metadata editor page is also read-only in view mode or on a walled item. So the host says why, and an inspector that honours the prop draws it while `readOnly` is true. Optional, so every existing host and inspector compiles and renders unchanged. - **`FlowNodeInspector`** wraps its panel, when `readOnly` and the reason is non-blank, in the treatment Studio's field rail and validation-rule editor carry: a `role="group"` named by the node, `tabIndex={0}`, `aria-describedby` an `sr-only` copy of the reason, and the trigger of a tooltip that shows it. One trigger for the panel, not one per control, because the reason is the same for all of them. The wrapper is inline, as in those two surfaces: no exported wrapper exists to reuse (the two are inline JSX and `DisabledReasonTrigger` is private to `@object-ui/components`). - **Studio's Automations pillar** passes `engine.studio.pkg.readonlyHint` as `readOnlyReason` when `editLocked`, one prop at the flow inspector's call site in `StudioDesignSurface.tsx`. ## Tests - **New: `FlowNodeInspector.readOnlyReason-28985.test.tsx`, 6 cases** on an `update_record` node: - read-only with a reason: the panel is a tab stop, named by the node, described by the reason, and holds every disabled control (ID, Label, Node Type, the config fields, Remove node; at least four, so a vacuous pass is not one); - hovering the disabled Remove node opens a tooltip with the reason; - Tab lands on the described panel and opens the tooltip; - controls: read-only with no reason renders no tab stop or description; a blank reason counts as none; a writable inspector ignores a reason and its controls are live. - **Changed: `StudioDesignSurface.automationsReadOnly-11124.test.tsx`.** The read-only case now asserts the opened node inspector sits inside the described group with the package's reason, through the real pillar, `FlowPreview` and `FlowInspector`; the writable control asserts no group. - **Red on main.** With `FlowNodeInspector.tsx` and `StudioDesignSurface.tsx` restored from `ab085a04`, the two files ran 4 failed and 12 passed: the three new reason cases and the host case fail, every control passes. With the fix, 16 of 16 pass. ## Acceptance notes - The tooltip also opens when hovering the header's close button in the stand-alone shell (outside Studio), since the whole panel is the trigger; the field rail behaves the same way. - The metadata editor page (`ResourceEditPage`) passes no reason yet, so its read-only node inspector is unchanged; its causes (no write access, a walled item, view mode) each need their own sentence, which is out of this card's scope and reported to the seat. - `FlowEdgeInspector` (a selected connection) disables its controls on `readOnly` the same way and is not covered by this card; reported to the seat. It now receives the reason through the same props and ignores it. ## Verification All readings on head `447e0f65` unless named (fix `608e38ed`, node-name follow-up `447e0f65`), base `ab085a04`. <details><summary>Tests</summary> - `pnpm exec vitest run` (from `ui/`) on the two pinned files plus `flowNodeLabels.sites-13200.test.ts`: 3 files, 51 tests passed. - Red on main: the two pinned files with `FlowNodeInspector.tsx` and `StudioDesignSurface.tsx` restored from `ab085a04`: 4 failed, 12 passed (the three reason cases and the host case). Restored with `git checkout HEAD`, `git diff HEAD` empty. - Sweep of the 116 ui test files that `git grep` finds naming `FlowNodeInspector`, `FlowInspector`, `AutomationsPillar` or `MetadataInspectorProps`, in three locked batches at `--maxWorkers=2`: 38/38 files (415 tests), 38/38 (474), 40/40 (330). The first run of the middle batch caught `flowNodeLabels.sites-13200.test.ts` (an id-shaped node name in the group's `aria-label`); `447e0f65` names it through `flowNodeDisplayName`, and that batch re-ran green on the final head. Narrowed on purpose: CI's Test UI runs the rest of app-shell. - `pnpm exec turbo run type-check --filter=@object-ui/app-shell`: 65 of 65 tasks successful. - `eslint` on the four changed ui files other than `StudioDesignSurface.tsx`: 0 errors (3 existing warnings in `FlowNodeInspector.tsx`, untouched lines). </details> <details><summary>Gates</summary> - `node scripts/pm/dispatch-gates.mjs --commands` derived 34 families; all 34 ran with exit 0, the 33 unlocked ones with this body as the PR event (`check-adr-0087-registration`, `check-changeset-no-major`, `check-closing-keyword-parity`, `check-sdui-manifest` among them), and `pnpm --dir ui exec vitest run scripts/` locked: 169 files passed, 2 skipped (4,900 tests). - `dispatch-gates --ran`: 34 derived, 34 run, 0 NOT-MEASURED, 0 UNRUN. - `check:new-line-citations`: 0 new citations. </details> --- _Generated by [Claude Code](https://claude.ai/code/session_01QsR2jTpDoNt6xHzNNCFygz)_ --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 7f4258f commit fea2dbb

6 files changed

Lines changed: 219 additions & 3 deletions

File tree

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
---
2+
'@object-ui/app-shell': patch
3+
---
4+
5+
fix(app-shell): a read-only flow node inspector says why its controls are disabled, on hover, on keyboard focus and to assistive tech
6+
7+
Clause-②: yes (widening)
8+
Metadata-protocol: none
9+
10+
On a read-only package, Studio's flow node inspector disabled every control (ID, Label, Node Type, the config fields, Remove node) and gave no reason anywhere: no tooltip, no accessible description, and Tab skipped the whole panel. The inspector body now carries the treatment Studio's field rail and validation-rule editor carry: it is a tab stop described by the reason, and hovering any disabled control in it shows the reason in a tooltip.
11+
12+
The reason is the host's to give, because the inspector is shared by hosts whose read-only causes differ. `MetadataInspectorProps` gains an optional `readOnlyReason?: string`; an inspector that honours it draws the reason while `readOnly` is true. Studio's Automations pillar passes the read-only package sentence. A host that passes nothing renders exactly as before.

‎ui/packages/app-shell/src/views/metadata-admin/inspector-registry.ts‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,15 @@ export interface MetadataInspectorProps {
6666
onBlockingIssuesChange?: (count: number) => void;
6767
/** Whether the host is in edit mode. False → disable inputs. */
6868
readOnly: boolean;
69+
/**
70+
* Why the inspector is read-only, as the sentence to show, when the host
71+
* knows it. The cause is the host's (a read-only package, view mode, a
72+
* walled item), so the host says it; an inspector that honours it explains
73+
* its disabled controls on hover, on keyboard focus and as an accessible
74+
* description. Omitted, the inspector draws no reason. Ignored while
75+
* `readOnly` is false.
76+
*/
77+
readOnlyReason?: string;
6978
/** Active UI locale for i18n. */
7079
locale: SupportedLocale;
7180
}
Lines changed: 146 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,146 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
/**
4+
* objectstack#28985 — a read-only flow node inspector says why its controls are
5+
* disabled, on hover, on keyboard focus and to assistive tech, when its host
6+
* hands it the reason (`readOnlyReason`).
7+
*
8+
* Measured on the card: on a read-only package every control of the node
9+
* inspector (ID, Label, Node Type, the config fields, Remove node) was disabled
10+
* with no `title`, no `aria-describedby` and no tab stop, so nothing said why.
11+
* The inspector body now carries the treatment Studio's field rail and
12+
* validation-rule editor carry: a tab stop described by an `sr-only` copy of
13+
* the reason, and the trigger of a tooltip that shows it.
14+
*
15+
* The reason is the host's: the inspector is shared by hosts whose read-only
16+
* causes differ, so a host that passes none gets the panel it always had.
17+
* Studio's Automations pillar passing the package's reason is pinned in
18+
* `StudioDesignSurface.automationsReadOnly-11124.test.tsx`.
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, cleanup } from '@testing-library/react';
25+
import userEvent from '@testing-library/user-event';
26+
27+
vi.mock('../previews/useFlowNodePalette', () => ({
28+
useActionConfigSchemas: () => ({}),
29+
useFlowNodePalette: () => [],
30+
}));
31+
vi.mock('../previews/useObjectFields', () => ({
32+
useObjectFields: () => ({ fields: [], loading: false, error: null }),
33+
}));
34+
35+
import { FlowNodeInspector } from './FlowNodeInspector';
36+
import { t } from '../i18n';
37+
38+
afterEach(cleanup);
39+
40+
vi.stubGlobal(
41+
'fetch',
42+
vi.fn(async () => new Response('not found', { status: 404 })),
43+
);
44+
45+
const REASON = 'Read-only package: switch to or create a writable package to edit.';
46+
47+
const DRAFT = {
48+
name: 'f',
49+
label: 'F',
50+
nodes: [
51+
{ id: 'start', type: 'start', label: 'Start' },
52+
{
53+
id: 'set_status',
54+
type: 'update_record',
55+
label: 'Set status',
56+
config: { objectName: 'task', fields: { status: 'done' } },
57+
},
58+
{ id: 'end', type: 'end', label: 'End' },
59+
],
60+
edges: [
61+
{ id: 'e1', source: 'start', target: 'set_status' },
62+
{ id: 'e2', source: 'set_status', target: 'end' },
63+
],
64+
};
65+
66+
function mount({ readOnly, readOnlyReason }: { readOnly: boolean; readOnlyReason?: string }) {
67+
return render(
68+
<FlowNodeInspector
69+
type="flow"
70+
name="f"
71+
draft={DRAFT}
72+
selection={{ kind: 'node', id: 'set_status', label: 'Set status' }}
73+
onPatch={() => {}}
74+
onClearSelection={() => {}}
75+
readOnly={readOnly}
76+
readOnlyReason={readOnlyReason}
77+
locale="en-US"
78+
/>,
79+
);
80+
}
81+
82+
/** The panel's form controls, Remove node included — the ones the reason is about. */
83+
const controls = (root: HTMLElement) => [
84+
...root.querySelectorAll<HTMLElement>('input, select, textarea, button[role="combobox"]'),
85+
screen.getByRole('button', { name: t('engine.inspector.flowNode.remove', 'en-US') }),
86+
];
87+
88+
describe('Flow node inspector, read-only with a reason from its host (objectstack#28985)', () => {
89+
it('the panel is a tab stop described by the reason, named by the node, and holds the disabled controls', () => {
90+
const { container } = mount({ readOnly: true, readOnlyReason: REASON });
91+
const body = container.querySelector('[data-disabled-reason]') as HTMLElement;
92+
expect(body).not.toBeNull();
93+
expect(body).toHaveAttribute('tabindex', '0');
94+
expect(body).toHaveAccessibleDescription(REASON);
95+
expect(body).toHaveAccessibleName('Set status');
96+
97+
const all = controls(body);
98+
// ID, Label, Node Type and Remove node alone are four; a vacuous pass is not one.
99+
expect(all.length).toBeGreaterThanOrEqual(4);
100+
for (const el of all) {
101+
expect(body).toContainElement(el);
102+
expect(el).toBeDisabled();
103+
}
104+
});
105+
106+
it('hovering the disabled "Remove node" opens a tooltip with the reason', async () => {
107+
const user = userEvent.setup();
108+
mount({ readOnly: true, readOnlyReason: REASON });
109+
const remove = screen.getByRole('button', { name: t('engine.inspector.flowNode.remove', 'en-US') });
110+
expect(remove).toBeDisabled();
111+
await user.hover(remove);
112+
expect(await screen.findByRole('tooltip')).toHaveTextContent(REASON);
113+
});
114+
115+
it('a keyboard user reaches the reason: Tab lands on the described panel, which opens the tooltip', async () => {
116+
const user = userEvent.setup();
117+
const { container } = mount({ readOnly: true, readOnlyReason: REASON });
118+
const body = container.querySelector('[data-disabled-reason]') as HTMLElement;
119+
await user.tab();
120+
expect(body).toHaveFocus();
121+
expect(await screen.findByRole('tooltip')).toHaveTextContent(REASON);
122+
});
123+
});
124+
125+
describe('Flow node inspector — the controls (objectstack#28985)', () => {
126+
it('a host that passes no reason gets the panel it always had: no tab stop, no description', () => {
127+
const { container } = mount({ readOnly: true });
128+
expect(container.querySelector('[data-disabled-reason]')).toBeNull();
129+
expect(container.querySelector('[tabindex="0"]')).toBeNull();
130+
for (const el of controls(container)) expect(el).toBeDisabled();
131+
});
132+
133+
it('a blank reason counts as none', () => {
134+
const { container } = mount({ readOnly: true, readOnlyReason: ' ' });
135+
expect(container.querySelector('[data-disabled-reason]')).toBeNull();
136+
});
137+
138+
it('a writable inspector ignores a reason: the controls are live and nothing is described', () => {
139+
const { container } = mount({ readOnly: false, readOnlyReason: REASON });
140+
expect(container.querySelector('[data-disabled-reason]')).toBeNull();
141+
expect(screen.queryByText(REASON)).toBeNull();
142+
const all = controls(container);
143+
expect(all.length).toBeGreaterThanOrEqual(4);
144+
for (const el of all) expect(el).toBeEnabled();
145+
});
146+
});

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

Lines changed: 44 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@
2222
import * as React from 'react';
2323
import { Plus } from 'lucide-react';
2424
import { resolveFlowTriggerKind } from '@objectstack/spec/automation';
25-
import { Input, Label } from '@object-ui/components';
25+
import { Input, Label, Tooltip, TooltipContent, TooltipProvider, TooltipTrigger } from '@object-ui/components';
2626
import type { MetadataInspectorProps } from '../inspector-registry.js';
2727
import { t, tFormat } from '../i18n.js';
2828
import {
@@ -284,7 +284,17 @@ function FlowNodeIdField({
284284
);
285285
}
286286

287-
export function FlowNodeInspector({ selection, draft, onPatch, onClearSelection, onSelectionChange, locale, readOnly }: MetadataInspectorProps) {
287+
export function FlowNodeInspector({
288+
selection,
289+
draft,
290+
onPatch,
291+
onClearSelection,
292+
onSelectionChange,
293+
locale,
294+
readOnly,
295+
readOnlyReason,
296+
}: MetadataInspectorProps) {
297+
const readOnlyReasonId = React.useId();
288298
// Resolve the selection to a node + how to write it back — a top-level draft
289299
// node, or a node nested inside a container region (#2670). Every edit goes
290300
// through loc.write, so the inspector never branches on where the node lives.
@@ -694,7 +704,7 @@ export function FlowNodeInspector({ selection, draft, onPatch, onClearSelection,
694704
// read-only — those live on the container's Advanced JSON).
695705
const nested = !!loc?.nested;
696706

697-
return (
707+
const shell = (
698708
<InspectorShell
699709
kindLabel={t('engine.inspector.flowNode.kind', locale)}
700710
title={node.label || node.id}
@@ -884,4 +894,35 @@ export function FlowNodeInspector({ selection, draft, onPatch, onClearSelection,
884894
)}
885895
</InspectorShell>
886896
);
897+
898+
const reason = readOnly ? readOnlyReason?.trim() : undefined;
899+
if (!reason) return shell;
900+
// Read-only with a reason from the host: every control above is disabled for
901+
// that one reason, and a disabled control takes neither hover nor focus. The
902+
// inspector body is the tooltip trigger, so hovering any of them shows it, and
903+
// a tab stop described by an `sr-only` copy, so a keyboard or screen-reader
904+
// user reaches it too. One trigger for the panel, not one per control: the
905+
// reason is the same for all of them. Same treatment as Studio's field rail
906+
// and validation-rule editor.
907+
return (
908+
<TooltipProvider delayDuration={200}>
909+
<Tooltip>
910+
<TooltipTrigger asChild>
911+
<div
912+
role="group"
913+
aria-label={flowNodeDisplayName(node, locale)}
914+
tabIndex={0}
915+
aria-describedby={readOnlyReasonId}
916+
data-disabled-reason=""
917+
data-testid="flow-node-readonly-reason"
918+
className="flex h-full min-h-0 flex-col focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-inset focus-visible:ring-ring"
919+
>
920+
{shell}
921+
<span id={readOnlyReasonId} className="sr-only">{reason}</span>
922+
</div>
923+
</TooltipTrigger>
924+
<TooltipContent side="left" align="start">{reason}</TooltipContent>
925+
</Tooltip>
926+
</TooltipProvider>
927+
);
887928
}

‎ui/packages/app-shell/src/views/studio-design/StudioDesignSurface.automationsReadOnly-11124.test.tsx‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -213,6 +213,12 @@ describe('Automations pillar on a read-only package (objectui#11124)', () => {
213213
const label = await inspectStartNode();
214214
expect(label).toBeDisabled();
215215
expect(within(screen.getByRole('complementary')).getByLabelText('ID')).toBeDisabled();
216+
// objectstack#28985 — the pillar hands the inspector the package's reason,
217+
// and the inspector body is the tab stop described by it.
218+
const body = screen.getByTestId('flow-node-readonly-reason');
219+
expect(body).toContainElement(label);
220+
expect(body).toHaveAttribute('tabindex', '0');
221+
expect(body).toHaveAccessibleDescription(READ_ONLY_REASON);
216222
});
217223

218224
it('offers no canvas edit: no "add connected node" or "insert node" handle', async () => {
@@ -256,6 +262,7 @@ describe('Automations pillar on a writable package — the control (objectui#111
256262

257263
const label = await inspectStartNode();
258264
expect(label).toBeEnabled();
265+
expect(screen.queryByTestId('flow-node-readonly-reason')).toBeNull();
259266

260267
fireEvent.click(toggle);
261268
await waitFor(() => expect(server.saves).toHaveLength(1), { timeout: 8000 });

‎ui/packages/app-shell/src/views/studio-design/StudioDesignSurface.tsx‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10721,6 +10721,7 @@ export function AutomationsPillar({
1072110721
// the Data pillar threads it (objectui#2259). objectstack#28570 —
1072210722
// a customized flow's editors are open.
1072310723
readOnly: editLocked,
10724+
readOnlyReason: editLocked ? t('engine.studio.pkg.readonlyHint', locale) : undefined,
1072410725
locale,
1072510726
})}
1072610727
</div>

0 commit comments

Comments
 (0)