refactor(a11y): optimize dialog focus management with centralized Escape handling

Enhanced the useDialogFocus hook to support conditional Escape key handling,
removing redundant Escape key listeners from individual dialog components.

Changes:
- Added shouldCloseOnEscape option to useDialogFocus (boolean or function callback)
- Removed redundant Escape key handlers from Modal, ExecutionReportDialog, and LoginDialog
- ExecutionReportDialog now uses shouldCloseOnEscape: () => !isExecuting to prevent
  closing during execution
- LoginDialog no longer has manual focus effect (handled by initialFocusSelector)
- Fixed React hooks order violations in ExecutionReportDialog
- Added proper TypeScript return types throughout

Benefits:
- Centralized Escape key logic in one place
- Consistent behavior across all dialogs
- Reduced code duplication (~50 lines removed)
- Easier to maintain and extend

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>

src/renderer/src/components/ExecutionReportDialog.tsx

@@ -49,40 +49,19 @@ export const ExecutionReportDialog: React.FC<ExecutionReportDialogProps> = ({
   startTime = null
 }) => {
   const dialogRef = useRef<HTMLDivElement>(null)
+  const [now, setNow] = React.useState(() => Date.now())
 
-  // Setup focus management with custom escape key handling
+  // Setup focus management with conditional escape key handling
   const { focusLockProps } = useDialogFocus({
     isOpen,
     dialogRef,
-    onClose
+    onClose,
+    shouldCloseOnEscape: () => !isExecuting // Only close when NOT executing
   })
 
-  // Custom escape key handling - only close when NOT executing
-  React.useEffect(() => {
-    if (!isOpen) return
-
-    const handleKeyDown = (event: KeyboardEvent) => {
-      // Only allow escape to close when not executing
-      if ((event.key === 'Escape' || event.keyCode === 27) && !isExecuting) {
-        event.preventDefault()
-        event.stopPropagation()
-        onClose()
-      }
-    }
-
-    window.addEventListener('keydown', handleKeyDown)
-    return () => {
-      window.removeEventListener('keydown', handleKeyDown)
-    }
-  }, [isOpen, isExecuting, onClose])
-
-  if (!isOpen) return null
-
   const hasErrors = errors.length > 0
   const showProgress = isExecuting && progress
 
-  const [now, setNow] = React.useState(Date.now())
-
   React.useEffect(() => {
     if (!showProgress || !startTime) return
 
@@ -120,6 +99,8 @@ export const ExecutionReportDialog: React.FC<ExecutionReportDialogProps> = ({
     }
   }, [showProgress, startTime, progress, now])
 
+  if (!isOpen) return null
+
   return (
     <FocusLock {...focusLockProps}>
       <div

src/renderer/src/components/LoginDialog.tsx

@@ -7,7 +7,7 @@
  * - Enter key to submit
  */
 
-import React, { useState, useEffect, useRef } from 'react'
+import React, { useState, useRef } from 'react'
 import FocusLock from 'react-focus-lock'
 import { useDialogFocus } from '../hooks/useDialogFocus'
 
@@ -43,22 +43,16 @@ export const LoginDialog: React.FC<LoginDialogProps> = ({
   })
 
   // Display error message with aria-live
-  const showError = (message: string) => {
+  const showError = (message: string): void => {
     setErrorMessage(message)
     // Also call the original onError callback for backward compatibility
     onError(message)
   }
 
-  // Focus on username input when dialog opens (maintained for compatibility)
+  const handleLogin = async (): Promise<void> => {
-  useEffect(() => {
+    // Clear error message when attempting login
-    if (isOpen && usernameInputRef.current) {
-      usernameInputRef.current.focus()
-    }
-    // Clear error message when dialog opens
     setErrorMessage('')
-  }, [isOpen])
 
-  const handleLogin = async () => {
     if (!username.trim()) {
       showError('请输入用户名')
       usernameInputRef.current?.focus()
@@ -80,11 +74,9 @@ export const LoginDialog: React.FC<LoginDialogProps> = ({
     }
   }
 
-  const handleKeyDown = (e: React.KeyboardEvent) => {
+  const handleKeyDown = (e: React.KeyboardEvent): void => {
     if (e.key === 'Enter') {
       handleLogin()
-    } else if (e.key === 'Escape') {
-      onCancel()
     }
   }
 

src/renderer/src/components/ui/Modal.tsx

@@ -4,7 +4,7 @@
  * A reusable modal dialog component.
  */
 
-import React, { useEffect, useCallback, useRef } from 'react'
+import React, { useRef, useMemo, useState } from 'react'
 import { X } from 'lucide-react'
 import FocusLock from 'react-focus-lock'
 import { useDialogFocus } from '../../hooks/useDialogFocus'
@@ -40,13 +40,18 @@ export function Modal({
   showCloseButton = true,
   triggerRef,
   titleId
-}: ModalProps) {
+}: ModalProps): React.JSX.Element | null {
   const dialogRef = useRef<HTMLDivElement>(null)
+  const [generatedId] = useState(
+    () => `modal-title-${Date.now().toString(36)}-${Math.random().toString(36).substr(2, 9)}`
+  )
 
-  // Generate unique title ID if not provided
+  // Use provided titleId or generated one
-  const generatedTitleId = titleId || `modal-title-${Math.random().toString(36).substr(2, 9)}`
+  const generatedTitleId = useMemo((): string => {
+    return titleId || generatedId
+  }, [titleId, generatedId])
 
-  // Setup focus management
+  // Setup focus management (includes Escape key handling)
   const { focusLockProps } = useDialogFocus({
     isOpen,
     dialogRef,
@@ -54,26 +59,6 @@ export function Modal({
     triggerRef: triggerRef || undefined
   })
 
-  // Handle escape key (maintained for backward compatibility, useDialogFocus also handles this)
-  const handleKeyDown = useCallback(
-    (event: KeyboardEvent) => {
-      if (event.key === 'Escape') {
-        onClose()
-      }
-    },
-    [onClose]
-  )
-
-  useEffect(() => {
-    if (isOpen) {
-      document.addEventListener('keydown', handleKeyDown)
-    }
-
-    return () => {
-      document.removeEventListener('keydown', handleKeyDown)
-    }
-  }, [isOpen, handleKeyDown])
-
   if (!isOpen) return null
 
   return (

src/renderer/src/hooks/useDialogFocus.ts

@@ -16,6 +16,11 @@ export interface UseDialogFocusOptions {
   initialFocusSelector?: string
   /** Whether to lock body scroll when dialog is open (default: true) */
   lockBodyScroll?: boolean
+  /**
+   * Whether Escape key should close the dialog (default: true)
+   * Can be a boolean or a function that receives the keyboard event and returns a boolean
+   */
+  shouldCloseOnEscape?: boolean | ((event: KeyboardEvent) => boolean)
 }
 
 /**
@@ -45,14 +50,15 @@ export interface UseDialogFocusReturn {
  *
  * @example
  * ```typescript
- * function MyDialog({ isOpen, onClose }) {
+ * function MyDialog({ isOpen, onClose, isExecuting }) {
  *   const dialogRef = useRef<HTMLDivElement>(null)
  *   const triggerRef = useRef<HTMLButtonElement>(null)
  *   const { focusLockEnabled, focusLockProps } = useDialogFocus({
  *     isOpen,
  *     dialogRef,
  *     onClose,
- *     triggerRef
+ *     triggerRef,
+ *     shouldCloseOnEscape: () => !isExecuting // Only close when not executing
  *   })
  *
  *   return (
@@ -73,26 +79,39 @@ export function useDialogFocus(options: UseDialogFocusOptions): UseDialogFocusRe
     onClose,
     triggerRef,
     initialFocusSelector,
-    lockBodyScroll = true
+    lockBodyScroll = true,
+    shouldCloseOnEscape = true
   } = options
 
   // Handle Escape key press
   useEffect(() => {
     if (!isOpen) return
 
-    const handleKeyDown = (event: KeyboardEvent) => {
+    const handleKeyDown = (event: KeyboardEvent): void => {
       if (event.key === 'Escape' || event.keyCode === 27) {
-        event.preventDefault()
+        // Check if we should close on escape
-        event.stopPropagation()
+        let shouldClose = true
-        onClose()
+        if (shouldCloseOnEscape !== undefined) {
+          if (typeof shouldCloseOnEscape === 'function') {
+            shouldClose = shouldCloseOnEscape(event)
+          } else {
+            shouldClose = shouldCloseOnEscape
+          }
+        }
+
+        if (shouldClose) {
+          event.preventDefault()
+          event.stopPropagation()
+          onClose()
+        }
       }
     }
 
     window.addEventListener('keydown', handleKeyDown)
-    return () => {
+    return (): void => {
       window.removeEventListener('keydown', handleKeyDown)
     }
-  }, [isOpen, onClose])
+  }, [isOpen, onClose, shouldCloseOnEscape])
 
   // Manage body scroll locking
   useEffect(() => {
@@ -119,7 +138,7 @@ export function useDialogFocus(options: UseDialogFocusOptions): UseDialogFocusRe
       }
     }
 
-    return () => {
+    return (): void => {
       // Cleanup on unmount or when isOpen changes
       if (isOpen) {
         const scrollY = document.body.style.top
@@ -139,7 +158,7 @@ export function useDialogFocus(options: UseDialogFocusOptions): UseDialogFocusRe
   useEffect(() => {
     if (!isOpen || !dialogRef.current) return
 
-    const setupFocus = () => {
+    const setupFocus = (): void => {
       const dialogElement = dialogRef.current
       if (!dialogElement) return
 
@@ -182,7 +201,7 @@ export function useDialogFocus(options: UseDialogFocusOptions): UseDialogFocusRe
   useEffect(() => {
     if (isOpen || !triggerRef?.current) return
 
-    const restoreFocus = () => {
+    const restoreFocus = (): void => {
       const triggerElement = triggerRef.current
       if (triggerElement && typeof triggerElement.focus === 'function') {
         // Delay to ensure dialog is fully unmounted