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
This commit is contained in:
Jinjing
2026-04-08 20:43:50 -07:00
committed by GitHub
parent 01f3483b21
commit 02a092b52f
7 changed files with 174 additions and 72 deletions
+2
View File
@@ -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 ? <RightSidebar /> : null}
</div>
<QuickOpen />
<ZoomOverlay />
<Toaster closeButton toastOptions={{ className: 'font-sans text-sm' }} />
</div>
)
@@ -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<ZoomLevelChangedEventDetail | null>(null)
const hideTimerRef = useRef<number | undefined>(undefined)
const unmountTimerRef = useRef<number | undefined>(undefined)
useEffect(() => {
const onZoomLevelChanged = (e: Event): void => {
const customEvent = e as CustomEvent<ZoomLevelChangedEventDetail>
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 (
<div
className={`pointer-events-none fixed inset-0 z-50 flex items-center justify-center transition-opacity duration-300 ${
visible ? 'opacity-100' : 'opacity-0'
}`}
>
<div
className={`flex items-center gap-3 rounded-full bg-popover/95 px-5 py-2.5 text-popover-foreground shadow-2xl border border-border/50 backdrop-blur-md transition-transform duration-300 ease-out ${
visible ? 'scale-100 translate-y-0' : 'scale-95 translate-y-4'
}`}
>
<Search className="size-4 text-muted-foreground" />
<div className="flex items-baseline gap-2">
<span className="text-xs font-medium text-muted-foreground">{title}</span>
<span className="text-sm font-bold tabular-nums">{detail.percent}%</span>
</div>
</div>
</div>
)
}
@@ -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'
@@ -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<PaneManager | null>
paneFontSizesRef: React.RefObject<Map<number, number>>
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])
}
@@ -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<PaneManager | null>
paneFontSizesRef: React.RefObject<Map<number, number>>
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])
}
+17 -3
View File
@@ -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))
})
)
+16
View File
@@ -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<ZoomLevelChangedEventDetail>(ZOOM_LEVEL_CHANGED_EVENT, {
detail: { type, percent }
})
)
}