From 02a092b52fd984e2d20aec64516f0ed3229690a5 Mon Sep 17 00:00:00 2001 From: Jinjing <6427696+AmethystLiang@users.noreply.github.com> Date: Wed, 8 Apr 2026 20:43:50 -0700 Subject: [PATCH] fix: address review findings (#402) - Fix ZoomOverlay to fully unmount after fade-out so the fixed overlay doesn't linger and interfere with Radix portal layering, click-outside detection, or focus management (fixes broken file switching) - Lower z-index from 9999 to z-50 to match app conventions - Fix editor zoom percent to use actual base font size from settings and computeEditorFontSize for clamping instead of hardcoded 14 - Extract useTerminalFontZoom to own file to keep keyboard-handlers under the 300-line lint limit --- src/renderer/src/App.tsx | 2 + src/renderer/src/components/ZoomOverlay.tsx | 72 +++++++++++++++++++ .../components/terminal-pane/TerminalPane.tsx | 3 +- .../terminal-pane/keyboard-handlers.ts | 71 +----------------- .../terminal-pane/useTerminalFontZoom.ts | 62 ++++++++++++++++ src/renderer/src/hooks/useIpcEvents.ts | 20 +++++- src/renderer/src/lib/zoom-events.ts | 16 +++++ 7 files changed, 174 insertions(+), 72 deletions(-) create mode 100644 src/renderer/src/components/ZoomOverlay.tsx create mode 100644 src/renderer/src/components/terminal-pane/useTerminalFontZoom.ts create mode 100644 src/renderer/src/lib/zoom-events.ts diff --git a/src/renderer/src/App.tsx b/src/renderer/src/App.tsx index 9e7baa240b8..cfcf61e90a0 100644 --- a/src/renderer/src/App.tsx +++ b/src/renderer/src/App.tsx @@ -16,6 +16,7 @@ import Landing from './components/Landing' import Settings from './components/settings/Settings' import RightSidebar from './components/right-sidebar' import QuickOpen from './components/QuickOpen' +import { ZoomOverlay } from './components/ZoomOverlay' import { useGitStatusPolling } from './components/right-sidebar/useGitStatusPolling' import { setRuntimeGraphStoreStateGetter, @@ -596,6 +597,7 @@ function App(): React.JSX.Element { {showSidebar && rightSidebarOpen ? : null} + ) diff --git a/src/renderer/src/components/ZoomOverlay.tsx b/src/renderer/src/components/ZoomOverlay.tsx new file mode 100644 index 00000000000..7b3bfd5a1bf --- /dev/null +++ b/src/renderer/src/components/ZoomOverlay.tsx @@ -0,0 +1,72 @@ +import { useEffect, useRef, useState } from 'react' +import { Search } from 'lucide-react' +import { ZOOM_LEVEL_CHANGED_EVENT } from '@/lib/zoom-events' +import type { ZoomLevelChangedEventDetail } from '@/lib/zoom-events' + +// Why: the overlay must fully unmount after its fade-out completes so the +// fixed-position container doesn't linger in the DOM and interfere with +// Radix portal layering, click-outside detection, or focus management +// used by dropdowns, context menus, and dialogs elsewhere in the app. +const DISPLAY_MS = 1500 +const FADE_MS = 300 + +export function ZoomOverlay(): React.JSX.Element | null { + const [visible, setVisible] = useState(false) + const [detail, setDetail] = useState(null) + const hideTimerRef = useRef(undefined) + const unmountTimerRef = useRef(undefined) + + useEffect(() => { + const onZoomLevelChanged = (e: Event): void => { + const customEvent = e as CustomEvent + setDetail(customEvent.detail) + setVisible(true) + + window.clearTimeout(hideTimerRef.current) + window.clearTimeout(unmountTimerRef.current) + + hideTimerRef.current = window.setTimeout(() => { + setVisible(false) + // Clear detail after the CSS fade-out transition finishes so the + // component fully unmounts and removes the fixed overlay from the DOM. + unmountTimerRef.current = window.setTimeout(() => { + setDetail(null) + }, FADE_MS) + }, DISPLAY_MS) + } + + window.addEventListener(ZOOM_LEVEL_CHANGED_EVENT, onZoomLevelChanged) + return () => { + window.removeEventListener(ZOOM_LEVEL_CHANGED_EVENT, onZoomLevelChanged) + window.clearTimeout(hideTimerRef.current) + window.clearTimeout(unmountTimerRef.current) + } + }, []) + + if (!detail) { + return null + } + + const title = + detail.type === 'ui' ? 'UI Zoom' : detail.type === 'editor' ? 'Editor Zoom' : 'Terminal Zoom' + + return ( +
+
+ +
+ {title} + {detail.percent}% +
+
+
+ ) +} diff --git a/src/renderer/src/components/terminal-pane/TerminalPane.tsx b/src/renderer/src/components/terminal-pane/TerminalPane.tsx index cc6ba59d198..aab49b882fc 100644 --- a/src/renderer/src/components/terminal-pane/TerminalPane.tsx +++ b/src/renderer/src/components/terminal-pane/TerminalPane.tsx @@ -14,7 +14,8 @@ import type { PtyTransport } from './pty-transport' import { fitPanes, shellEscapePath } from './pane-helpers' import { EMPTY_LAYOUT, paneLeafId, serializeTerminalLayout } from './layout-serialization' import { createExpandCollapseActions } from './expand-collapse' -import { useTerminalKeyboardShortcuts, useTerminalFontZoom } from './keyboard-handlers' +import { useTerminalKeyboardShortcuts } from './keyboard-handlers' +import { useTerminalFontZoom } from './useTerminalFontZoom' import CloseTerminalDialog from './CloseTerminalDialog' import { TerminalErrorToast } from './TerminalErrorToast' import TerminalContextMenu from './TerminalContextMenu' diff --git a/src/renderer/src/components/terminal-pane/keyboard-handlers.ts b/src/renderer/src/components/terminal-pane/keyboard-handlers.ts index b63837feb4c..82f24eafb22 100644 --- a/src/renderer/src/components/terminal-pane/keyboard-handlers.ts +++ b/src/renderer/src/components/terminal-pane/keyboard-handlers.ts @@ -209,20 +209,17 @@ export function useTerminalKeyboardShortcuts({ if (isEditableTarget(e.target)) { return } - const manager = managerRef.current if (!manager) { return } - e.preventDefault() e.stopPropagation() const pane = manager.getActivePane() ?? manager.getPanes()[0] if (!pane) { return } - const transport = paneTransportsRef.current.get(pane.id) - transport?.sendInput('\x1b[13;2u') + paneTransportsRef.current.get(pane.id)?.sendInput('\x1b[13;2u') } // Ctrl+Backspace → send \x17 (backward-kill-word) to PTY. @@ -237,20 +234,17 @@ export function useTerminalKeyboardShortcuts({ if (isEditableTarget(e.target)) { return } - const manager = managerRef.current if (!manager) { return } - e.preventDefault() e.stopPropagation() const pane = manager.getActivePane() ?? manager.getPanes()[0] if (!pane) { return } - const transport = paneTransportsRef.current.get(pane.id) - transport?.sendInput('\x17') + paneTransportsRef.current.get(pane.id)?.sendInput('\x17') } // Alt+Backspace → send ESC + DEL (\x1b\x7f, backward-kill-word) to PTY. @@ -265,20 +259,17 @@ export function useTerminalKeyboardShortcuts({ if (isEditableTarget(e.target)) { return } - const manager = managerRef.current if (!manager) { return } - e.preventDefault() e.stopPropagation() const pane = manager.getActivePane() ?? manager.getPanes()[0] if (!pane) { return } - const transport = paneTransportsRef.current.get(pane.id) - transport?.sendInput('\x1b\x7f') + paneTransportsRef.current.get(pane.id)?.sendInput('\x1b\x7f') } window.addEventListener('keydown', onKeyDown, { capture: true }) @@ -305,59 +296,3 @@ export function useTerminalKeyboardShortcuts({ onRequestClosePane ]) } - -type FontZoomDeps = { - isActive: boolean - managerRef: React.RefObject - paneFontSizesRef: React.RefObject> - settingsRef: React.RefObject<{ terminalFontSize?: number } | null> -} - -export function useTerminalFontZoom({ - isActive, - managerRef, - paneFontSizesRef, - settingsRef -}: FontZoomDeps): void { - useEffect(() => { - if (!isActive) { - return - } - const MIN_FONT_SIZE = 8 - const MAX_FONT_SIZE = 32 - const FONT_SIZE_STEP = 1 - - return window.api.ui.onTerminalZoom((direction) => { - const manager = managerRef.current - if (!manager) { - return - } - const pane = manager.getActivePane() - if (!pane) { - return - } - - const globalSize = settingsRef.current?.terminalFontSize ?? 14 - const currentSize = paneFontSizesRef.current.get(pane.id) ?? globalSize - - let nextSize: number - if (direction === 'reset') { - nextSize = globalSize - paneFontSizesRef.current.delete(pane.id) - } else if (direction === 'in') { - nextSize = Math.min(MAX_FONT_SIZE, currentSize + FONT_SIZE_STEP) - paneFontSizesRef.current.set(pane.id, nextSize) - } else { - nextSize = Math.max(MIN_FONT_SIZE, currentSize - FONT_SIZE_STEP) - paneFontSizesRef.current.set(pane.id, nextSize) - } - - pane.terminal.options.fontSize = nextSize - try { - pane.fitAddon.fit() - } catch { - /* ignore */ - } - }) - }, [isActive, managerRef, paneFontSizesRef, settingsRef]) -} diff --git a/src/renderer/src/components/terminal-pane/useTerminalFontZoom.ts b/src/renderer/src/components/terminal-pane/useTerminalFontZoom.ts new file mode 100644 index 00000000000..aa8e64bf1c7 --- /dev/null +++ b/src/renderer/src/components/terminal-pane/useTerminalFontZoom.ts @@ -0,0 +1,62 @@ +import { useEffect } from 'react' +import type { PaneManager } from '@/lib/pane-manager/pane-manager' +import { dispatchZoomLevelChanged } from '@/lib/zoom-events' + +type FontZoomDeps = { + isActive: boolean + managerRef: React.RefObject + paneFontSizesRef: React.RefObject> + settingsRef: React.RefObject<{ terminalFontSize?: number } | null> +} + +export function useTerminalFontZoom({ + isActive, + managerRef, + paneFontSizesRef, + settingsRef +}: FontZoomDeps): void { + useEffect(() => { + if (!isActive) { + return + } + const MIN_FONT_SIZE = 8 + const MAX_FONT_SIZE = 32 + const FONT_SIZE_STEP = 1 + + return window.api.ui.onTerminalZoom((direction) => { + const manager = managerRef.current + if (!manager) { + return + } + const pane = manager.getActivePane() + if (!pane) { + return + } + + const globalSize = settingsRef.current?.terminalFontSize ?? 14 + const currentSize = paneFontSizesRef.current.get(pane.id) ?? globalSize + + let nextSize: number + if (direction === 'reset') { + nextSize = globalSize + paneFontSizesRef.current.delete(pane.id) + } else if (direction === 'in') { + nextSize = Math.min(MAX_FONT_SIZE, currentSize + FONT_SIZE_STEP) + paneFontSizesRef.current.set(pane.id, nextSize) + } else { + nextSize = Math.max(MIN_FONT_SIZE, currentSize - FONT_SIZE_STEP) + paneFontSizesRef.current.set(pane.id, nextSize) + } + + pane.terminal.options.fontSize = nextSize + try { + pane.fitAddon.fit() + } catch { + /* ignore */ + } + + const percent = Math.round((nextSize / globalSize) * 100) + dispatchZoomLevelChanged('terminal', percent) + }) + }, [isActive, managerRef, paneFontSizesRef, settingsRef]) +} diff --git a/src/renderer/src/hooks/useIpcEvents.ts b/src/renderer/src/hooks/useIpcEvents.ts index 8059e141dab..b32d0e2f901 100644 --- a/src/renderer/src/hooks/useIpcEvents.ts +++ b/src/renderer/src/hooks/useIpcEvents.ts @@ -2,9 +2,11 @@ import { useEffect } from 'react' import { useAppStore } from '../store' import { applyUIZoom } from '@/lib/ui-zoom' import { ensureWorktreeHasInitialTerminal } from '@/lib/worktree-activation' -import { nextEditorFontZoomLevel } from '@/lib/editor-font-zoom' +import { nextEditorFontZoomLevel, computeEditorFontSize } from '@/lib/editor-font-zoom' import type { UpdateStatus } from '../../../shared/types' import { createUpdateToastController } from './update-toast-controller' +import { zoomLevelToPercent, ZOOM_MIN, ZOOM_MAX } from '@/components/settings/SettingsConstants' +import { dispatchZoomLevelChanged } from '@/lib/zoom-events' const ZOOM_STEP = 0.5 @@ -120,7 +122,7 @@ export function useIpcEvents(): void { // Zoom handling for menu accelerators and keyboard fallback paths. unsubs.push( window.api.ui.onTerminalZoom((direction) => { - const { activeView, activeTabType, editorFontZoomLevel, setEditorFontZoomLevel } = + const { activeView, activeTabType, editorFontZoomLevel, setEditorFontZoomLevel, settings } = useAppStore.getState() const target = resolveZoomTarget({ activeView, @@ -134,14 +136,26 @@ export function useIpcEvents(): void { const next = nextEditorFontZoomLevel(editorFontZoomLevel, direction) setEditorFontZoomLevel(next) void window.api.ui.set({ editorFontZoomLevel: next }) + + // Why: use the same base font size the editor surfaces use (terminalFontSize) + // and computeEditorFontSize to account for clamping, so the overlay percent + // matches the actual rendered size. + const baseFontSize = settings?.terminalFontSize ?? 13 + const actual = computeEditorFontSize(baseFontSize, next) + const percent = Math.round((actual / baseFontSize) * 100) + dispatchZoomLevelChanged('editor', percent) return } const current = window.api.ui.getZoomLevel() - const next = + const rawNext = direction === 'in' ? current + ZOOM_STEP : direction === 'out' ? current - ZOOM_STEP : 0 + const next = Math.max(ZOOM_MIN, Math.min(ZOOM_MAX, rawNext)) + applyUIZoom(next) void window.api.ui.set({ uiZoomLevel: next }) + + dispatchZoomLevelChanged('ui', zoomLevelToPercent(next)) }) ) diff --git a/src/renderer/src/lib/zoom-events.ts b/src/renderer/src/lib/zoom-events.ts new file mode 100644 index 00000000000..eade941ea70 --- /dev/null +++ b/src/renderer/src/lib/zoom-events.ts @@ -0,0 +1,16 @@ +export type ZoomTargetType = 'ui' | 'editor' | 'terminal' + +export type ZoomLevelChangedEventDetail = { + type: ZoomTargetType + percent: number +} + +export const ZOOM_LEVEL_CHANGED_EVENT = 'orca:zoom-level-changed' + +export function dispatchZoomLevelChanged(type: ZoomTargetType, percent: number): void { + window.dispatchEvent( + new CustomEvent(ZOOM_LEVEL_CHANGED_EVENT, { + detail: { type, percent } + }) + ) +}