Skip to content

fix(web): Esc closes the theme editor, not Settings - #25130

Open
AatmanAJ wants to merge 1 commit into
pingdotgg:mainfrom
AatmanAJ:fix/web-theme-editor-escape
Open

AatmanAJ wants to merge 1 commit into
pingdotgg:mainfrom
AatmanAJ:fix/web-theme-editor-escape

Conversation

@AatmanAJ

Copy link
Copy Markdown
Contributor

Problem

In Settings > Appearance, the theme editor floats beside the Settings dialog, and Esc reached the dialog in two cases:

  • Esc while inspecting cancelled the inspector and also closed Settings, although the header says "Esc to cancel".
  • Esc with focus inside the editor, for example in the theme name, closed Settings while the editor stayed open over the next page.

This is a small, focused fix for an obvious bug, so I am submitting it without a prior issue: the editor's own header says "Esc to cancel", but Esc also closes all of Settings.

Change

  • The inspector's capture-phase Esc listener also calls stopImmediatePropagation(), so Esc unwinds only the inspector.
  • The editor panel closes itself on an Esc pressed inside it and stops the event. Popups opened from the editor are portaled outside the panel and keep their own Esc.

Verification

  • New browser test ThemeEditorPanel.escape.browser.test.tsx renders the editor beside a modal dialog: Esc in the editor closes only the editor, and Esc while inspecting closes neither. Both cases fail on main and pass with this change.
  • Lint and the web typecheck are clean.

Before / After

Before

After

Full-quality recordings: before (MP4) · after (MP4)

🤖 Generated with Claude Code

Esc while inspecting cancelled the inspector and also closed Settings,
and Esc inside the editor closed Settings while the editor stayed open.
Stop the inspector's Esc from reaching other listeners, and close the
editor on an Esc pressed inside it. Popups opened from the editor are
portaled outside it and keep their own Esc.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@github-actions github-actions Bot added vouch:unvouched PR author is not yet trusted in the VOUCHED list. size:XS 0-9 changed lines (additions + deletions). labels Oct 25, 2026
@macroscopeapp

macroscopeapp Bot commented Oct 25, 2026

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Approved at ab1afdb

Macroscope's review found this PR approvable — This is a small, self-contained Escape-key bug fix: inspection now cancels without closing Settings, while Escape inside the theme editor closes only the editor. The runtime change is localized and accompanied by focused browser coverage.

You can add or adjust custom eligibility rules. Learn more.

@coderabbitai

coderabbitai Bot commented Oct 25, 2026

Copy link
Copy Markdown
Contributor

Review in Change Stack →

No actionable comments were generated in the recent review.

Recent review info
Run configuration
Commits

Reviewing files that changed from the base of the PR and between b76a110 and ab1afdb.

Files selected for processing: 2
  • apps/web/src/components/settings/ThemeEditorPanel.escape.browser.test.tsx
  • apps/web/src/components/settings/ThemeEditorPanel.tsx

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 8 remain after this review.

Walkthrough

The theme editor now handles Escape events during color inspection and editor use. Browser tests check that these events do not trigger the Settings dialog’s open-change callback.

Changes: Theme editor Escape handling

Layer / File(s) Summary
Escape event handling and dialog isolation
apps/web/src/components/settings/ThemeEditorPanel.tsx, apps/web/src/components/settings/ThemeEditorPanel.escape.browser.test.tsx
The panel stops Escape events during color inspection and closes the editor for eligible Escape events inside the panel. Browser tests check both behaviors while Settings is open.

Priority: Normal

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix

Suggested labels: size:XS

Suggested reviewers: maria-rcks, macroscopeapp

Merge Risk: Minimal · up to ab1af

The Escape-handling change is ready to merge with no identified actionable risk.

Pre-merge checks: Passed 4
Check name Status Explanation
Title check Passed The title "fix(web): Esc closes the theme editor, not Settings" follows conventional commit format and names the primary change: Esc should close the theme editor rather than the Settings dialog. The …
Description check Passed The description follows the template's main sections: Problem, Change, and Verification. Problem and Change are clearly filled in with expected behavior, reproduction context, and a concrete explanati…
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Full details: Title check

Explanation

The title "fix(web): Esc closes the theme editor, not Settings" follows conventional commit format and names the primary change: Esc should close the theme editor rather than the Settings dialog. The diff adds stopImmediatePropagation to the inspector's Esc listener and closes the editor on Esc inside it, which matches the title. The title is concise, specific, and not vague.

Full details: Description check

Explanation

The description follows the template's main sections: Problem, Change, and Verification. Problem and Change are clearly filled in with expected behavior, reproduction context, and a concrete explanation of the fix. Verification names a specific browser test, states the observed pass/fail results on main versus the change, reports lint and typecheck results, and includes before/after GIFs and MP4 links. The Scope and approval section is missing. The author does explain why the fix qualifies as a small, focused fix of an obvious bug without a prior issue, which is the exemption the template describes, but no triaged issue or discussion link is provided. The description also ends with a Claude Code attribution rather than the required model and harness line, though it names the tool. These gaps are minor relative to the overall completeness.

Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Comment @coderabbitai help to get the list of available commands.

maria-rcks commented Oct 30, 2026 •

Copy link
Copy Markdown
Collaborator

Note

Written by gpt-6.1-sol on behalf of Maria

Verified in a real dev environment

verified exact head ab1afdb10dcb41295ca4f1cb32db14dff8e781ab with playwright from the repo and /usr/bin/google-chrome on linux (1440 × 1000). installed each checkout with vp i, ran main and the pr sequentially with isolated state, and used the real dev web client. no mocks, fixtures, or demo provider.

baseline: github/main at 32ecd71726, in a separate worktree. settings > appearance > create theme > type "Escape verification" > escape closed settings and left the editor floating over chat. inspection > escape also closed settings.

pr: the same name-field escape closes only the editor. inspection > escape cancels inspection while keeping the editor and settings open. neighboring checks: escape dismisses the accent color popup while keeping its editor and settings; after closing the editor, escape closes settings normally.

created a project for the real /home/user/t3code git repository and a thread through the app. selected codex gpt-6.1-sol, submitted a prompt, and saw the actual provider reply "theme escape verification ready." in the client.

blacksmith: vp test run apps/web/src/components/settings/ThemeEditorPanel.escape.browser.test.tsx passed both tests (after installing the worker's missing playwright browser). existing hosted checks: 9 successful, 0 failing, 0 pending. no source changes or pushes were needed.

before: main after escape

main leaves the editor open after closing settings

after: pr after the same escape

pr closes the editor and preserves settings

pr inspection cancellation preserves editor and settings

pr escape interaction recording

real codex gpt-6.1-sol reply in the client

recording inspected; uploaded playback and seeking checked in the t3 browser player. the clip shows interaction state changes, not a performance measurement.

Not verified

native electron, ios/android, macos/windows, remote relay/tunnel connections, and other providers were not exercised on this linux web pass. no full repository suite was run. current-base conflict evaluation was pending in git.cafe; this is not a merge-readiness claim.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:XS 0-9 changed lines (additions + deletions). vouch:unvouched PR author is not yet trusted in the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants