From 2d6d7c550f04c069e0142e82a3aa5aee52abc350 Mon Sep 17 00:00:00 2001 From: aydnOktay Date: Wed, 19 Aug 2026 23:30:53 -0500 Subject: [PATCH] fix(desktop): keep Radix context menus when asChild overwrites data-slot MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The app-wide context-menu coordinator recognizes surfaces that own a Radix menu by `[data-slot="context-menu-trigger"]`. Radix `asChild` merges as mergeProps(slotProps, childProps), so a child that sets its own `data-slot` wins and the marker never reaches the DOM. The status bar footer is `data-slot="statusbar"`, so the coordinator swallowed its right-click and showed the window-verbs fallback instead — leaving every default-hidden status bar item, the context meter included, unreachable from the UI. Stamp a dedicated `data-hermes-context-menu-trigger` after `{...props}` on ContextMenuTrigger and bail on that marker. Any asChild surface with its own `data-slot` is covered, not just the status bar. --- .../context-menu/app-context-menu.test.tsx | 38 +++++++++++++++++++ .../src/app/context-menu/app-context-menu.tsx | 9 ++++- .../src/components/ui/context-menu.tsx | 13 ++++++- 3 files changed, 58 insertions(+), 2 deletions(-) diff --git a/apps/desktop/src/app/context-menu/app-context-menu.test.tsx b/apps/desktop/src/app/context-menu/app-context-menu.test.tsx index ea406caa10..08494f1463 100644 --- a/apps/desktop/src/app/context-menu/app-context-menu.test.tsx +++ b/apps/desktop/src/app/context-menu/app-context-menu.test.tsx @@ -3,6 +3,11 @@ import { MemoryRouter } from 'react-router' import { afterEach, describe, expect, it, vi } from 'vitest' import { registerTerminalContextMenu } from '@/app/right-sidebar/terminal/terminal-context-menu' +import { + ContextMenu, + ContextMenuTrigger, + HERMES_CONTEXT_MENU_TRIGGER_ATTR +} from '@/components/ui/context-menu' import { formatCombo } from '@/lib/keybinds/combo' import { $previewTabs, closeRightRail } from '@/store/preview' import { $connection } from '@/store/session' @@ -354,6 +359,22 @@ describe('AppContextMenu', () => { expect($contextMenu.get()).toBeNull() }) + it('leaves asChild radix triggers alone when the child overwrites data-slot', () => { + installBridge() + mountMenu() + // Status bar: ContextMenuTrigger asChild over a footer whose data-slot + // is "statusbar". The coordinator must still recognize the dedicated + // marker — otherwise the window-verbs fallback eats the gesture. + const host = attach( + '' + ) + + fireEvent.contextMenu(host.querySelector('span')!) + + expect($contextMenu.get()).toBeNull() + expect(screen.queryByText('Settings')).toBeNull() + }) + it('shows the terminal menu through a registered handle', async () => { installBridge() mountMenu() @@ -567,3 +588,20 @@ describe('AppContextMenu guest (in-app browser)', () => { expect(screen.queryByText('Add to dictionary')).toBeNull() }) }) + +describe('ContextMenuTrigger asChild', () => { + it('keeps the coordinator marker when the child overwrites data-slot', () => { + render( + + +
bar
+
+
+ ) + + const footer = screen.getByText('bar') + + expect(footer.getAttribute('data-slot')).toBe('statusbar') + expect(footer.hasAttribute(HERMES_CONTEXT_MENU_TRIGGER_ATTR)).toBe(true) + }) +}) diff --git a/apps/desktop/src/app/context-menu/app-context-menu.tsx b/apps/desktop/src/app/context-menu/app-context-menu.tsx index 96e478bd06..53966819b1 100644 --- a/apps/desktop/src/app/context-menu/app-context-menu.tsx +++ b/apps/desktop/src/app/context-menu/app-context-menu.tsx @@ -5,6 +5,7 @@ import { useNavigate } from 'react-router' import { terminalMenuHandleFor } from '@/app/right-sidebar/terminal/terminal-context-menu' import { Codicon } from '@/components/ui/codicon' +import { HERMES_CONTEXT_MENU_TRIGGER_ATTR } from '@/components/ui/context-menu' import { writeClipboardText } from '@/components/ui/copy-button' import { DropdownMenu, @@ -609,7 +610,13 @@ export function AppContextMenu() { const element = event.target instanceof Element ? event.target : null // Surfaces with their own Radix context menu keep the whole gesture. - if (element?.closest('[data-slot="context-menu-trigger"]')) { + // Guard the dedicated marker first: Radix `asChild` Slot merges + // `mergeProps(slotProps, childProps)` so the child's `data-slot` wins + // (status bar footer is `data-slot="statusbar"`). The marker is stamped + // after `{...props}` on ContextMenuTrigger and is not overwritten. + if ( + element?.closest(`[${HERMES_CONTEXT_MENU_TRIGGER_ATTR}], [data-slot="context-menu-trigger"]`) + ) { return } diff --git a/apps/desktop/src/components/ui/context-menu.tsx b/apps/desktop/src/components/ui/context-menu.tsx index 286418baf9..34bfb3728d 100644 --- a/apps/desktop/src/components/ui/context-menu.tsx +++ b/apps/desktop/src/components/ui/context-menu.tsx @@ -12,8 +12,19 @@ function ContextMenuPortal({ ...props }: React.ComponentProps } +/** Coordinator marker that survives Radix `asChild` Slot merges. + * `data-slot` is overwritten when the child sets its own `data-slot` + * (status bar footer); this attribute is not. */ +export const HERMES_CONTEXT_MENU_TRIGGER_ATTR = 'data-hermes-context-menu-trigger' + function ContextMenuTrigger({ ...props }: React.ComponentProps) { - return + return ( + + ) } function ContextMenuGroup({ ...props }: React.ComponentProps) {