fix(web): make terminal context-menu actions reliable - #8459
Adamulek123 wants to merge 4 commits into
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:
Comment |
github-actions
Bot
added
vouch:unvouched
![macroscopeapp[bot]](https://avatars.githubusercontent.com/in/900172?s=60&v=4){kind=small}
| if (event.isComposing) return null; | ||
|
|
||
| for (const item of items) { | ||
| if (item.disabled) continue; |
There was a problem hiding this comment.
🟡 Medium src/contextMenuFallback.ts:207
A header: true item with an accelerator still returns its id from contextMenuAcceleratorAction, so pressing the shortcut executes an action that is non-interactive in the fallback and absent from the Electron menu. Exclude header items from accelerator matching.
| if (item.disabled) continue; | |
| if (item.disabled || item.header) continue; |
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/web/src/contextMenuFallback.ts around line 207:
A `header: true` item with an `accelerator` still returns its `id` from `contextMenuAcceleratorAction`, so pressing the shortcut executes an action that is non-interactive in the fallback and absent from the Electron menu. Exclude header items from accelerator matching.
There was a problem hiding this comment.
🟡 Medium
t3code/apps/web/src/contextMenuFallback.ts
Line 537 in 6f9f05a
Nested compact submenus are moved back to the right after openSubmenu places them on the left, so near the viewport edge they overlap their parent. The requestAnimationFrame callback in openMenu reclamps using the original right-side preferredLeft; clamp the menu's current position instead.
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/web/src/contextMenuFallback.ts around line 537:
Nested compact submenus are moved back to the right after `openSubmenu` places them on the left, so near the viewport edge they overlap their parent. The `requestAnimationFrame` callback in `openMenu` reclamps using the original right-side `preferredLeft`; clamp the menu's current position instead.
There was a problem hiding this comment.
Reviewed the changed web UI surfaces (contextMenuFallback.ts, localApi.ts, ThreadTerminalDrawer.tsx) against the shared menu primitives. The imperative fallback's default rows now correctly let the Tailwind classes own height/typography (the removed inline min-height/font-size were shadowing min-h-8/text-base), and the accelerator kbd mirrors MenuShortcut exactly, so styling ownership there looks right. Two items below: one type-scale divergence from the shared menu contract, and one dropped Electron/browser constraint comment.
Posted via Macroscope — UI Consistency
| const rowBase = usesCompactLayout | ||
| ? "flex min-h-8 w-full cursor-default select-none items-center gap-2 rounded-sm px-2 py-1 text-left text-sm outline-none transition-colors sm:min-h-7 sm:text-xs" | ||
| : "flex min-h-8 w-full cursor-default select-none items-center gap-2 rounded-sm px-2 py-1 text-left text-base outline-none transition-colors sm:min-h-7 sm:text-sm"; |
There was a problem hiding this comment.
The compact layout changes the menu row type scale (text-sm / sm:text-xs), not just the width. Every other context menu in the app — the React MenuItem primitive and this file's default layout — renders rows at text-base sm:text-sm, so the terminal menu becomes the only 12px context menu on desktop, and at sm its label ends up the same size as the text-xs accelerator kbd (which is styled to be the secondary element, matching MenuShortcut), collapsing the label/shortcut hierarchy.
Since the compact layout already widens the popup to 14rem, it does not need smaller text; consider letting compact own only min-width and keeping the shared row scale.
| const rowBase = usesCompactLayout | |
| ? "flex min-h-8 w-full cursor-default select-none items-center gap-2 rounded-sm px-2 py-1 text-left text-sm outline-none transition-colors sm:min-h-7 sm:text-xs" | |
| : "flex min-h-8 w-full cursor-default select-none items-center gap-2 rounded-sm px-2 py-1 text-left text-base outline-none transition-colors sm:min-h-7 sm:text-sm"; | |
| const rowBase = | |
| "flex min-h-8 w-full cursor-default select-none items-center gap-2 rounded-sm px-2 py-1 text-left text-base outline-none transition-colors sm:min-h-7 sm:text-sm"; |
Posted via Macroscope — UI Consistency
| export function terminalContextMenuItems( | ||
| availability: { readonly canAddToChat: boolean; readonly canCopy: boolean }, | ||
| platform = typeof navigator === "undefined" ? "" : navigator.platform, | ||
| ): ContextMenuItem<TerminalContextMenuAction>[] { |
There was a problem hiding this comment.
The JSDoc that explained the Electron/browser constraint behind this menu was dropped with the rename: it documented why Paste is always offered (the browser and Electron's default editing menu can only paste into an editable element, so a canvas terminal never gets a usable entry from them). That rationale is still the reason paste is unconditionally present here, and it now also justifies presentation: "styled" at the call sites. Consider restoring it.
| export function terminalContextMenuItems( | |
| availability: { readonly canAddToChat: boolean; readonly canCopy: boolean }, | |
| platform = typeof navigator === "undefined" ? "" : navigator.platform, | |
| ): ContextMenuItem<TerminalContextMenuAction>[] { | |
| /** | |
| * Terminal canvas menu: the selection actions (disabled until a selection | |
| * exists) plus Paste. Paste is always offered because the browser (and | |
| * Electron's default editing menu) can only paste into an editable element, | |
| * so a canvas terminal never gets a usable entry from them. | |
| */ | |
| export function terminalContextMenuItems( | |
| availability: { readonly canAddToChat: boolean; readonly canCopy: boolean }, | |
| platform = typeof navigator === "undefined" ? "" : navigator.platform, | |
| ): ContextMenuItem<TerminalContextMenuAction>[] { |
Posted via Macroscope — UI Consistency
1 participant
What Changed
Unified selection-popup and right-click actions for Copy, Paste, and Add to chat. Terminal-owned menu requests now reject safely, ignore stale actions, cancel on teardown, and restore focus only for Copy and Paste.
Why
The old paths could deliver duplicate or stale actions, leave a styled menu open after terminal teardown, and report an unhandled rejection when the automatic popup failed to open.
Stack dependency
This branch is stacked on #8458. Review the focused two-file diff or the final two commits,
e208071fand6f9f05ab. GitHub must temporarily compare this fork branch againstmainbecause fork branches cannot be selected as an upstream PR base. Rebase ontomainand mark ready after #8458 merges.UI Changes
This changes terminal-menu interaction and focus behavior, but not its final visual design. The PR remains a draft until its dependency lands and integrated browser/Electron evidence can be recorded.
Validation
vp run --filter @t3tools/web typecheckgit diff --checkagainst feat(web): add explicit context-menu layout and focus options #8458Checklist
Implemented with GPT-5.6 Codex in T3 Code.
Note
Fix terminal context-menu actions with accelerators and abort lifecycle
acceleratortoContextMenuItemand extendsLocalApi.contextMenu.showwithlayout,presentation,restoreFocus, andsignaloptions in ipc.ts.acceleratorthrough Electron menu normalization and template building in ElectronMenu.ts.TerminalViewportin ThreadTerminalDrawer.tsx to use anAbortController-owned menu lifecycle, platform-specific accelerators (mac vs non-mac), and focus restore only for copy/paste.localApinow allows a styled web menu on desktop and always dismisses the fallback onclose()in localApi.ts.restoreFocus: 'on-dismiss'; stale or aborted requests skip side effects and focus restoration. ReviewrunTerminalMenuRequestandshowContextMenuFallbackto confirm abort and dismissal paths.📊 Macroscope summarized 6f9f05a. 5 files reviewed, 2 issues evaluated, 0 issues filtered, 2 comments posted
🗂️ Filtered Issues