Skip to content

fix(composer): restore focus after dialog close#16733

Open
ousugo wants to merge 1 commit into
CherryHQ:mainfrom
ousugo:fix/composer-focus-restore
Open

fix(composer): restore focus after dialog close#16733
ousugo wants to merge 1 commit into
CherryHQ:mainfrom
ousugo:fix/composer-focus-restore

Conversation

@ousugo

@ousugo ousugo commented Jul 4, 2026

Copy link
Copy Markdown
Collaborator

🚨 Branch strategy — read before opening this PR

The v2 refactor has merged into main, so main is the default branch for active development (v1 and v2 code currently coexist there — expect large, breaking changes).

  • Active development (features, refactors, optimizations, fixes for the current codebase) → target main (the default base).
  • v1 maintenance (hotfixes and subsequent v1 releases) → branch from and target v1, not main.

A v1 fix does not auto-carry to main: if the same bug exists on main, open a separate forward-port PR targeting main. Before touching subsystems being replaced, read docs/references/data/ and watch for @deprecated markers — they flag code being deleted.

What this PR does

Before this PR:

Closing composer-adjacent dialogs and native pickers could leave focus on the dialog trigger, the page body, or another intermediate control instead of returning to the composer input. The save path for assistant/agent edit dialogs could also schedule duplicate focus restoration callbacks.

After this PR:

Composer tool actions and assistant/agent resource dialogs restore focus consistently after closing. Save callbacks refresh data without scheduling a second focus restoration, and tests cover cancel/save paths plus tool-menu and visible-button web search flows.

Fixes # N/A

Why we need it and why it was done in this way

The following tradeoffs were made:

Focus restoration is explicitly wired through the composer input adapter and deferred with requestAnimationFrame so it runs after dialogs and overlays close. For the visible Web Search button, the fallback focus target is the clicked trigger because that path does not have a composer input adapter.

The following alternatives were considered:

Relying on Radix/native default focus behavior was rejected because these flows are opened from several indirect entry points and the restored target was inconsistent. Using duplicated requestAnimationFrame stubs in each test was also rejected in favor of a shared test helper.

Links to places where the discussion took place: N/A

Breaking changes

If this PR introduces breaking changes, please describe the changes and the impact on users.

None.

Special notes for your reviewer

The file picker cancel path intentionally restores focus to the composer as part of the same "close the transient UI and continue typing" behavior.

Checklist

This checklist is not enforcing, but it's a reminder of items that could be relevant to every PR.
Approvers are expected to review this list.

  • Branch: This PR targets the correct branch — main for active development, v1 for v1 maintenance fixes
  • PR: The PR description is expressive enough and will help future contributors
  • Code: Write code that humans can understand and Keep it simple
  • Refactor: You have left the code cleaner than you found it (Boy Scout Rule)
  • Upgrade: Impact of this change on upgrade flows was considered and addressed if required
  • Documentation: A user-guide update was considered and is present (link) or not required. Check this only when the PR introduces or changes a user-facing feature or behavior.
  • Self-review: I have reviewed my own code (e.g., via /gh-pr-review, gh pr diff, or GitHub UI) before requesting review from others

Release note

Fixes composer focus restoration after closing prompt, resource, web search, and file picker dialogs.

@ousugo ousugo requested a review from a team July 4, 2026 00:26
@ousugo ousugo closed this Jul 4, 2026
@ousugo ousugo deleted the fix/composer-focus-restore branch July 4, 2026 12:35
@ousugo ousugo restored the fix/composer-focus-restore branch July 4, 2026 12:55
@ousugo ousugo reopened this Jul 4, 2026
@ousugo ousugo reopened this Jul 4, 2026

@kangfenmao kangfenmao left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

LGTM — verified no double focus restoration on edit-save (onOpenChange fires before onSaved; handleEditSaved intentionally does not re-restore) and single restore on create-submit/cancel paths. Visible WebSearch button safely captures event.currentTarget before the async close. Types and renderBelowControls wiring check out; CI green. Two optional test-convention nits (prefer the @test-mocks/requestAnimationFrame alias over the deep relative path; move the mid-file import to the top in the three tool test files) — non-blocking.

@ousugo ousugo force-pushed the fix/composer-focus-restore branch from cbac630 to 0932be5 Compare July 6, 2026 08:27
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.

2 participants