Skip to content

Commit e8b10a6

Browse files
fix(app-shell): Studio's flow definition switch keeps keyboard focus across a toggle (#29449)
<!-- claude-projects-attribution: {"github_login":"zhuangjianguo"} --> _Requested by **Steedos** · [project thread](https://claude.ai/code/project/chan_018ETKaNwMZCNRYD7D8Q9Ubb?thread=cmsg_018ETKaNwMZCNRYD7D8Q9Ubb7bgo9GLavj5QghibLqpzHe)_ Fixes #29376 Clause-②: no Metadata-protocol: none The two lines are copied from the claim (comment 6579229963). ## 维护者速读 - 改了什么:Studio › Automations 里可写包流程的「启用/停用」开关(流程定义开关,`StudioDesignSurface.tsx`),保存进行中不再是 `disabled`,改为 `aria-disabled`,并在点击处理里拒绝再次按下;只读包、没有画布的情况仍是 `disabled`,说明文字不变。 - 为什么改:浏览器会把焦点从一个变成 `disabled` 的已聚焦控件挪到页面 body,保存结束后也不会还回来。用键盘或读屏的作者每拨一次开关(保存成功或被拒都一样),都得从页面顶部重新 Tab 回来。这与 #29228(PR #29375)修过的部署开关是同一个缺陷,本 PR 用同样的做法。 - 风险与回滚:只改一个开关的 `onClick` / `disabled` / `aria-disabled` 三处属性和一个样式类;请求中视觉上仍是半透明(`aria-disabled:opacity-50`)。保存中按第二次空格不会再发保存,由点击守卫保证(测试与消融都钉住了)。回滚即撤销本 PR。 - 先看:`StudioDesignSurface.tsx` 里定义开关的 `inert` / `busy` 两行和 `onClick`;新测试文件 `StudioDesignSurface.definitionSwitchFocus-29376.test.tsx`。 - 两分钟验证:`pnpm --dir ui exec vitest run packages/app-shell/src/views/studio-design/StudioDesignSurface.definitionSwitchFocus-29376.test.tsx`,预期 3 个用例全部通过。 - 需要你做的:无,席位落地。 ## What changes - `ui/packages/app-shell/src/views/studio-design/StudioDesignSurface.tsx`, the flow definition switch only: while a save is in flight (`saving`), the switch carries `aria-disabled="true"` instead of `disabled`, and its `onClick` refuses the press while `saving`. `disabled` stays for the two cases where the switch cannot be flipped at all (no canvas to edit, or a read-only package), so a read-only switch is unchanged: disabled, out of the tab order, described by the package's reason through its focusable wrapper. The in-flight dimming adds `aria-disabled:opacity-50` beside `disabled:opacity-50`, so the switch looks as before. - A comment at the switch says why the busy state is `aria-disabled`. - `toggleEnabled` is not touched; the guard sits in the switch's handler, as in PR #29375. ## Why Measured live by the #29228 os-dev in Chromium 153 on a fresh showcase (a scratch writable package with one flow): with the definition switch focused, Space saved the draft (`PUT 200`) and `document.activeElement` was the page body afterwards. The switch drew `disabled={!isEditable || !!saving || readOnly}`, and `toggleEnabled` sets `saving = 'draft'`. A focused control that turns `disabled` loses focus and nothing returns it. Same shape as #29228; this applies PR #29375's approach (focus never leaves, so there is nothing to restore and no host plumbing). ## Tests New file `StudioDesignSurface.definitionSwitchFocus-29376.test.tsx`, on the harness of `StudioDesignSurface.automationsReadOnly-11124` (the real `FlowPreview` and `FlowInspector`, a client double whose draft save is held on a gate and can be refused with `ITEM_LOCKED`): - `it.each` over a saved flip and a refused one: focus the switch, press Space with `@testing-library/user-event`. In flight, `document.activeElement` is the switch, it is `aria-disabled="true"` and still enabled; a second Space leaves the switch on the first flip (`aria-checked="false"`) and the save count at 1. After the answer (off when saved; the refusal shown and back on when refused), the switch is the same element, `document.activeElement` is still the switch, and the save count is still 1. - Read-only package: the switch is `disabled`, carries no `aria-disabled`, is described by the read-only reason, and cannot take focus. - **Why the test runs a focus fixup.** happy-dom keeps focus on a control that turns `disabled`; a browser does not. `runFocusFixup` (copied from PR #29375's test) moves focus to the body when the focused switch turns disabled, so the focus pins can fail. - **Why the second press is pinned by state, not only by count.** Draft saves are queued one at a time (`useDraftSaveGuard`), so a second save waits behind the held one and the server count alone cannot see it in flight. The first draft of this test checked only the count and the guard ablation went red later than intended (at the post-answer `aria-checked`); the test now pins the switch's state right after the second Space. - Ablation 1, the card's: `disabled={inert}` to `disabled={inert || !!saving}` through `scripts/ablation-replace.mjs` (anchor x1 to x0, blob ef4c436 to 987c0f3b): both flip cases red at `expect(document.activeElement).toBe(sw)` in flight (line 190, received the body), read-only case green. Restored: blob equals HEAD, `git diff HEAD` empty. - Ablation 2, the guard: `if (!saving) void toggleEnabled();` to `if (true) void toggleEnabled();` (blob ef4c436 to 4e50f0fd): both flip cases red at the in-flight `aria-checked` pin (line 198). Restored, same proof. ## Acceptance notes - A live re-run of the #29228 probe on a fresh showcase was not done (the card calls it welcome, not owed); the evidence is the happy-dom pins with the browser focus fixup, and the two ablations. - The guard reads `saving`, which every save in this pillar sets (the autosave and publish too), so the switch also refuses a press while an autosave is in flight; before, it was `disabled` then, so nothing new is refused. ## Verification <details><summary>Suites, type-check, lint</summary> - At 9128595 (after the first merge of main) and again at 39356c4 (after the second): the new file plus the six other files that draw this switch or the ledger switch (`automationsReadOnly-11124`, `readOnlyFlowSwitch-27356`, `autosaveSwitch-11232`, `packageLessFlows-11553`, `AutomationsPillar.customizeFlow-28570`, `AutomationsPillar.ledgerSwitch-28148`): 7 files, 70 tests passed. - `turbo run type-check --filter=@object-ui/app-shell` (`tsc --noEmit` and `tsc -p tsconfig.test.json`): green at a73ccee (the merge brings nothing into this package's files). - eslint from `ui/` on the two touched source files: 0 errors; the 46 warnings in `StudioDesignSurface.tsx` are pre-existing, none in the changed hunk. The repo-wide run is CI's Lint UI. </details> <details><summary>Gates</summary> `node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack` derived 34 commands; the list is the same before and after the merge of main. All 34 ran, each exit 0, including `node scripts/check-sdui-manifest.mjs` and `pnpm --dir ui exec vitest run scripts/` (169 files passed, 2 skipped; 4921 tests). After the merge of main the 31 faster commands ran again at 9128595; `check-comment-mask-corpus`, `check:query-options-erasure` and the ui `scripts/` suite were not re-run (the merge brings nothing into this diff's files). The second merge of main (39356c4) touches none of this diff's files either; only the seven suites above were re-run there. `--ran` reconciliation at 9128595: 34 derived, 34 run, 0 NOT-MEASURED. </details> ## 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. (Measured on the touched files only; see Verification.) - [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. (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. - [x] The *Before Submitting* list in CONTRIBUTING.md is done. 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01QsR2jSQ9vnVPEcFMUbd1EJ --- _Generated by [Claude Code](https://claude.ai/code/session_01QsR2jSQ9vnVPEcFMUbd1EJ)_ --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 0620fe3 commit e8b10a6

3 files changed

Lines changed: 251 additions & 3 deletions

File tree

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
---
2+
'@object-ui/app-shell': patch
3+
---
4+
5+
fix(app-shell): Studio's flow definition switch keeps keyboard focus while its save is in flight and after the answer lands
6+
7+
Clause-②: no
8+
Metadata-protocol: none
9+
10+
On Studio › Automations, a writable package's flow carries a switch that turns the flow on or off in its draft. While that save was in flight the switch drew `disabled`, and a browser moves focus off a focused control that turns disabled, to the page body, where it stayed once the save landed. A keyboard or screen reader author had to tab back from the top of the page after every flip, saved or refused. In flight the switch is now `aria-disabled` and refuses the press in its own handler, so it keeps focus and a second Space still sends no second save. A read-only package's switch, and one with no canvas to edit, stay `disabled` and described as before.
Lines changed: 226 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,226 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
/**
4+
* objectstack#29376 — a writable package's flow definition switch keeps
5+
* keyboard focus across a flip.
6+
*
7+
* Measured live in Chromium 153 on a fresh showcase: a scratch writable
8+
* package with one flow, its definition switch focused, Space; the draft save
9+
* answered 200 and `document.activeElement` was the page body afterwards. The
10+
* switch drew `disabled` while the save was in flight, and a browser moves
11+
* focus off a focused control that turns disabled and never hands it back.
12+
* In flight it is now `aria-disabled` and refuses the press in its handler.
13+
*
14+
* Pinned here:
15+
* - a keyboard flip, saved or refused, leaves focus on the switch while the
16+
* save is in flight and after it lands, and a second Space in flight sends
17+
* no second save;
18+
* - a read-only package's switch is unchanged: `disabled`, no
19+
* `aria-disabled`, described by the package's reason, not focusable.
20+
*
21+
* The harness is `StudioDesignSurface.automationsReadOnly-11124`'s: the REAL
22+
* registered `FlowPreview` and `FlowInspector`, and a client double whose
23+
* draft save can be held on a gate and refused.
24+
*/
25+
26+
import '@testing-library/jest-dom/vitest';
27+
import * as React from 'react';
28+
import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest';
29+
import { render, screen, cleanup, waitFor } from '@testing-library/react';
30+
import userEvent from '@testing-library/user-event';
31+
import { MemoryRouter } from 'react-router-dom';
32+
33+
const PKG = 'com.acme.app';
34+
35+
const NODES = [
36+
{ id: 'start', type: 'start', label: 'Start' },
37+
{ id: 'end', type: 'end', label: 'End' },
38+
];
39+
const EDGES = [{ id: 'e1', source: 'start', target: 'end' }];
40+
const FLOW = { name: 'notify_owner', label: 'Notify owner', type: 'autolaunched', status: 'active', nodes: NODES, edges: EDGES };
41+
42+
const LOCKED_MESSAGE = 'flow/notify_owner belongs to a read-only package and cannot be changed.';
43+
44+
const server = vi.hoisted(() => ({
45+
active: new Map<string, Record<string, unknown>>(),
46+
saves: [] as Array<{ type: string; name: string; body: Record<string, unknown> }>,
47+
/** When set, a draft save waits on this gate before it answers. */
48+
gate: null as null | Promise<void>,
49+
/** When true, a draft save is refused the way a locked package refuses it. */
50+
refuse: false,
51+
}));
52+
53+
const mockClient = vi.hoisted(() => {
54+
const k = (type: string, name: string) => `${type}/${name}`;
55+
return {
56+
list: vi.fn(async (type: string) =>
57+
[...server.active.entries()]
58+
.filter(([key]) => key.startsWith(`${type}/`))
59+
.map(([, row]) => ({ name: row.name, label: row.label ?? row.name })),
60+
),
61+
listDrafts: vi.fn(async () => []),
62+
listTypes: vi.fn(async () => ({ entries: [] })),
63+
get: vi.fn(async () => null),
64+
references: vi.fn(async () => []),
65+
layered: vi.fn(async (type: string, name: string) => {
66+
const eff = server.active.get(k(type, name)) ?? null;
67+
return { code: null, overlay: eff, overlayScope: eff ? 'env' : null, effective: eff, editable: true, deletable: true, resettable: false, lock: 'none' };
68+
}),
69+
getDraft: vi.fn(async (type: string, name: string) => {
70+
throw Object.assign(new Error(`No pending draft exists for ${type}/${name}.`), { code: 'NO_DRAFT', status: 404 });
71+
}),
72+
save: vi.fn(async (type: string, name: string, item: unknown) => {
73+
server.saves.push({ type, name, body: JSON.parse(JSON.stringify(item)) as Record<string, unknown> });
74+
if (server.gate) await server.gate;
75+
if (server.refuse) throw Object.assign(new Error(LOCKED_MESSAGE), { code: 'ITEM_LOCKED', status: 403 });
76+
return { type, name, item };
77+
}),
78+
publish: vi.fn(async () => ({ success: true })),
79+
reset: vi.fn(async () => ({})),
80+
};
81+
});
82+
83+
vi.mock('../metadata-admin/useMetadata', async (importOriginal) => {
84+
const mod = await importOriginal<typeof import('../metadata-admin/useMetadata')>();
85+
return { ...mod, useMetadataClient: () => mockClient, useMetadataTypes: () => ({ entries: [] }) };
86+
});
87+
88+
vi.mock('./packages-io', async (importOriginal) => {
89+
const mod = await importOriginal<typeof import('./packages-io')>();
90+
return { ...mod, fetchPackages: vi.fn(async () => []) };
91+
});
92+
93+
vi.mock('@object-ui/react', async (importOriginal) => {
94+
const mod = await importOriginal<typeof import('@object-ui/react')>();
95+
return { ...mod, useAdapter: () => dataSource };
96+
});
97+
98+
vi.mock('sonner', () => ({ toast: { success: vi.fn(), error: vi.fn() } }));
99+
100+
import { AutomationsPillar } from './StudioDesignSurface';
101+
import { createEmptyDataSource, failOnAbsorbedFetchError } from './__tests__/emptyDataSource';
102+
import { registerMetadataPreview } from '../metadata-admin/preview-registry';
103+
import { registerMetadataInspector } from '../metadata-admin/inspector-registry';
104+
import { FlowPreview } from '../metadata-admin/previews/FlowPreview';
105+
import { FlowInspector } from '../metadata-admin/inspectors/FlowInspector';
106+
import { t } from '../metadata-admin/i18n';
107+
108+
const dataSource = createEmptyDataSource();
109+
failOnAbsorbedFetchError();
110+
111+
// The pillar's `/automation/_status` probe and the inspector's action-catalog
112+
// read both go through the global `fetch`; one module-scope double answers
113+
// "absent" so each keeps its documented fallback.
114+
vi.stubGlobal(
115+
'fetch',
116+
vi.fn(async () => new Response('null', { status: 404, headers: { 'content-type': 'application/json' } })),
117+
);
118+
119+
registerMetadataPreview('flow', FlowPreview);
120+
registerMetadataInspector('flow', FlowInspector);
121+
122+
beforeEach(() => {
123+
server.active.clear();
124+
server.saves.length = 0;
125+
server.gate = null;
126+
server.refuse = false;
127+
for (const fn of Object.values(mockClient)) (fn as unknown as { mockClear: () => void }).mockClear();
128+
server.active.set(`flow/${FLOW.name}`, JSON.parse(JSON.stringify(FLOW)));
129+
});
130+
131+
afterEach(cleanup);
132+
133+
function renderPillar(readOnly: boolean) {
134+
return render(
135+
<MemoryRouter initialEntries={[`/studio/${PKG}/automations`]}>
136+
<AutomationsPillar packageId={PKG} readOnly={readOnly} />
137+
</MemoryRouter>,
138+
);
139+
}
140+
141+
/** The canvas's own status pill, which reads the same `draft.status` as the switch. */
142+
function canvasStatus(): string {
143+
return screen.getByText('Status:').nextElementSibling?.textContent ?? '';
144+
}
145+
const STATUS_ON = t('engine.studio.auto.enabled', 'en');
146+
147+
async function openFlow(): Promise<HTMLElement> {
148+
await waitFor(() => expect(canvasStatus()).toBe(STATUS_ON), { timeout: 8000 });
149+
return screen.getByRole('switch');
150+
}
151+
152+
const READ_ONLY_REASON = 'Read-only package — switch to or create a writable package to edit.';
153+
154+
// A browser runs the HTML focus fixup: a focused control that turns `disabled`
155+
// drops focus to the body (measured in Chromium 153, and not handed back when
156+
// it is enabled again). happy-dom keeps focus on it, so the test runs the
157+
// fixup on the switch the way the browser does, else the focus pins could not
158+
// fail. (happy-dom's `blur()` does nothing on a disabled control, so focus is
159+
// moved to the body itself.)
160+
function runFocusFixup(el: HTMLElement): () => void {
161+
const observer = new MutationObserver(() => {
162+
if (el.hasAttribute('disabled') && document.activeElement === el) document.body.focus();
163+
});
164+
observer.observe(el, { attributes: true, attributeFilter: ['disabled'] });
165+
return () => observer.disconnect();
166+
}
167+
168+
describe('Automations — the flow definition switch keeps keyboard focus across a flip (objectstack#29376)', () => {
169+
it.each([
170+
['saved', false],
171+
['refused', true],
172+
])('a keyboard flip that is %s leaves focus on the switch, and a second Space in flight sends nothing', async (_answer, refuse) => {
173+
server.refuse = refuse;
174+
let release!: () => void;
175+
server.gate = new Promise<void>((resolve) => {
176+
release = resolve;
177+
});
178+
const user = userEvent.setup();
179+
renderPillar(false);
180+
const sw = await openFlow();
181+
const stopFixup = runFocusFixup(sw);
182+
sw.focus();
183+
expect(document.activeElement).toBe(sw);
184+
185+
await user.keyboard(' ');
186+
await waitFor(() => expect(server.saves).toHaveLength(1), { timeout: 8000 });
187+
expect(server.saves[0]!.body.status).toBe('obsolete');
188+
await waitFor(() => expect(sw).toHaveAttribute('aria-disabled', 'true'));
189+
// In flight: still focused, refused to a press, still focusable.
190+
expect(document.activeElement).toBe(sw);
191+
expect(sw).toBeEnabled();
192+
// The second press is refused. Draft saves run one at a time
193+
// (`useDraftSaveGuard`'s queue), so a second save would wait behind the
194+
// held one and the server count alone cannot see it in flight: the switch
195+
// still showing the first flip can, and the count after release does.
196+
await user.keyboard(' ');
197+
await new Promise((r) => setTimeout(r, 50));
198+
expect(sw).toHaveAttribute('aria-checked', 'false');
199+
expect(server.saves).toHaveLength(1);
200+
201+
release();
202+
await waitFor(() => expect(sw).not.toHaveAttribute('aria-disabled'), { timeout: 8000 });
203+
if (refuse) {
204+
await screen.findByText(LOCKED_MESSAGE, undefined, { timeout: 8000 });
205+
await waitFor(() => expect(sw).toHaveAttribute('aria-checked', 'true'), { timeout: 8000 });
206+
} else {
207+
expect(sw).toHaveAttribute('aria-checked', 'false');
208+
}
209+
// The answer landed on the same switch, and focus is still on it.
210+
expect(screen.getByRole('switch')).toBe(sw);
211+
expect(document.activeElement).toBe(sw);
212+
await new Promise((r) => setTimeout(r, 50));
213+
expect(server.saves).toHaveLength(1);
214+
stopFixup();
215+
});
216+
217+
it('a read-only package’s switch stays disabled, out of the tab order, and described by its reason', async () => {
218+
renderPillar(true);
219+
const sw = await openFlow();
220+
expect(sw).toBeDisabled();
221+
expect(sw).not.toHaveAttribute('aria-disabled');
222+
expect(document.getElementById(sw.getAttribute('aria-describedby') ?? '')).toHaveTextContent(READ_ONLY_REASON);
223+
sw.focus();
224+
expect(document.activeElement).not.toBe(sw);
225+
});
226+
});

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

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -10858,13 +10858,25 @@ export function AutomationsPillar({
1085810858
: flowEnabled
1085910859
? 'bg-emerald-500'
1086010860
: 'bg-muted-foreground/40';
10861+
// objectstack#29376 — while a save is in flight the switch is
10862+
// `aria-disabled`, never `disabled`: a focused control that turns
10863+
// `disabled` drops focus to the page, and nothing hands it back when
10864+
// the save lands, so a keyboard author would tab back from the top
10865+
// after every flip. The press is refused in the handler instead, so a
10866+
// second Space sends no second save. A switch that cannot be flipped
10867+
// at all (no canvas, or a read-only package) stays `disabled`.
10868+
const inert = !isEditable || readOnly;
10869+
const busy = !inert && !!saving;
1086110870
const toggle = (
1086210871
<button
1086310872
type="button"
1086410873
role="switch"
1086510874
aria-checked={flowEnabled}
10866-
onClick={toggleEnabled}
10867-
disabled={!isEditable || !!saving || readOnly}
10875+
onClick={() => {
10876+
if (!saving) void toggleEnabled();
10877+
}}
10878+
disabled={inert}
10879+
aria-disabled={busy || undefined}
1086810880
aria-describedby={readOnly ? toggleReadOnlyReasonId : undefined}
1086910881
// A read-only package's reason is the wrapper's (objectui#12303),
1087010882
// not a `title` the disabled button can never show.
@@ -10877,7 +10889,7 @@ export function AutomationsPillar({
1087710889
'inline-flex h-7 items-center gap-2 rounded-md px-1 text-[12px] ' +
1087810890
(readOnly
1087910891
? 'text-muted-foreground disabled:pointer-events-none'
10880-
: 'text-foreground hover:bg-muted disabled:opacity-50')
10892+
: 'text-foreground hover:bg-muted disabled:opacity-50 aria-disabled:opacity-50')
1088110893
}
1088210894
>
1088310895
<span className={'relative inline-flex h-5 w-9 shrink-0 items-center rounded-full transition-colors ' + track}>

0 commit comments

Comments
 (0)