From fed446bc618b2a327c1786423fd852e40b7eeb16 Mon Sep 17 00:00:00 2001 From: Chris Lorenzo Date: Wed, 20 May 2026 23:16:12 -0400 Subject: [PATCH] refactor(focus): split propagateKeyPress into phase helpers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The function had grown to 135 lines with two near-identical 9-line throttle checks (once in the capture loop, once in the bubble loop) and two distinct sub-algorithms inlined. Pull them apart: - `isElementThrottled(elm, sameKey, currentTime)` — single source of truth for per-element rate-limiting; previously copy-pasted across the two loops. - `runCapturePhase(...)` — root→leaf walk, returns whether to stop. - `runBubblePhase(...)` — leaf→root walk, returns `{ handled, lastHandlerSeen }` so the debug log keeps its "which element had a handler but didn't claim" output without a function-scope mutable. `propagateKeyPress` is now a flat orchestrator: global throttle → history record → capture → bubble → debug log. Behavior is unchanged; the LoC goes up slightly (helpers carry signatures and one short doc comment each) but the orchestrator body drops from ~135 to ~50 lines. Co-Authored-By: Claude Opus 4.7 --- src/core/focusManager.ts | 202 ++++++++++++++++++++++----------------- 1 file changed, 112 insertions(+), 90 deletions(-) diff --git a/src/core/focusManager.ts b/src/core/focusManager.ts index 0a207bb..45d1acc 100644 --- a/src/core/focusManager.ts +++ b/src/core/focusManager.ts @@ -273,60 +273,34 @@ const updateFocusPath = ( let lastGlobalKeyPressTime = 0; let lastInputKey: string | number | undefined; -const propagateKeyPress = ( +const isElementThrottled = ( + elm: ElementNode, + sameKey: boolean, + currentTime: number, +): boolean => + elm.throttleInput !== undefined && + sameKey && + elm._lastAnyKeyPressTime !== undefined && + currentTime - elm._lastAnyKeyPressTime < elm.throttleInput; + +// Walk focus path root→leaf. Returns true if a capture handler claimed the +// event (or an element on the path is currently rate-limited). +const runCapturePhase = ( + fp: ElementNode[], e: KeyboardEvent, - mappedEvent?: string, - isHold: boolean = false, - isUp: boolean = false, + mappedEvent: string | undefined, + isUp: boolean, + sameKey: boolean, + currentTime: number, ): boolean => { - const currentTime = performance.now(); - const key = e.key || e.keyCode; - const sameKey = lastInputKey === key; - lastInputKey = key; - - if (!isUp && Config.throttleInput) { - if ( - sameKey && - currentTime - lastGlobalKeyPressTime < Config.throttleInput - ) { - if (isDev && Config.keyDebug) { - console.log( - `Keypress throttled by global Config.throttleInput: ${Config.throttleInput}ms`, - ); - } - return false; - } - lastGlobalKeyPressTime = currentTime; - } - - // Record the key for focus-history attribution (keyup presses don't trigger focus changes) - if (!isUp) { - _pendingHistoryKey = { keyPressed: key, mappedKey: mappedEvent }; - } - - const fp = focusPath(); - const numItems = fp.length; - if (numItems === 0) return false; - - let handlerAvailable: ElementNode | undefined; const finalFocusElm = fp[0]!; const keyBase = mappedEvent || e.key; const captureEvent = `onCapture${keyBase}${isUp ? 'Release' : ''}`; const captureKey = isUp ? 'onCaptureKeyRelease' : 'onCaptureKey'; - for (let i = numItems - 1; i >= 0; i--) { + for (let i = fp.length - 1; i >= 0; i--) { const elm = fp[i]!; - - // Check throttle for capture phase - if (elm.throttleInput) { - if ( - sameKey && - elm._lastAnyKeyPressTime !== undefined && - currentTime - elm._lastAnyKeyPressTime < elm.throttleInput - ) { - return true; - } - } + if (isElementThrottled(elm, sameKey, currentTime)) return true; const captureHandler = elm[captureEvent] || elm[captureKey]; if ( @@ -337,73 +311,121 @@ const propagateKeyPress = ( return true; } } + return false; +}; - let eventHandlerKey: string | undefined; - let fallbackHandlerKey: 'onKeyHold' | 'onKeyPress' | undefined; - - if (mappedEvent) { - eventHandlerKey = isUp ? `on${mappedEvent}Release` : `on${mappedEvent}`; - } - - if (!isUp) { - fallbackHandlerKey = isHold ? 'onKeyHold' : 'onKeyPress'; - } - - for (let i = 0; i < numItems; i++) { +// Walk focus path leaf→root. Returns whether the event was handled and the +// last element that had *any* matching handler (for the no-handler debug log). +const runBubblePhase = ( + fp: ElementNode[], + e: KeyboardEvent, + mappedEvent: string | undefined, + isHold: boolean, + isUp: boolean, + sameKey: boolean, + currentTime: number, +): { handled: boolean; lastHandlerSeen: ElementNode | undefined } => { + const finalFocusElm = fp[0]!; + const eventHandlerKey = mappedEvent + ? isUp + ? `on${mappedEvent}Release` + : `on${mappedEvent}` + : undefined; + const fallbackHandlerKey: 'onKeyHold' | 'onKeyPress' | undefined = isUp + ? undefined + : isHold + ? 'onKeyHold' + : 'onKeyPress'; + + let lastHandlerSeen: ElementNode | undefined; + + for (let i = 0; i < fp.length; i++) { const elm = fp[i]!; - - // Check throttle for bubbling phase - if (elm.throttleInput) { - if ( - sameKey && - elm._lastAnyKeyPressTime !== undefined && - currentTime - elm._lastAnyKeyPressTime < elm.throttleInput - ) { - return true; - } + if (isElementThrottled(elm, sameKey, currentTime)) { + return { handled: true, lastHandlerSeen }; } let handled = false; - if (eventHandlerKey) { const eventHandler = elm[eventHandlerKey]; if (isFunction(eventHandler)) { - handlerAvailable = elm; - if (eventHandler.call(elm, e, elm, finalFocusElm) === true) { - handled = true; - } + lastHandlerSeen = elm; + handled = eventHandler.call(elm, e, elm, finalFocusElm) === true; } } - - // Check for the fallback handler if its key is defined and not already handled by specific key handler if (!handled && fallbackHandlerKey) { const fallbackHandler = elm[fallbackHandlerKey]; if (isFunction(fallbackHandler)) { - handlerAvailable = elm; - if ( - fallbackHandler.call(elm, e, mappedEvent, elm, finalFocusElm) === true - ) { - handled = true; - } + lastHandlerSeen = elm; + handled = + fallbackHandler.call(elm, e, mappedEvent, elm, finalFocusElm) === + true; } } if (handled) { elm._lastAnyKeyPressTime = currentTime; - return true; + return { handled: true, lastHandlerSeen }; } } + return { handled: false, lastHandlerSeen }; +}; + +const propagateKeyPress = ( + e: KeyboardEvent, + mappedEvent?: string, + isHold: boolean = false, + isUp: boolean = false, +): boolean => { + const currentTime = performance.now(); + const key = e.key || e.keyCode; + const sameKey = lastInputKey === key; + lastInputKey = key; + + if (!isUp && Config.throttleInput) { + if ( + sameKey && + currentTime - lastGlobalKeyPressTime < Config.throttleInput + ) { + if (isDev && Config.keyDebug) { + console.log( + `Keypress throttled by global Config.throttleInput: ${Config.throttleInput}ms`, + ); + } + return false; + } + lastGlobalKeyPressTime = currentTime; + } + + // Keyup events don't trigger focus changes, so don't record their key. + if (!isUp) { + _pendingHistoryKey = { keyPressed: key, mappedKey: mappedEvent }; + } + + const fp = focusPath(); + if (fp.length === 0) return false; + + if (runCapturePhase(fp, e, mappedEvent, isUp, sameKey, currentTime)) { + return true; + } + + const { handled, lastHandlerSeen } = runBubblePhase( + fp, + e, + mappedEvent, + isHold, + isUp, + sameKey, + currentTime, + ); + if (handled) return true; if (isDev && Config.keyDebug && !isUp) { - if (handlerAvailable) { - console.log( - `Keypress bubbled, key="${e.key}", mappedEvent=${mappedEvent}, isHold=${isHold}, isUp=${isUp}`, - handlerAvailable, - ); + const detail = `key="${e.key}", mappedEvent=${mappedEvent}, isHold=${isHold}, isUp=${isUp}`; + if (lastHandlerSeen) { + console.log(`Keypress bubbled, ${detail}`, lastHandlerSeen); } else { - console.log( - `No event handler available for keypress: key="${e.key}", mappedEvent=${mappedEvent}, isHold=${isHold}, isUp=${isUp}`, - ); + console.log(`No event handler available for keypress: ${detail}`); } }