Skip to content

fix(web): keep dialog controls clickable over desktop titlebars - #10978

Closed
efemaer wants to merge 6 commits into
pingdotgg:mainfrom
efemaer:codex/fix-theme-modal-close
Closed

efemaer wants to merge 6 commits into
pingdotgg:mainfrom
efemaer:codex/fix-theme-modal-close

Conversation

@efemaer

@efemaer efemaer commented Sep 9, 2026

Copy link
Copy Markdown

Fixes #11573

What Changed

Mark shared dialog popups with -webkit-app-region: no-drag so their controls remain interactive when they overlap a desktop titlebar. This restores the theme import dialog's existing X hover state and center click, and covers the shared confirmation-dialog and command-palette popup styles.

Why

Searching for a theme makes the modal taller and moves its close button into the desktop header's window-drag region. Electron intercepts native pointer events in that region even though the dialog renders above it. The center of the X does not hover or close the dialog; the lower edge below the drag strip remains clickable.

Reproduction:

  1. In the desktop app, open Settings → Appearance → Themes → Add theme.
  2. Select Dracula under Popular, or search for it, and wait for results.
  3. With the expanded modal near the top of the window, click the center of the X, then its bottom edge.

Reproduced on macOS with native pointer input: two center clicks left the dialog open, while a lower-edge click closed it. Browser checks at 1280×900 and 1280×720 passed even before the fix, which narrowed the problem to desktop hit testing.

Verification

  • Reproduced the failure in an isolated Electron 44.1.0 window rendering the real ThemeImportDialog and WorkspacePageHeader. With the desktop drag region still enabled, the fixed X shows its hover state and closes on a native center click.
  • Repeated the original theme-search/close flow in the full local web app after the fix.
  • Full web unit suite: 351 files, 4,453 tests passed.
  • Web typecheck and targeted lint/format checks passed; standards and spec reviews found no issues.

The native pointer check is the regression proof here: the existing non-browser unit suite cannot exercise Electron's window-drag hit testing. No class-string assertion was added.

UI Changes

Evidence uses an isolated reproduction with no user data. Screenshots and video are cropped to the dialog header and search controls. The video labels mark the tested click locations; pauses are trimmed.

Before: the pointer is over the X center, but the hover background does not appear.

Before: the X center does not respond to hover

After: the same center position shows the existing hover background.

After: the X center responds to hover

Native interaction recording (11 seconds): center click ignored before the fix; bottom-edge click succeeds; center click succeeds after the fix.

theme-close-before-after.mp4

Checklist

  • This PR is small and focused
  • I explained what changed and why
  • I included before/after screenshots for any UI changes
  • I included a video for animation/interaction changes

Implemented with GPT-6-Astra in the Codex harness.

Summary by CodeRabbit

  • Bug Fixes
    • Improved dialog interaction in webkit-based desktop apps by ensuring dialog popups can be selected and interacted with independently of draggable window areas.

Exclude shared dialog popups from Electron window dragging. Tall theme search results place the close button over the titlebar drag region, which intercepted its center while leaving the bottom edge clickable.

Verified native Electron center clicks before and after, the full browser theme flow, all 4,453 web tests, web typecheck, and targeted lint/format checks. Standards and spec reviews found no issues.

Implemented with GPT-6-Astra in Codex.
@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 Sep 9, 2026
@efemaer
efemaer marked this pull request as ready for review September 9, 2026 19:21
@coderabbitai

coderabbitai Bot commented Sep 9, 2026

Copy link
Copy Markdown

Review Change StackReview Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Advanced

Run ID: 7f23b20f-8ce6-4a70-b2c7-1135d7ef11c2

📥 Commits

Reviewing files that changed from the base of the PR and between 61f35d1 and 1902f50.

📒 Files selected for processing (1)
  • apps/web/src/components/ui/dialog-styles.ts

Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review.


📝 Walkthrough

Walkthrough

The dialog popup class now marks the popup as outside the WebKit draggable window region. All other dialog classes remain unchanged.

Changes

Dialog popup interaction

Layer / File(s) Summary
Exclude dialog popup from dragging
apps/web/src/components/ui/dialog-styles.ts
DIALOG_POPUP_BASE_CLASS now includes [-webkit-app-region:no-drag] at the start of its class string.

Priority: ⬇️ Low

Estimated code review effort: 1 (Trivial) | ~2 minutes

Change: Bug fix

Suggested reviewers: chrisdeeming

Merge Risk: ⚪ Minimal · up to 1902f

The popup controls are excluded from the native drag region, preserving interaction without introducing merge-blocking risk.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 1…
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.
Title check ✅ Passed The title clearly and concisely describes the main fix: keeping dialog controls clickable over desktop titlebars.
Description check ✅ Passed The description is complete and focused. It explains the change, cause, reproduction steps, verification results, UI impact, screenshots, video, and checklist completion.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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

Preserve the dialog no-drag fix in the shared popup base while retaining main's standard and media dialog variants.
@efemaer
efemaer marked this pull request as draft September 13, 2026 09:55
@efemaer
efemaer marked this pull request as ready for review September 13, 2026 09:55
@cursor

cursor Bot commented Sep 13, 2026

Copy link
Copy Markdown
Contributor

Bugbot is paused — on-demand spend limit reached

Bugbot uses usage-based billing for this team and has hit its on-demand spend limit.

A team admin can raise the spend limit in the Cursor dashboard, or wait for the next billing cycle to continue.

@cursor

cursor Bot commented Sep 13, 2026

Copy link
Copy Markdown
Contributor

Bugbot is paused — on-demand spend limit reached

Bugbot uses usage-based billing for this team and has hit its on-demand spend limit.

A team admin can raise the spend limit in the Cursor dashboard, or wait for the next billing cycle to continue.

@juliusmarminge

Copy link
Copy Markdown
Member

Superseded by #11593, which already merged a fix for #11573. Closing this PR as superseded.

@juliusmarminge

Copy link
Copy Markdown
Member

Superseded by #11593, which already landed the same -webkit-app-region: no-drag dialog fix and closed #11573. Closing this PR as leftover hygiene.

@efemaer
efemaer deleted the codex/fix-theme-modal-close branch September 15, 2026 17:51
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.

[Bug]: Theme import dialog close button ignores clicks over desktop titlebar

2 participants