Skip to content

fix(ui): close archive Undo dialogs reliably - #871

Merged
cfal merged 2 commits into
mainfrom
fix/archive-dialog-escape
Oct 8, 2026
Merged

cfal merged 2 commits into
mainfrom
fix/archive-dialog-escape

Conversation

@yongkangc

@yongkangc yongkangc commented Oct 8, 2026 •

Copy link
Copy Markdown
Collaborator

Problem

The Chromium archive Undo test could press Escape before the bulk archive flow had mounted the New Chat dialog. CI then observed one dialog still open at archive-undo.test.ts:55 because the Escape event had no registered dialog layer to close.

When the New Chat project path had its directory picker open, closing that picker restored focus to the path input, whose focus handler immediately reopened the picker. Escape could not progress to the parent dialog.

Fix

  • Wait for the New Chat surface before exercising Escape.
  • Close the layered New Chat surfaces with a specific locator.
  • Suppress the path focus callback while the directory picker returns focus, with a bounded fallback for non-focus-restoring closes.
  • Cover the focus handoff with a component regression test.

Validation

  • bun run --cwd web test -- src/lib/components/project-paths/__tests__/ProjectPathField.test.ts (13 passed)
  • bun run --cwd web check
  • bun run build
  • bun test --max-concurrency=1 --timeout=180000 tests/chromium/archive-undo.test.ts (1 passed)
  • Reproduced the original failure from CI run 37772420268

Copilot AI balanced review requested due to automatic review settings October 8, 2026 13:22

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@cfal

cfal commented Oct 8, 2026

Copy link
Copy Markdown
Owner

Reviewed commit: 89f40242fbab758b9e26e6a2e7812396d006ca33

High

  • archive-undo.test.ts:50-56: The new unscoped getByRole('dialog') assertion fails Playwright strict mode when both the New Chat modal and its Select Directory child dialog are visible. The reviewed commit’s Chromium E2E check fails at this exact line with two matches. Merely scoping the assertion is insufficient because one Escape would close the child layer rather than New Chat. Fix by locating the New Chat modal specifically and dismissing its Close control, or explicitly close and await removal of the directory browser before sending Escape to the parent. Then rerun archive-undo.test.ts.

@yongkangc yongkangc changed the title test(chromium): synchronize archive dialog Escape fix(ui): close archive Undo dialogs reliably Oct 8, 2026
@cfal
cfal merged commit 0602da8 into main Oct 8, 2026
26 checks passed
@cfal
cfal deleted the fix/archive-dialog-escape branch October 8, 2026 14:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants