Skip to content

fix(web): Mod+T opens the tab menu while a pull request tab is open - #19065

Merged
juliusmarminge merged 5 commits into
pingdotgg:mainfrom
riccardopll:riccardo/fix-mod-t-pr-tab
Oct 20, 2026
Merged

juliusmarminge merged 5 commits into
pingdotgg:mainfrom
riccardopll:riccardo/fix-mod-t-pr-tab

Conversation

@riccardopll

@riccardopll riccardopll commented Oct 13, 2026 •

Copy link
Copy Markdown
Contributor

Mod+T did nothing with a pull request tab open because its closed comment composer stays mounted. The panel now treats popups as blocking only while open or closing, matching the existing composer focus selector. Empty-panel letter shortcuts use the same corrected filter.

Credit to @riccardopll for the original fix and evidence in #19065. The production approach remains small; its two added tests were replaced with one state-transition regression in the existing browser suite.

Before — Ctrl+T leaves the tab menu closed while the PR composer is closed:

Before: Ctrl+T leaves the tab menu closed

After — Ctrl+T opens the tab menu with the same PR tab and closed composer:

After: Ctrl+T opens the tab menu

Before interaction · After interaction, including the open-composer guard

Verification:

  • Browser cases reduced from 19 to 18 across the same two files (11 → 10 keyboard cases; 8 panel cases unchanged). The new transition case reproduces the closed-popup failure on main and passes after the fix, covering open, closing and closed states for Mod+T and empty-panel letter shortcuts.
  • All 18 browser cases and 145 keybinding tests passed. Web typecheck, targeted lint, formatting and diff checks passed.
  • Live Linux web: actual Ctrl+T opens the menu with a closed kept-mounted PR composer, remains blocked with that composer open even after blurring its editor, and works again after dismissal. Matching 1280×800 captures use isolated synthetic PR detail/activity query fixtures and the real composer and launcher components. No comment was submitted.
  • GitHub-backed PR loading, macOS Cmd+T, Windows, Electron desktop, native iOS/Android, and remote/relay/tunnel connections were not verified. Desktop shares this renderer path; native mobile is unchanged. The closing transition is covered by the browser regression rather than a live timing claim.

Implementation: Codex gpt-6.1-sol; test design: Codex gpt-6-astra; independent reviews: Claude claude-opus-5-5 and Codex gpt-6-sol. Orchestrated through T3 Code using the Codex harness.

@github-actions github-actions Bot added vouch:unvouched PR author is not yet trusted in the VOUCHED list. size:S 10-29 changed lines (additions + deletions). labels Oct 13, 2026
@macroscopeapp

macroscopeapp Bot commented Oct 13, 2026

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Approved at 956c0f4

Macroscope's review found this PR approvable — This is a small, self-contained keyboard bug fix that distinguishes active popups from closed keep-mounted popups without changing the shortcut set or other product behavior. Focused tests cover both closed and closing overlay states.

Notes:

  • All code in this push has already been reviewed. Approvability was decided on eligibility alone.

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

@coderabbitai

coderabbitai Bot commented Oct 13, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

Warning

Review limit reached

Only developers with an assigned seat can use this organization's usage-based review budget, and seats here are assigned manually. Ask an admin to assign a seat, or change the review continuation mode in Billing.

Next included review available in 7 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used all 10 included reviews currently available.

Learn how review limits work.

Review configuration:

⚙️ Run configuration
  • Configuration used: Path: .coderabbit.config.ts
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: efdcfdde-687b-4647-a32d-a3cac5b8488a
📥 Commits

Reviewing files that changed from the base of the PR and between 956c0f4 and 8ee86b7.

📒 Files selected for processing: 1
  • apps/web/src/components/RightPanelTabs.keyboard.browser.test.tsx

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Path: .coderabbit.config.ts
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 28ad781d-5a47-49bc-b893-bd3ecf18ea79
📥 Commits

Reviewing files that changed from the base of the PR and between 98a6246 and 956c0f4.

📒 Files selected for processing (2)
  • apps/web/src/components/RightPanelTabs.keyboard.test.tsx
  • apps/web/src/components/RightPanelTabs.tsx

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

📝 Walkthrough

The launcher shortcut blocker now matches popup elements only while they are open or closing. Keyboard tests cover an open dialog, a hidden closed popover, and a popover in its closing transition.

Changes: Launcher shortcut popup-state filtering

Layer / File(s) Summary
Filter shortcut blocking by popup state
apps/web/src/components/RightPanelTabs.tsx, apps/web/src/components/RightPanelTabs.keyboard.test.tsx
The popup selectors now match open or closing elements. Tests verify that an open dialog and a closing popover block the shortcut, while a hidden closed popover does not.

Priority: ⬇️ Low

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

Change: Bug fix

Suggested reviewers: bil0000

Merge Risk: ⚪ Minimal · up to 956c0

Open and closing popups continue to block Mod+T, while hidden closed popups do not. No outstanding merge-blocking issue is evident.

Pre-merge checks: Passed 4
Check name Status Explanation
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 is concise, specific, and accurately describes the shortcut fix implemented by the pull request.
Description check Passed The description explains the problem, fix, verification results, UI evidence, test coverage, and unverified environments. It does not include an explicit Scope and approval section or state why the li…
Full details: Description check

Explanation

The description explains the problem, fix, verification results, UI evidence, test coverage, and unverified environments. It does not include an explicit Scope and approval section or state why the linked prior PR satisfies that requirement, but the overall description is mostly complete.

✨ 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.

@juliusmarminge juliusmarminge self-assigned this Oct 19, 2026
@juliusmarminge

Copy link
Copy Markdown
Member

Taking over this one as maintainer for batch pull-request-03. I will preserve the original intent, update it for current main, verify the overlay states and Mod+T interaction, and push follow-up commits here. Thanks @riccardopll for the fix and evidence.

@github-actions github-actions Bot added size:M 30-99 changed lines (additions + deletions). and removed size:S 10-29 changed lines (additions + deletions). labels Oct 20, 2026
@juliusmarminge
juliusmarminge merged commit a8dec0e into pingdotgg:main Oct 20, 2026
25 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:M 30-99 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