From 8d0540af50ddfb53f40de010d58fff6730f81f53 Mon Sep 17 00:00:00 2001 From: Chinesezjc Date: Tue, 20 Oct 2026 19:47:46 +0800 Subject: [PATCH] fix(schedule): hand focus to the heading when a late readback lands --- apps/web/tests/schedule-after.e2e.ts | 4 +-- .../src/client/TaskManagerPage.tsx | 29 +++++++++++++++---- .../tests/task-manager-page.client.spec.tsx | 18 ++++++++++++ 3 files changed, 43 insertions(+), 8 deletions(-) diff --git a/apps/web/tests/schedule-after.e2e.ts b/apps/web/tests/schedule-after.e2e.ts index 32fdd747ee..aa089709c3 100644 --- a/apps/web/tests/schedule-after.e2e.ts +++ b/apps/web/tests/schedule-after.e2e.ts @@ -1318,8 +1318,8 @@ describe.skipIf(MODE === 'record')('web e2e: active Schedule catalog', () => { expect(remaining.map(task => task.title)).toContain('Keep this running task') await page.getByText('Cleared 2 inactive tasks.', { exact: true }).waitFor({ timeout: 15_000 }) // The trigger left with the rows it removed, so the page heading owns the - // focus after the confirmation closes (a browser-level check: jsdom does - // not move focus to the document body when a focused control unmounts). + // focus after the confirmation closes; this confirms the heading fallback + // in a real browser. await expect.poll(() => page.evaluate(() => document.activeElement?.tagName), { timeout: 15_000 }) .toBe('H1') // The page follows the acknowledged removal: no ended row or cleanup action diff --git a/packages/client/ui-schedule/src/client/TaskManagerPage.tsx b/packages/client/ui-schedule/src/client/TaskManagerPage.tsx index 6940f448a8..0d38531291 100644 --- a/packages/client/ui-schedule/src/client/TaskManagerPage.tsx +++ b/packages/client/ui-schedule/src/client/TaskManagerPage.tsx @@ -70,6 +70,9 @@ export function TaskManagerPage(props: TaskManagerPageProps) { // Set when the confirmation opens: its settled removal can hide the trigger // before the Modal returns focus, and so can another client's cleanup. const clearFocusPending = useRef(false) + // Set when the removal settles: a readback superseded by `schedule/changed` + // can publish its rows after the dialog closed, removing the trigger then. + const clearSettled = useRef(false) const confirming = records.find(record => record.id === confirmId) // The bulk action's target and count come from the complete catalog, not from // the filtered rows: it removes every ended task, whatever the view shows. @@ -124,13 +127,26 @@ export function TaskManagerPage(props: TaskManagerPageProps) { // The Modal restores focus to the element it captured when it opened; the // trigger can be removed or rebuilt while the confirmation is open, so the // page points focus at the current control, or the heading when it is gone. + // A settled removal whose readback lands later unmounts the trigger after the + // dialog closed, which drops focus to the document body; that handoff is + // caught by the same effect watching the ended count. useEffect(() => { - if (clearConfirm || !clearFocusPending.current) return - clearFocusPending.current = false - const trigger = clearTriggerRef.current - const target = trigger?.isConnected === true ? trigger : headingRef.current - target?.focus() - }, [clearConfirm]) + if (clearConfirm) return + if (clearFocusPending.current) { + clearFocusPending.current = false + const trigger = clearTriggerRef.current + if (trigger?.isConnected === true) { + trigger.focus() + return + } + clearSettled.current = false + headingRef.current?.focus() + return + } + if (!clearSettled.current || endedCount > 0 || document.activeElement !== document.body) return + clearSettled.current = false + headingRef.current?.focus() + }, [clearConfirm, endedCount]) const confirmClearEnded = (): void => { // The confirmation button stays focusable while the removal runs, so a @@ -138,6 +154,7 @@ export function TaskManagerPage(props: TaskManagerPageProps) { if (clearing) return setClearing(true) void onClearEnded().finally(() => { + clearSettled.current = true setClearing(false) setClearConfirm(false) }) diff --git a/packages/client/ui-schedule/tests/task-manager-page.client.spec.tsx b/packages/client/ui-schedule/tests/task-manager-page.client.spec.tsx index 23fb019d07..ae5eaae4ca 100644 --- a/packages/client/ui-schedule/tests/task-manager-page.client.spec.tsx +++ b/packages/client/ui-schedule/tests/task-manager-page.client.spec.tsx @@ -1085,6 +1085,24 @@ describe('clearing inactive tasks', () => { expect(document.activeElement).toBe(rebuilt) }) + it('moves focus to the heading when a late readback removes the trigger', async () => { + const pending = Promise.withResolvers() + const onClearEnded = vi.fn(() => pending.promise) + const h = mount({ records: [at, endedTask] }, en, { onClearEnded }) + const trigger = screen.getByRole('button', { name: en['clear.action'] }) + trigger.focus() + fireEvent.click(trigger) + fireEvent.click(screen.getByRole('button', { name: en['clear.confirm'] })) + // The removal settles while the catalog still lists the ended row: the + // dialog closes and focus returns to the trigger. + await act(async () => { pending.resolve(undefined) }) + expect(document.activeElement).toBe(trigger) + // A readback superseded by `schedule/changed` publishes afterwards and takes + // the trigger with its row; focus must not stay on the document body. + act(() => { h.update({ records: [at] }) }) + expect(document.activeElement).toBe(screen.getByRole('heading', { level: 1 })) + }) + it('closes only the confirmation on Escape while a task detail is open', () => { mount({ records: [at, endedTask] }) fireEvent.click(screen.getByRole('button', { name: 'Review release' }))