Skip to content

Commit 64ba32b

Browse files
fix(app-shell): Studio's flow ledger switch keeps keyboard focus across a toggle (#29375)
<!-- claude-projects-attribution: {"github_login":"zhuangjianguo"} --> _Requested by **Steedos** · [project thread](https://claude.ai/code/project/chan_018ETKaNwMZCNRYD7D8Q9Ubb?thread=cmsg_018ETKaNwMZCNRYD7D8Q9UbbSUpafmdjxMoChnMwbXjjou)_ Fixes #29228 Clause-②: no Metadata-protocol: none The two lines are copied from the claim (comment 6576300768). ## 维护者速读 - 改了什么:Studio › Automations 里只读包流程的「本部署开/关」开关(`FlowLedgerSwitch`),请求进行中不再是 `disabled`,改为 `aria-disabled`,并在开关自己的点击处理里拒绝再次按下;被部署姿态挡住(walled)时仍是 `disabled`,说明文字不变。 - 为什么改:浏览器会把焦点从一个变成 `disabled` 的已聚焦控件挪到页面 body,请求结束后也不会还回来。用键盘或读屏的作者每拨一次开关(成功或 409 被拒都一样),都得从页面顶部重新 Tab 回来。本 PR 在真实 Chromium 上前后对比实测过。 - 风险与回滚:只改一个组件的三处属性和一个点击守卫;视觉上请求中仍是半透明(`aria-disabled:opacity-50`)。请求中按第二次空格不发第二个请求,由组件内守卫保证(测试与消融都钉住了)。回滚即撤销本 PR。 - 先看:`FlowLedgerSwitch.tsx` 的 `onClick` / `disabled` / `aria-disabled` 三行;`AutomationsPillar.ledgerSwitch-28148.test.tsx` 末尾新增的 describe 块(含模拟浏览器 focus fixup 的 `runFocusFixup`)。 - 两分钟验证:`pnpm --dir ui exec vitest run packages/app-shell/src/views/studio-design/AutomationsPillar.ledgerSwitch-28148.test.tsx`,预期 12 个用例全部通过。 - 需要你做的:无,席位落地。 ## What changes - `ui/packages/app-shell/src/views/studio-design/FlowLedgerSwitch.tsx`: while a toggle is in flight (`busy`), the switch carries `aria-disabled="true"` instead of `disabled`, and its `onClick` refuses the press while `busy`. `disabled` stays for the walled case only (`disabled={!!walledReason}`), so a walled switch is unchanged: disabled, out of the tab order, described by the console's sentence through its focusable wrapper. The in-flight dimming moves from `disabled:opacity-50` to `aria-disabled:opacity-50` on the same branch, so the switch looks as before. - The doc comment says why the busy state is `aria-disabled`. - The host (`StudioDesignSurface.tsx`) is not touched: the switch keeps focus by never losing it, so nothing has to restore it. ## Why this approach, not focus restore The card offered two routes. I chose `aria-disabled` plus a guarded handler while busy: - **Focus never leaves.** With restore, focus still drops to the body for the whole flight and comes back when the answer lands, so a screen reader announces the page body and then the switch again on every flip. With `aria-disabled` there is nothing to announce in between: the switch stays focused and is read as unavailable while the request runs. - **No host plumbing.** Restore needs a ref from the pillar and a focus call after both the success and the refusal paths (the refusal sets the pillar's shared `error`, which also renders the refusal strip). Keeping it in the component keeps the file surface to the component. - **The double-toggle guard sits beside the switch.** `toggleLedger` in the pillar has no busy check of its own; before, `disabled` was the guard. Now the component's handler is, and a test pins it. ## Tests `AutomationsPillar.ledgerSwitch-28148.test.tsx`, a new describe block on the existing harness (the toggle door doubled at `fetch`, held on a gate so the request is in flight): - `it.each` over an accepted flip and a refused one (409 `DELETE_RESTRICTED`): focus the switch, press Space with `@testing-library/user-event`. While the request is in flight, `document.activeElement` is the switch, the switch is `aria-disabled="true"` and still enabled, and a second Space sends no second request. After the answer (the switch reads off, or the 409 sentence is the alert and the switch still reads on), the switch is the same element and `document.activeElement` is still the switch. - The walled case: the switch is `disabled`, carries no `aria-disabled`, is described by the walled reason, and cannot take focus. The existing walled case (nothing sent on a click) stays as it was. - **Why the test runs a focus fixup.** happy-dom keeps focus on a control that turns `disabled`; a browser does not (measured below). Without help, the focus pins could not fail. `runFocusFixup` watches the switch's `disabled` attribute and moves focus to the body when the focused switch turns disabled, as the browser does. (happy-dom's `blur()` does nothing on a disabled control, so it focuses the body directly.) - Ablation 1, the card's: `disabled={busy || !!walledReason}` restored through `scripts/ablation-replace.mjs` (anchor x1 to x0, blob d907ded to 2420b809): both new focus cases red at `expect(document.activeElement).toBe(sw)` (received the body), 10 others green. Restore proven: blob equals HEAD, `git diff HEAD` empty. - Ablation 2, the guard: `if (!busy) onToggle();` replaced by an unguarded `onToggle()` (blob d907ded to e0ae1470): both new cases red at the request count (2 requests, expected 1). Restored, same proof. - `AutomationsPillar.ledgerSwitch-28148.test.tsx` 12/12 and `AutomationsPillar.customizeFlow-28570.test.tsx` (the other file that draws the ledger switch) 10/10, at 9dfcb64. ## Acceptance notes - **The writable package's definition switch has the same defect (measured, outside this card's file surface).** On a writable package's flow, Studio draws the definition switch in `StudioDesignSurface.tsx` with `disabled={!isEditable || !!saving || readOnly}`. Measured live on the same showcase: a scratch writable package with one flow, the definition switch focused, Space; the draft save answered 200 and `document.activeElement` was the body afterwards. Reported to the seat as a finding; not changed here. - Live probe scripts and readings are in the card thread's project folder (`issue-29228/live/`). ## Verification - **Live, before and after, in Chromium 153** (head cd93d34 under a Vite-served Console, backend `objectstack dev --seed-admin --fresh` from `examples/app-showcase`, signed in as the seeded admin). Studio › Automations on the read-only showcase package, the ledger switch focused, Space, then a second Space while the request was in flight (the toggle answer held 800 ms by a route handler): - accepted leg, `showcase_task_done_notify_owner` (200, put back afterwards through the same door) and refused leg, `showcase_notify_owner` (409 `DELETE_RESTRICTED`, naming its caller); - this branch, both legs: in flight `document.activeElement` is the switch, `aria-disabled="true"`, `disabled` false; after the answer, still the switch; one toggle request per leg; - origin/main's `FlowLedgerSwitch.tsx` swapped in under Vite (then restored, blob equal to HEAD, `git diff HEAD` empty), both legs: in flight `disabled` true and `document.activeElement` is the body; after the answer, still the body; one toggle request per leg; - after the restore, this branch again: both legs keep focus on the switch. - **Mechanism, bare page in the same Chromium:** a focused button set `disabled` leaves `document.activeElement` on the body, and enabling it again does not bring focus back; a focused button set `aria-disabled` keeps focus. - **Gates:** `node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack` derived 34 commands for this diff. All 34 ran at 951c859, each exit 0, including `node scripts/check-sdui-manifest.mjs` and `pnpm --dir ui exec vitest run scripts/` (169 files, 4903 tests). `--ran` reconciliation: 34 derived, 34 run, 0 NOT-MEASURED. After the two merges of main, the 31 faster commands ran again at 9dfcb64, each exit 0; `check-comment-mask-corpus`, `check:query-options-erasure` and the ui `scripts/` suite were not re-run (the merges bring nothing into the three files of this diff). - **Type-check:** `turbo run type-check --filter=@object-ui/app-shell` (both `tsc --noEmit` and `tsc -p tsconfig.test.json`) green at 951c859. - **Lint:** eslint on the two touched ui files, exit 0. The repo-wide run is CI's Lint UI. ## 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_01QsR2jTE8TKgsLoAjojtZ4P --- _Generated by [Claude Code](https://claude.ai/code/session_01QsR2jTE8TKgsLoAjojtZ4P)_ --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 53dda03 commit 64ba32b

3 files changed

Lines changed: 101 additions & 4 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 ledger switch keeps keyboard focus while its toggle is in flight and after the answer lands
6+
7+
Clause-②: no
8+
Metadata-protocol: none
9+
10+
On Studio › Automations, a read-only package's flow offers the switch that turns it on or off for this deployment. While a toggle 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 answer landed. A keyboard or screen reader author had to tab back from the top of the page after every flip, accepted 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 request. A switch the deployment's posture walls off stays `disabled` and described by its reason, as before.

‎ui/packages/app-shell/src/views/studio-design/AutomationsPillar.ledgerSwitch-28148.test.tsx‎

Lines changed: 79 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,10 @@
2222
* - a refusal is the server's sentence, verbatim, announced as an alert
2323
* (objectstack#29139), and announced again when the next flip is refused;
2424
* - a writable package's flow (CONTROL) keeps its definition switch and its
25-
* Delete, with no ledger switch.
25+
* Delete, with no ledger switch;
26+
* - a keyboard flip keeps focus on the switch while the request is in flight
27+
* and after its answer lands, accepted or refused, and a second Space in
28+
* flight sends nothing (objectstack#29228).
2629
*
2730
* The harness is `AutomationsPillar.deleteFlow-27356`'s: the metadata server
2831
* doubled at the client, the `/automation` doors doubled at `fetch`.
@@ -32,6 +35,7 @@ import '@testing-library/jest-dom/vitest';
3235
import * as React from 'react';
3336
import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest';
3437
import { render, screen, fireEvent, cleanup, waitFor } from '@testing-library/react';
38+
import userEvent from '@testing-library/user-event';
3539
import { toast } from 'sonner';
3640
import { MemoryRouter } from 'react-router-dom';
3741

@@ -297,3 +301,77 @@ describe('Automations — the ledger switch on a read-only package (ADR-0146 D4)
297301
expect(await screen.findByTestId('auto-item-delete')).toBeInTheDocument();
298302
});
299303
});
304+
305+
// objectstack#29228 — a focused control that turns `disabled` drops focus to
306+
// the page, and nothing hands it back. In flight the switch is refused to a
307+
// press but stays focusable, so a keyboard author's focus never leaves it.
308+
// A browser runs the HTML focus fixup: a focused control that turns `disabled`
309+
// drops focus to the body (measured in Chromium 153, and not handed back when
310+
// it is enabled again). happy-dom keeps focus on it, so the test runs the
311+
// fixup on the switch the way the browser does, else the focus pins could not
312+
// fail. (happy-dom's `blur()` does nothing on a disabled control, so focus is
313+
// moved to the body itself.)
314+
function runFocusFixup(el: HTMLElement): () => void {
315+
const observer = new MutationObserver(() => {
316+
if (el.hasAttribute('disabled') && document.activeElement === el) document.body.focus();
317+
});
318+
observer.observe(el, { attributes: true, attributeFilter: ['disabled'] });
319+
return () => observer.disconnect();
320+
}
321+
322+
describe('Automations — the ledger switch keeps keyboard focus across a flip (objectstack#29228)', () => {
323+
const subflowSentence =
324+
"Flow 'approval' cannot be disabled while 1 packaged flow still calls it as a subflow: 'notify_owner'.";
325+
326+
it.each([
327+
['accepted', null],
328+
['refused (409)', { status: 409, body: { success: false, error: { code: 'DELETE_RESTRICTED', message: subflowSentence } } }],
329+
])('a keyboard flip that is %s leaves focus on the switch, and a second Space in flight sends nothing', async (_answer, refusal) => {
330+
doors.toggle = refusal;
331+
let release!: () => void;
332+
doors.toggleGate = new Promise<void>((resolve) => {
333+
release = resolve;
334+
});
335+
const user = userEvent.setup();
336+
mount();
337+
const sw = await ledgerSwitch();
338+
const stopFixup = runFocusFixup(sw);
339+
sw.focus();
340+
expect(document.activeElement).toBe(sw);
341+
342+
await user.keyboard(' ');
343+
await waitFor(() => expect(calls('/automation/approval/toggle')).toHaveLength(1));
344+
await waitFor(() => expect(sw).toHaveAttribute('aria-disabled', 'true'));
345+
// In flight: still focused, refused to a press, still focusable.
346+
expect(document.activeElement).toBe(sw);
347+
expect(sw).toBeEnabled();
348+
await user.keyboard(' ');
349+
await new Promise((r) => setTimeout(r, 50));
350+
expect(calls('/automation/approval/toggle')).toHaveLength(1);
351+
352+
release();
353+
await waitFor(() => expect(sw).not.toHaveAttribute('aria-disabled'));
354+
if (refusal) {
355+
expect((await screen.findByRole('alert')).textContent).toBe(subflowSentence);
356+
expect(sw).toHaveAttribute('aria-checked', 'true');
357+
} else {
358+
await waitFor(() => expect(sw).toHaveAttribute('aria-checked', 'false'));
359+
}
360+
// The answer landed on the same switch, and focus is still on it.
361+
expect(screen.getByTestId('auto-ledger-switch')).toBe(sw);
362+
expect(document.activeElement).toBe(sw);
363+
stopFixup();
364+
});
365+
366+
it('a walled switch stays disabled, out of the tab order, and described by its reason', async () => {
367+
posture = 'group';
368+
operator = false;
369+
mount();
370+
const sw = await ledgerSwitch();
371+
expect(sw).toBeDisabled();
372+
expect(sw).not.toHaveAttribute('aria-disabled');
373+
expect(sw.getAttribute('aria-describedby')).toContain(screen.getByTestId('auto-ledger-walled-reason').id);
374+
sw.focus();
375+
expect(document.activeElement).not.toBe(sw);
376+
});
377+
});

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

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,12 @@
2121
* own sentence for why (`notRunningReason`, rendered verbatim, ⛔ never parsed).
2222
* When the caller's posture facts say the door refuses them (`walledReason`),
2323
* the switch draws disabled, described by the console's own sentence.
24+
*
25+
* While a toggle is in flight (`busy`) the switch is `aria-disabled`, never
26+
* `disabled`: a focused control that turns `disabled` drops focus to the page,
27+
* and nothing hands it back when the answer lands, so a keyboard author would
28+
* tab back from the top after every flip. The press is refused here instead,
29+
* so a second Space sends no second request.
2430
*/
2531

2632
import * as React from 'react';
@@ -57,15 +63,18 @@ export function FlowLedgerSwitch({ checked, reading, notRunningReason, busy, wal
5763
aria-checked={checked}
5864
data-testid="auto-ledger-switch"
5965
data-reading={reading}
60-
onClick={onToggle}
61-
disabled={busy || !!walledReason}
66+
onClick={() => {
67+
if (!busy) onToggle();
68+
}}
69+
disabled={!!walledReason}
70+
aria-disabled={busy || undefined}
6271
aria-describedby={describedBy}
6372
title={walledReason ? undefined : why ? `${hint}\n${why}` : hint}
6473
className={
6574
'inline-flex h-7 items-center gap-2 rounded-md px-1 text-[12px] ' +
6675
(walledReason
6776
? 'text-muted-foreground disabled:pointer-events-none'
68-
: 'text-foreground hover:bg-muted disabled:opacity-50')
77+
: 'text-foreground hover:bg-muted aria-disabled:opacity-50')
6978
}
7079
>
7180
<span

0 commit comments

Comments
 (0)