fix(web): block type-to-focus behind open dialogs - #8139
Conversation
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review. 📝 WalkthroughWalkthroughChatView now detects open alert-dialog, command-dialog, dialog, and sheet popups during type-to-focus behavior. ChangesChat focus behavior
Estimated code review effort: 1 (Trivial) | ~2 minutes Merge Risk: ⚪ Minimal · up to The change prevents type-to-focus from redirecting printable keys into the composer while supported dialogs are open, while preserving focus behavior for closed keep-mounted sheets. No actionable merge-blocking risk remains beyond normal checks and review. Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 38e9afe514
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
One finding: the new open-dialog selector list misses the mobile sidebar sheet, which overrides the sheet-popup slot, so type-to-focus still types into the composer behind it.
Posted via Macroscope — UI Consistency
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 38e9afe. Configure here.
There was a problem hiding this comment.
Verification
Reviewed against the actual sources, not just the diff. The fix is correct.
Premise confirmed. Nothing in apps/web renders data-slot="dialog" (exact match). All dialog primitives render suffixed slots (dialog-popup, alert-dialog-popup, dialog-viewport, etc.), so the old guard was dead code and printable keys could reach the composer behind any open dialog.
Selectors match the real DOM:
dialog-popup(ui/dialog.tsx:75),alert-dialog-popup(ui/alert-dialog.tsx:68),command-dialog-popup(ui/command.tsx:72),sheet-popup(ui/sheet.tsx:93) — all present.- The mobile sidebar overrides the popup slot with
data-slot="sidebar"+data-mobile="true"(ui/sidebar.tsx:242-254), so it would be missed by thesheet-popupselector. The dedicated fifth selector is needed and correct. The desktop sidebar never rendersdata-mobile="true", so it can't false-positive.
State filtering is correct and necessary. Verified against the installed @base-ui/react 1.5.0 (popupStateMapping.js): popups carry data-open when open, data-closed when closed, data-ending-style while animating out. Two consequences:
RightPanelSheetrenders akeepMountedSheetPopupthat stays in the DOM when closed. Without the:is([data-open],[data-ending-style])filter, that closed sheet would permanently block type-to-focus. The filter handles it — closed keep-mounted popups only carrydata-closed.- During exit transitions the popup has
data-closed+data-ending-style, so the guard survives close animations.data-openis present during enter (data-starting-styleis additive), so entering isn't missed either.
Coverage is complete. Inventoried every *-popup slot in apps/web: interactive layers (dialog, alert-dialog, command-dialog, sheet, menu, select, popover, combobox, autocomplete) are all in the list. tooltip-popup and toast-popup are correctly excluded — passive/hover-only surfaces shouldn't block typing.
Non-blocking suggestions
-
A regression test would have caught the original bug. The defect class here is "selector silently rotted" —
[data-slot="dialog"]matched nothing and no test noticed. A small unit test forshouldTypeToFocusComposerasserting (a)falsewith an opendialog-popupin the document, and (b)truewith a keep-mounted closedsheet-popup(data-closedonly) would pin both the fix and the subtle state-filter contract. The PR only re-runs existing suites. -
LAUNCHER_SHORTCUT_BLOCKING_LAYERS(RightPanelTabs.tsx:107) now drifts further. That parallel list has nosheet-popup/mobile-sidebar entries and no state filtering, so launcher letter shortcuts can still fire while a sheet is open. Pre-existing and out of scope here, but consider extracting one shared constant so the two lists can't diverge independently again.
Reviewed with GLM 5.3 via pi.
ApprovabilityVerdict: Approved at Macroscope's review found this PR approvable — This is a small, self-contained UI bug fix that updates the chat keyboard guard to recognize active dialog, sheet, and mobile-sidebar overlays. Its runtime impact is limited to preventing unintended composer focus while those overlays are open or closing. You can add or adjust custom eligibility rules. Learn more. |
|
@codex re-review |
|
Codex Review: Didn't find any major issues. 🎉 Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
## What's Changed * fix(web): align un-settle banner action by @StiensWout in pingdotgg/t3code#9033 * fix(web): block type-to-focus behind open dialogs by @Lucenx9 in pingdotgg/t3code#8139 * feat(shortcuts): copy active thread reference by @maria-rcks in pingdotgg/t3code#8994 **Full Changelog**: pingdotgg/t3code@v0.0.38-nightly.20260901.1246...v0.0.38-nightly.20260901.1248 Upstream release: https://github.com/pingdotgg/t3code/releases/tag/v0.0.38-nightly.20260901.1248
## What's Changed * Add mobile composer attachment menu with video support by @juliusmarminge in pingdotgg/t3code#8843 * fix(web): restore unified activity logs and composer banners by @t3dotgg in pingdotgg/t3code#8734 * fix(web): address composer banner review follow-ups by @juliusmarminge in pingdotgg/t3code#8850 * fix(web): widen sync banners and simplify the working timer by @juliusmarminge in pingdotgg/t3code#8855 * fix(preview): improve browser recording quality by @maria-rcks in pingdotgg/t3code#8839 * fix(web): mark pull request links as external by @juliusmarminge in pingdotgg/t3code#8856 * fix(mobile): replace Callstack glass with Expo glass by @juliusmarminge in pingdotgg/t3code#8862 * fix(server): skip IDE detection in Claude probes by @yashranaway in pingdotgg/t3code#8634 * chore(macroscope): review diagnostic overrides by @t3-code[bot] in pingdotgg/t3code#8917 * fix(contracts): accept CLI event origins by @nateEc in pingdotgg/t3code#8905 * fix(web): hide invalid slash skill completions by @nateEc in pingdotgg/t3code#8904 * fix(mobile): defer draft navigation until submission completes by @juliusmarminge in pingdotgg/t3code#8914 * chore: disable CodeRabbit review status by @juliusmarminge in pingdotgg/t3code#8933 * Delete app.json by @juliusmarminge in pingdotgg/t3code#8934 * fix(web): show scrollbar for wide markdown tables by @UtkarshUsername in pingdotgg/t3code#8868 * fix(mobile): shimmer active tool rows by @juliusmarminge in pingdotgg/t3code#8932 * chore(deps): bump Electron to 43.4.1 by @StiensWout in pingdotgg/t3code#8626 * fix(chat): smooth worktree setup status by @juliusmarminge in pingdotgg/t3code#8922 * feat(mobile): add video playback with native iOS controls by @juliusmarminge in pingdotgg/t3code#8919 * fix(web): prevent chat metadata overlap by @MatthewFeroz in pingdotgg/t3code#8851 * fix(server): preserve usage cache outside walked roots by @Lucenx9 in pingdotgg/t3code#8540 * feat(mobile): add native image and PDF previews by @juliusmarminge in pingdotgg/t3code#8959 * fix(server): allow long thread IDs in HTTP routes by @nateEc in pingdotgg/t3code#8898 * fix(shared): preserve Windows shell PATH priority by @UtkarshUsername in pingdotgg/t3code#8748 * fix(web): make WSL settings searchable by @UtkarshUsername in pingdotgg/t3code#8881 * feat(web): add expand/collapse all control to the files surface by @UtkarshUsername in pingdotgg/t3code#8889 * feat(mobile): upload attachments while composing by @juliusmarminge in pingdotgg/t3code#8978 * fix(chat): keep agent activity visible between actions by @maria-rcks in pingdotgg/t3code#8984 * fix(server): isolate remote web session cookies by @Bil0000 in pingdotgg/t3code#8085 * feat(pull-requests): link GitHub references in markdown by @maria-rcks in pingdotgg/t3code#8812 * perf(server): reduce frequency of full tool call output being loaded into memory from db by @t3dotgg in pingdotgg/t3code#8988 * feat(web): add pull request list filters by @maria-rcks in pingdotgg/t3code#8809 * feat(web): search individual settings by detail by @maria-rcks in pingdotgg/t3code#8831 * feat(client): render viewed images in work logs by @maria-rcks in pingdotgg/t3code#8936 * fix(client): use package import for markdown image helpers by @juliusmarminge in pingdotgg/t3code#9010 * test: remove static presentation snapshots by @t3-code[bot] in pingdotgg/t3code#9008 * perf(server): bound snapshot activity payload memory by @t3dotgg in pingdotgg/t3code#9000 * perf(server): cut idle CPU use and stop provider event leaks by @t3dotgg in pingdotgg/t3code#8187 * perf(server): scan only appended transcript bytes for usage summaries by @StiensWout in pingdotgg/t3code#9024 * perf(server): cut chatty tool-update frames by 90% by @Adamulek123 in pingdotgg/t3code#8368 * fix(server): settle threads server-side by @t3dotgg in pingdotgg/t3code#8600 * fix(clients): dedupe skills in composer menus by @Adamulek123 in pingdotgg/t3code#8043 * fix(server): stop OpenCode child sessions by @t3dotgg in pingdotgg/t3code#9005 * perf(web): defer pull request line stats until visible by @Adamulek123 in pingdotgg/t3code#6471 * perf(server): skip full-message reads while streaming by @t3dotgg in pingdotgg/t3code#9032 * perf(client-runtime): halve server config bootstrap traffic by @Adamulek123 in pingdotgg/t3code#8367 * fix(web): align un-settle banner action by @StiensWout in pingdotgg/t3code#9033 * fix(web): block type-to-focus behind open dialogs by @Lucenx9 in pingdotgg/t3code#8139 * feat(shortcuts): copy active thread reference by @maria-rcks in pingdotgg/t3code#8994 * fix(mobile): keep thread scroll bounds current after animations by @juliusmarminge in pingdotgg/t3code#9013 * fix(server): cache project favicon resolution by @willsheldon in pingdotgg/t3code#9080 * feat(claude): add Claude Fable 5.1 model by @q1 in pingdotgg/t3code#9078 * fix(preview): restore recording and macOS rendering after Electron 43 by @StiensWout in pingdotgg/t3code#9001 * feat(desktop): add configurable quit shortcut confirmation by @juliusmarminge in pingdotgg/t3code#9076 * feat(web): open project settings from thread menus by @SunkenInTime in pingdotgg/t3code#8925 * fix(chat): reuse one row for live activity by @maria-rcks in pingdotgg/t3code#9062 * feat(models): discover Claude models from remote manifest by @juliusmarminge in pingdotgg/t3code#9084 * Revert "fix(chat): reuse one row for live activity" by @maria-rcks in pingdotgg/t3code#9096 * fix(web): sync sidebar PR state from open panel by @t3-code[bot] in pingdotgg/t3code#9092 * fix(web): changing projects no longer creates a draft by @extoci in pingdotgg/t3code#9097 * fix(web): keep theme placeholder text dimmer than entered text by @flamboh in pingdotgg/t3code#9104 * fix(web): keep the selected environment when changing projects by @t3dotgg in pingdotgg/t3code#9102 ## New Contributors * @willsheldon made their first contribution in pingdotgg/t3code#9080 * @q1 made their first contribution in pingdotgg/t3code#9078 **Full Changelog**: pingdotgg/t3code@v0.0.37...v0.0.38 Upstream release: https://github.com/pingdotgg/t3code/releases/tag/v0.0.38

Problem
The chat type-to-focus guard checks for
[data-slot="dialog"], but the app's dialog primitives render popup slots such asdialog-popupandalert-dialog-popup. A printable key can therefore be redirected into the composer behind an open dialog.Fix
Match the actual dialog, alert-dialog, command-dialog, and sheet popup slots, including the mobile sidebar's overridden sheet slot. The selectors cover Base UI's open and ending-style states, preserving the guard through exit transitions without matching fully closed keep-mounted popups.
No visual styling changes.
Verification
vp test run apps/web/src/components/RightPanelTabs.test.tsx apps/web/src/components/ChatView.logic.test.ts(65 passed)pnpm typecheckinapps/webvp lint apps/web/src/components/ChatView.tsxreact-doctor --verbose --scope changed --base upstream/main(no issues)Built with GPT-5.6 Sol via pi.
Note
Low Risk
Narrow, chat keyboard-guard change with no auth or data impact; risk is mainly if something still relied on the old generic
dialogslot match.Overview
Fixes a bug where printable keys could still focus the chat composer while a modal was open, because the guard looked for
[data-slot="dialog"]instead of the real popup elements.TYPE_TO_FOCUS_FLOATING_LAYER_SELECTORinChatView.tsxnow targets open floating layers:dialog-popup,alert-dialog-popup,command-dialog-popup,sheet-popup, and the mobile sidebar sheet. Selectors require Base UIdata-openordata-ending-styleso keep-mounted closed sheets do not block type-to-focus. Existing menu, select, popover, combobox, and autocomplete popup selectors are unchanged.Reviewed by Cursor Bugbot for commit 7c7ed48. Bugbot is set up for automated code reviews on this repo. Configure here.
Note
Block type-to-focus behind open dialogs in
ChatViewChatView.TYPE_TO_FOCUS_FLOATING_LAYER_SELECTORin ChatView.tsx with selectors foralert-dialog,command-dialog,dialog,sheet, and the mobile sidebar.openorendingstate, replacing the generic dialog slot selector.Macroscope summarized 7c7ed48.