fix(shortcut): fixed global keyboard commands provider to follow latest ref pattern (#2569)

* fix(shortcut): fixed global commands provider to follow best practices

* cleanup

* ack PR comment
This commit is contained in:
Waleed
2025-12-24 00:25:15 -08:00
committed by GitHub
parent 3a50ce4d99
commit 1145f5c043
3 changed files with 23 additions and 74 deletions

View File

@@ -14,11 +14,6 @@ import { createLogger } from '@/lib/logs/console/logger'
const logger = createLogger('GlobalCommands') const logger = createLogger('GlobalCommands')
/**
* Detects if the current platform is macOS.
*
* @returns True if running on macOS, false otherwise
*/
function isMacPlatform(): boolean { function isMacPlatform(): boolean {
if (typeof window === 'undefined') return false if (typeof window === 'undefined') return false
return ( return (
@@ -27,18 +22,6 @@ function isMacPlatform(): boolean {
) )
} }
/**
* Represents a parsed keyboard shortcut.
*
* We support the following modifiers:
* - Mod: maps to Meta on macOS, Ctrl on other platforms
* - Ctrl, Meta, Shift, Alt
*
* Examples:
* - "Mod+A"
* - "Mod+Shift+T"
* - "Meta+K"
*/
export interface ParsedShortcut { export interface ParsedShortcut {
key: string key: string
mod?: boolean mod?: boolean
@@ -48,24 +31,10 @@ export interface ParsedShortcut {
alt?: boolean alt?: boolean
} }
/**
* Declarative command registration.
*/
export interface GlobalCommand { export interface GlobalCommand {
/** Unique id for the command. If omitted, one is generated. */
id?: string id?: string
/** Shortcut string in the form "Mod+Shift+T", "Mod+A", "Meta+K", etc. */
shortcut: string shortcut: string
/**
* Whether to allow the command to run inside editable elements like inputs,
* textareas or contenteditable. Defaults to true to ensure browser defaults
* are overridden when desired.
*/
allowInEditable?: boolean allowInEditable?: boolean
/**
* Handler invoked when the shortcut is matched. Use this to trigger actions
* like navigation or dispatching application events.
*/
handler: (event: KeyboardEvent) => void handler: (event: KeyboardEvent) => void
} }
@@ -80,16 +49,13 @@ interface GlobalCommandsContextValue {
const GlobalCommandsContext = createContext<GlobalCommandsContextValue | null>(null) const GlobalCommandsContext = createContext<GlobalCommandsContextValue | null>(null)
/**
* Parses a human-readable shortcut into a structured representation.
*/
function parseShortcut(shortcut: string): ParsedShortcut { function parseShortcut(shortcut: string): ParsedShortcut {
const parts = shortcut.split('+').map((p) => p.trim()) const parts = shortcut.split('+').map((p) => p.trim())
const modifiers = new Set(parts.slice(0, -1).map((p) => p.toLowerCase())) const modifiers = new Set(parts.slice(0, -1).map((p) => p.toLowerCase()))
const last = parts[parts.length - 1] const last = parts[parts.length - 1]
return { return {
key: last.length === 1 ? last.toLowerCase() : last, // keep non-letter keys verbatim key: last.length === 1 ? last.toLowerCase() : last,
mod: modifiers.has('mod'), mod: modifiers.has('mod'),
ctrl: modifiers.has('ctrl'), ctrl: modifiers.has('ctrl'),
meta: modifiers.has('meta') || modifiers.has('cmd') || modifiers.has('command'), meta: modifiers.has('meta') || modifiers.has('cmd') || modifiers.has('command'),
@@ -98,16 +64,10 @@ function parseShortcut(shortcut: string): ParsedShortcut {
} }
} }
/**
* Checks if a KeyboardEvent matches a parsed shortcut, honoring platform-specific
* interpretation of "Mod" (Meta on macOS, Ctrl elsewhere).
*/
function matchesShortcut(e: KeyboardEvent, parsed: ParsedShortcut): boolean { function matchesShortcut(e: KeyboardEvent, parsed: ParsedShortcut): boolean {
const isMac = isMacPlatform() const isMac = isMacPlatform()
const expectedCtrl = parsed.ctrl || (parsed.mod ? !isMac : false) const expectedCtrl = parsed.ctrl || (parsed.mod ? !isMac : false)
const expectedMeta = parsed.meta || (parsed.mod ? isMac : false) const expectedMeta = parsed.meta || (parsed.mod ? isMac : false)
// Normalize key for comparison: for letters compare lowercase
const eventKey = e.key.length === 1 ? e.key.toLowerCase() : e.key const eventKey = e.key.length === 1 ? e.key.toLowerCase() : e.key
return ( return (
@@ -119,10 +79,6 @@ function matchesShortcut(e: KeyboardEvent, parsed: ParsedShortcut): boolean {
) )
} }
/**
* Provider that captures global keyboard shortcuts and routes them to
* registered commands. Commands can be registered from any descendant component.
*/
export function GlobalCommandsProvider({ children }: { children: ReactNode }) { export function GlobalCommandsProvider({ children }: { children: ReactNode }) {
const registryRef = useRef<Map<string, RegistryCommand>>(new Map()) const registryRef = useRef<Map<string, RegistryCommand>>(new Map())
const isMac = useMemo(() => isMacPlatform(), []) const isMac = useMemo(() => isMacPlatform(), [])
@@ -140,13 +96,11 @@ export function GlobalCommandsProvider({ children }: { children: ReactNode }) {
allowInEditable: cmd.allowInEditable ?? true, allowInEditable: cmd.allowInEditable ?? true,
}) })
createdIds.push(id) createdIds.push(id)
logger.info('Registered global command', { id, shortcut: cmd.shortcut })
} }
return () => { return () => {
for (const id of createdIds) { for (const id of createdIds) {
registryRef.current.delete(id) registryRef.current.delete(id)
logger.info('Unregistered global command', { id })
} }
} }
}, []) }, [])
@@ -155,8 +109,6 @@ export function GlobalCommandsProvider({ children }: { children: ReactNode }) {
const onKeyDown = (e: KeyboardEvent) => { const onKeyDown = (e: KeyboardEvent) => {
if (e.isComposing) return if (e.isComposing) return
// Evaluate matches in registration order (latest registration wins naturally
// due to replacement on same id). Break on first match.
for (const [, cmd] of registryRef.current) { for (const [, cmd] of registryRef.current) {
if (!cmd.allowInEditable) { if (!cmd.allowInEditable) {
const ae = document.activeElement const ae = document.activeElement
@@ -168,16 +120,8 @@ export function GlobalCommandsProvider({ children }: { children: ReactNode }) {
} }
if (matchesShortcut(e, cmd.parsed)) { if (matchesShortcut(e, cmd.parsed)) {
// Always override default browser behavior for matched commands.
e.preventDefault() e.preventDefault()
e.stopPropagation() e.stopPropagation()
logger.info('Executing global command', {
id: cmd.id,
shortcut: cmd.shortcut,
key: e.key,
isMac,
path: typeof window !== 'undefined' ? window.location.pathname : undefined,
})
try { try {
cmd.handler(e) cmd.handler(e)
} catch (err) { } catch (err) {
@@ -197,22 +141,28 @@ export function GlobalCommandsProvider({ children }: { children: ReactNode }) {
return <GlobalCommandsContext.Provider value={value}>{children}</GlobalCommandsContext.Provider> return <GlobalCommandsContext.Provider value={value}>{children}</GlobalCommandsContext.Provider>
} }
/**
* Registers a set of global commands for the lifetime of the component.
*
* Returns nothing; cleanup is automatic on unmount.
*/
export function useRegisterGlobalCommands(commands: GlobalCommand[] | (() => GlobalCommand[])) { export function useRegisterGlobalCommands(commands: GlobalCommand[] | (() => GlobalCommand[])) {
const ctx = useContext(GlobalCommandsContext) const ctx = useContext(GlobalCommandsContext)
if (!ctx) { if (!ctx) {
throw new Error('useRegisterGlobalCommands must be used within GlobalCommandsProvider') throw new Error('useRegisterGlobalCommands must be used within GlobalCommandsProvider')
} }
const commandsRef = useRef<GlobalCommand[]>([])
const list = typeof commands === 'function' ? commands() : commands
commandsRef.current = list
useEffect(() => { useEffect(() => {
const list = typeof commands === 'function' ? commands() : commands const wrappedCommands = commandsRef.current.map((cmd) => ({
const unregister = ctx.register(list) ...cmd,
handler: (event: KeyboardEvent) => {
const currentCmd = commandsRef.current.find((c) => c.id === cmd.id)
if (currentCmd) {
currentCmd.handler(event)
}
},
}))
const unregister = ctx.register(wrappedCommands)
return unregister return unregister
// We intentionally want to register once for the given commands
// eslint-disable-next-line react-hooks/exhaustive-deps // eslint-disable-next-line react-hooks/exhaustive-deps
}, []) }, [])
} }

View File

@@ -1055,7 +1055,7 @@ export function Chat() {
{isStreaming ? ( {isStreaming ? (
<Button <Button
onClick={handleStopStreaming} onClick={handleStopStreaming}
className='h-[22px] w-[22px] rounded-full p-0 transition-colors !bg-[var(--c-C0C0C0)] hover:!bg-[var(--c-D0D0D0)]' className='!bg-[var(--c-C0C0C0)] hover:!bg-[var(--c-D0D0D0)] h-[22px] w-[22px] rounded-full p-0 transition-colors'
> >
<Square className='h-2.5 w-2.5 fill-black text-black' /> <Square className='h-2.5 w-2.5 fill-black text-black' />
</Button> </Button>

View File

@@ -133,6 +133,13 @@ export function Panel() {
} }
} }
/**
* Cancels the currently executing workflow
*/
const cancelWorkflow = useCallback(async () => {
await handleCancelExecution()
}, [handleCancelExecution])
/** /**
* Runs the workflow with usage limit check * Runs the workflow with usage limit check
*/ */
@@ -144,13 +151,6 @@ export function Panel() {
await handleRunWorkflow() await handleRunWorkflow()
}, [usageExceeded, handleRunWorkflow]) }, [usageExceeded, handleRunWorkflow])
/**
* Cancels the currently executing workflow
*/
const cancelWorkflow = useCallback(async () => {
await handleCancelExecution()
}, [handleCancelExecution])
// Chat state // Chat state
const { isChatOpen, setIsChatOpen } = useChatStore() const { isChatOpen, setIsChatOpen } = useChatStore()
const { isOpen: isVariablesOpen, setIsOpen: setVariablesOpen } = useVariablesStore() const { isOpen: isVariablesOpen, setIsOpen: setVariablesOpen } = useVariablesStore()
@@ -300,7 +300,6 @@ export function Panel() {
{ {
id: 'run-workflow', id: 'run-workflow',
handler: () => { handler: () => {
// Do exactly what the Run button does
if (isExecuting) { if (isExecuting) {
void cancelWorkflow() void cancelWorkflow()
} else { } else {