diff --git a/app/src/components/commands/CommandPalette.tsx b/app/src/components/commands/CommandPalette.tsx index 8bb4da6b1..9024a8476 100644 --- a/app/src/components/commands/CommandPalette.tsx +++ b/app/src/components/commands/CommandPalette.tsx @@ -2,6 +2,7 @@ import * as Dialog from '@radix-ui/react-dialog'; import { Command } from 'cmdk'; import { useMemo, useSyncExternalStore } from 'react'; +import { GROUP_LABEL_KEYS } from '../../lib/commands/globalActions'; import { hotkeyManager } from '../../lib/commands/hotkeyManager'; import { registry } from '../../lib/commands/registry'; import type { RegisteredAction } from '../../lib/commands/types'; @@ -81,34 +82,40 @@ export default function CommandPalette({ open, onOpenChange }: Props) { {groups.map(([groupName, items]) => ( - {items.map(action => ( - runAction(action)} - className="flex items-center gap-3 px-4 py-2 cursor-pointer aria-selected:bg-cmd-surface-elevated"> - {action.icon ? ( - - ) : ( - - )} - {action.label} - {action.hint && ( - - {action.hint} - - )} - {action.shortcut && } - - ))} + {items.map(action => { + const label = action.labelKey ? t(action.labelKey) : action.label; + return ( + runAction(action)} + className="flex items-center gap-3 px-4 py-2 cursor-pointer aria-selected:bg-cmd-surface-elevated"> + {action.icon ? ( + + ) : ( + + )} + {label} + {action.hint && ( + + {action.hint} + + )} + {action.shortcut && } + + ); + })} ))} -
- {t('commandPalette.shortcutHint')} +
+ {/* Advertise ⌘/ (allowed while this search box is focused) rather + than ?, which the hotkey manager skips inside inputs. */} + + {t('shortcuts.title')}
diff --git a/app/src/components/commands/CommandProvider.tsx b/app/src/components/commands/CommandProvider.tsx index 19a78da7f..cc7eec229 100644 --- a/app/src/components/commands/CommandProvider.tsx +++ b/app/src/components/commands/CommandProvider.tsx @@ -1,10 +1,15 @@ -import { type ReactNode, useEffect, useMemo, useState } from 'react'; +import { type ReactNode, useEffect, useMemo, useRef, useState } from 'react'; import { useNavigate } from 'react-router-dom'; -import { registerGlobalActions } from '../../lib/commands/globalActions'; +import { type GlobalActionHandlers, registerGlobalActions } from '../../lib/commands/globalActions'; import { hotkeyManager } from '../../lib/commands/hotkeyManager'; import { registry } from '../../lib/commands/registry'; import { ScopeContext } from '../../lib/commands/ScopeContext'; +import { useAppDispatch } from '../../store/hooks'; +import { toggleSidebar } from '../../store/layoutSlice'; +import { APP_SHELL_LAYOUT_ID } from '../layout/shell/RootShellLayout'; +import { useNewChat } from '../layout/shell/useNewChat'; +import KeyboardShortcutsModal from '../shortcuts/KeyboardShortcutsModal'; import CommandPalette from './CommandPalette'; let instanceCount = 0; @@ -15,9 +20,32 @@ interface Props { export default function CommandProvider({ children }: Props) { const navigate = useNavigate(); + const dispatch = useAppDispatch(); + const newChat = useNewChat(); const [paletteOpen, setPaletteOpen] = useState(false); + const [shortcutsOpen, setShortcutsOpen] = useState(false); const [globalFrame, setGlobalFrame] = useState(null); + // Latest handlers held in a ref so the registration effect runs exactly once + // (on mount) — `useHomeNav`'s callback changes as the route/threads change, + // and re-running the effect would tear down and rebuild the whole hotkey + // frame on every navigation. + const handlersRef = useRef(null as unknown as GlobalActionHandlers); + handlersRef.current = { + navigate: path => navigate(path), + newChat, + toggleSidebar: () => dispatch(toggleSidebar({ id: APP_SHELL_LAYOUT_ID })), + // The two overlays are mutually exclusive: opening one dismisses the other. + openPalette: () => { + setShortcutsOpen(false); + setPaletteOpen(o => !o); + }, + openShortcuts: () => { + setPaletteOpen(false); + setShortcutsOpen(o => !o); + }, + }; + useEffect(() => { instanceCount += 1; if (instanceCount > 1) { @@ -26,25 +54,27 @@ export default function CommandProvider({ children }: Props) { hotkeyManager.init(); const frame = hotkeyManager.pushFrame('global', 'root'); registry.setActiveStack(hotkeyManager.getStackSymbols()); - const disposeGlobalActions = registerGlobalActions(navigate, frame); - const paletteBinding = hotkeyManager.bind(frame, { - shortcut: 'mod+k', - handler: () => { - setPaletteOpen(o => !o); - }, - allowInInput: true, - id: 'meta.open-palette', - }); + + // Stable indirection: registered handlers always call through to the latest + // ref value, so closures captured at registration time stay current. + const stableHandlers: GlobalActionHandlers = { + navigate: path => handlersRef.current.navigate(path), + newChat: () => handlersRef.current.newChat(), + toggleSidebar: () => handlersRef.current.toggleSidebar(), + openPalette: () => handlersRef.current.openPalette(), + openShortcuts: () => handlersRef.current.openShortcuts(), + }; + const disposeGlobalActions = registerGlobalActions(stableHandlers, frame); setGlobalFrame(frame); return () => { disposeGlobalActions(); - hotkeyManager.unbind(frame, paletteBinding); hotkeyManager.popFrame(frame); registry.setActiveStack(hotkeyManager.getStackSymbols()); instanceCount -= 1; }; - }, [navigate]); + // Mount-once: handlers are reached via `handlersRef`, not deps. + }, []); useEffect(() => { if (!globalFrame) return; @@ -61,6 +91,7 @@ export default function CommandProvider({ children }: Props) { {children} + ); } diff --git a/app/src/components/commands/__tests__/CommandPalette.test.tsx b/app/src/components/commands/__tests__/CommandPalette.test.tsx index 27924bdda..8cb5b8f06 100644 --- a/app/src/components/commands/__tests__/CommandPalette.test.tsx +++ b/app/src/components/commands/__tests__/CommandPalette.test.tsx @@ -73,8 +73,10 @@ describe('CommandPalette', () => { expect(onOpenChange).toHaveBeenCalledWith(false); }); - it('renders footer hint', () => { + it('renders footer hint advertising the in-palette shortcuts key', () => { render( {}} />); - expect(screen.getByText(/Press \? for all shortcuts/i)).toBeInTheDocument(); + // Advertises ⌘/ (works while the search box is focused), not ? (skipped in + // inputs). The label reuses the shared "Keyboard Shortcuts" string. + expect(screen.getByText('Keyboard Shortcuts')).toBeInTheDocument(); }); }); diff --git a/app/src/components/commands/__tests__/CommandProvider.test.tsx b/app/src/components/commands/__tests__/CommandProvider.test.tsx index 3c52923b2..4da142523 100644 --- a/app/src/components/commands/__tests__/CommandProvider.test.tsx +++ b/app/src/components/commands/__tests__/CommandProvider.test.tsx @@ -1,65 +1,57 @@ -import { act, render, screen } from '@testing-library/react'; +import { act, screen } from '@testing-library/react'; import userEvent from '@testing-library/user-event'; -import { MemoryRouter } from 'react-router-dom'; import { beforeEach, describe, expect, it } from 'vitest'; import { hotkeyManager } from '../../../lib/commands/hotkeyManager'; import { pressKey } from '../../../test/commandTestUtils'; +import { renderWithProviders } from '../../../test/test-utils'; import CommandProvider from '../CommandProvider'; beforeEach(() => { hotkeyManager.teardown(); }); +function renderProvider() { + return renderWithProviders( + +
child
+
+ ); +} + describe('CommandProvider', () => { it('mounts and registers seed actions', () => { - render( - - -
child
-
-
- ); + renderProvider(); expect(screen.getByText('child')).toBeInTheDocument(); }); it('opens palette on mod+K', async () => { - render( - - -
child
-
-
- ); + renderProvider(); act(() => { pressKey({ key: 'k', mod: true }); }); expect(await screen.findByRole('dialog', { name: /Command palette/i })).toBeInTheDocument(); }); - it.skip('opens help on ? (disabled — help overlay temporarily off)', async () => { - render( - - -
child
-
-
- ); + it('opens help on ?', async () => { + renderProvider(); act(() => { pressKey({ key: '?' }); }); expect(await screen.findByRole('dialog', { name: /Keyboard shortcuts/i })).toBeInTheDocument(); }); + it('opens help on mod+/', async () => { + renderProvider(); + act(() => { + pressKey({ key: '/', mod: true }); + }); + expect(await screen.findByRole('dialog', { name: /Keyboard shortcuts/i })).toBeInTheDocument(); + }); + it('Esc closes open overlay', async () => { const user = userEvent.setup(); - render( - - -
child
-
-
- ); + renderProvider(); act(() => { pressKey({ key: 'k', mod: true }); }); @@ -68,20 +60,15 @@ describe('CommandProvider', () => { expect(screen.queryByRole('dialog')).not.toBeInTheDocument(); }); - it.skip('palette and help mutually exclusive (disabled — help overlay temporarily off)', async () => { - render( - - -
child
-
-
- ); + it('palette and help are mutually exclusive', async () => { + renderProvider(); act(() => { pressKey({ key: 'k', mod: true }); }); expect(await screen.findByRole('dialog', { name: /Command palette/i })).toBeInTheDocument(); + // Opening the shortcuts directory dismisses the palette. act(() => { - pressKey({ key: '?' }); + pressKey({ key: '/', mod: true }); }); expect(await screen.findByRole('dialog', { name: /Keyboard shortcuts/i })).toBeInTheDocument(); expect(screen.queryByRole('dialog', { name: /Command palette/i })).not.toBeInTheDocument(); diff --git a/app/src/components/layout/shell/CollapsedNavRail.test.tsx b/app/src/components/layout/shell/CollapsedNavRail.test.tsx index 9b0508992..f9281b4c0 100644 --- a/app/src/components/layout/shell/CollapsedNavRail.test.tsx +++ b/app/src/components/layout/shell/CollapsedNavRail.test.tsx @@ -1,6 +1,7 @@ import { fireEvent, screen } from '@testing-library/react'; import { beforeEach, describe, expect, it, vi } from 'vitest'; +import { registry } from '../../../lib/commands/registry'; import { renderWithProviders } from '../../../test/test-utils'; import CollapsedNavRail from './CollapsedNavRail'; @@ -19,11 +20,11 @@ vi.mock('../../../services/analytics', () => ({ trackEvent: vi.fn() })); describe('CollapsedNavRail', () => { beforeEach(() => vi.clearAllMocks()); - it('renders Home, Wallet, and every primary nav destination as icon buttons', () => { + it('renders Home, Keyboard Shortcuts, and every primary nav destination as icon buttons', () => { renderWithProviders(, { initialEntries: ['/home'] }); for (const key of [ 'nav.home', - 'nav.wallet', + 'shortcuts.title', 'nav.chat', 'nav.human', 'nav.brain', @@ -34,30 +35,22 @@ describe('CollapsedNavRail', () => { } }); - it('wallet button navigates to /settings/wallet-balances', () => { + it('shortcuts button opens the keyboard-shortcuts help directory', () => { + const runAction = vi.spyOn(registry, 'runAction').mockReturnValue(true); renderWithProviders(, { initialEntries: ['/home'] }); - fireEvent.click(screen.getByRole('button', { name: 'nav.wallet' })); - // Carries the backgroundLocation so the desktop Settings modal renders over - // the page it was opened from. - expect(mockNavigate).toHaveBeenCalledWith('/settings/wallet-balances', { - state: { backgroundLocation: expect.objectContaining({ pathname: '/home' }) }, - }); + fireEvent.click(screen.getByRole('button', { name: 'shortcuts.title' })); + expect(runAction).toHaveBeenCalledWith('meta.keyboard-shortcuts'); + runAction.mockRestore(); }); - it('wallet button has correct data-analytics-id', () => { + it('shortcuts button has correct data-analytics-id', () => { renderWithProviders(, { initialEntries: ['/home'] }); - expect(screen.getByRole('button', { name: 'nav.wallet' })).toHaveAttribute( + expect(screen.getByRole('button', { name: 'shortcuts.title' })).toHaveAttribute( 'data-analytics-id', - 'collapsed-rail-wallet' + 'collapsed-rail-shortcuts' ); }); - it('wallet button is marked active when on /settings/wallet-balances', () => { - renderWithProviders(, { initialEntries: ['/settings/wallet-balances'] }); - const btn = screen.getByRole('button', { name: 'nav.wallet' }); - expect(btn.className).toMatch(/bg-white|dark:bg-neutral-800/); - }); - it('navigates to a destination path when its icon is clicked', () => { renderWithProviders(, { initialEntries: ['/home'] }); fireEvent.click(screen.getByRole('button', { name: 'nav.brain' })); @@ -104,12 +97,9 @@ describe('CollapsedNavRail', () => { ); }); - it('defers to Wallet on the wallet sub-page — only one icon stays active', () => { + it('marks Settings active on the wallet sub-page (no separate wallet rail item)', () => { renderWithProviders(, { initialEntries: ['/settings/wallet-balances'] }); - expect(screen.getByRole('button', { name: 'nav.settings' })).not.toHaveAttribute( - 'aria-current' - ); - expect(screen.getByRole('button', { name: 'nav.wallet' })).toHaveAttribute( + expect(screen.getByRole('button', { name: 'nav.settings' })).toHaveAttribute( 'aria-current', 'page' ); diff --git a/app/src/components/layout/shell/CollapsedNavRail.tsx b/app/src/components/layout/shell/CollapsedNavRail.tsx index 4caa8dd22..ef74a57fc 100644 --- a/app/src/components/layout/shell/CollapsedNavRail.tsx +++ b/app/src/components/layout/shell/CollapsedNavRail.tsx @@ -2,11 +2,11 @@ import { useMemo } from 'react'; import { useLocation, useNavigate } from 'react-router-dom'; import { NAV_TABS, type NavTab } from '../../../config/navConfig'; +import { registry } from '../../../lib/commands/registry'; import { useT } from '../../../lib/i18n/I18nContext'; import { trackEvent } from '../../../services/analytics'; import { useAppSelector } from '../../../store/hooks'; import { selectUnreadCount } from '../../../store/notificationSlice'; -import { settingsNavState } from '../../settings/modal/settingsOverlay'; import { Tooltip } from '../../ui'; import { NavIcon } from './navIcons'; import { useHomeNav } from './useHomeNav'; @@ -51,11 +51,7 @@ export default function CollapsedNavRail() { }; const homeActive = location.pathname === '/chat' || location.pathname.startsWith('/chat/'); - // Settings defers to the more-specific Wallet rail item so the wallet sub-page - // doesn't light up both icons at once. - const settingsActive = - matchActive('/settings', location.pathname) && - !matchActive('/settings/wallet-balances', location.pathname); + const settingsActive = matchActive('/settings', location.pathname); return (