feat: keybind ui tweaks, redo/undo configuration (#517)

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

ui/components/MenuBar.tsx

@@ -2,7 +2,7 @@
 
 import { CopyIcon, MinusIcon, SquareIcon, XIcon } from 'lucide-react'
 import Image from 'next/image'
-import { useCallback, useEffect, useState } from 'react'
+import { useCallback, useEffect, useMemo, useState } from 'react'
 import { useTranslation } from 'react-i18next'
 
 import { fitCanvasToViewport, resetCanvasScale } from '@/components/Canvas'
@@ -21,13 +21,11 @@ import { getConfig, startPipeline } from '@/lib/api/default/default'
 import { isTauri, openExternalUrl } from '@/lib/backend'
 import { exportCurrentProjectAs, importPages } from '@/lib/io/pagesIo'
 import { closeProject, redoOp, selectAllTextNodesOnCurrentPage, undoOp } from '@/lib/io/scene'
+import { formatShortcutForDisplay, getPlatform } from '@/lib/shortcutUtils'
 import { useEditorUiStore } from '@/lib/stores/editorUiStore'
 import { usePreferencesStore } from '@/lib/stores/preferencesStore'
 import { useSelectionStore } from '@/lib/stores/selectionStore'
 
-const isMacOS = () =>
-  typeof navigator !== 'undefined' && /Mac|iPhone|iPad|iPod/.test(navigator.userAgent)
-
 const windowControls = {
   async close() {
     const { getCurrentWindow } = await import('@tauri-apps/api/window')
@@ -66,6 +64,8 @@ export function MenuBar() {
   const [settingsTab, setSettingsTab] = useState<TabId>('appearance')
   const hasPage = useSelectionStore((s) => s.pageId !== null)
   const hasScene = useScene().scene !== null
+  const shortcuts = usePreferencesStore((state) => state.shortcuts)
+  const isMac = useMemo(() => getPlatform() === 'mac', [])
 
   const requirePageId = () => {
     const id = useSelectionStore.getState().pageId
@@ -173,8 +173,8 @@ export function MenuBar() {
     },
   ]
 
-  const isNativeMacOS = isTauri() && isMacOS()
-  const isWindowsTauri = isTauri() && !isMacOS()
+  const isNativeMacOS = isTauri() && isMac
+  const isWindowsTauri = isTauri() && !isMac
 
   return (
     <div className='flex h-8 items-center border-b border-border bg-background text-[13px] text-foreground'>
@@ -264,7 +264,7 @@ export function MenuBar() {
               onSelect={() => void undoOp()}
             >
               {t('menu.undo')}
-              <MenubarShortcut>{isMacOS() ? '⌘Z' : 'Ctrl+Z'}</MenubarShortcut>
+              <MenubarShortcut>{formatShortcutForDisplay(shortcuts.undo, isMac)}</MenubarShortcut>
             </MenubarItem>
             <MenubarItem
               data-testid='menu-edit-redo'
@@ -273,7 +273,7 @@ export function MenuBar() {
               onSelect={() => void redoOp()}
             >
               {t('menu.redo')}
-              <MenubarShortcut>{isMacOS() ? '⇧⌘Z' : 'Ctrl+Shift+Z'}</MenubarShortcut>
+              <MenubarShortcut>{formatShortcutForDisplay(shortcuts.redo, isMac)}</MenubarShortcut>
             </MenubarItem>
             <MenubarSeparator />
             <MenubarItem
@@ -283,7 +283,7 @@ export function MenuBar() {
               onSelect={() => selectAllTextNodesOnCurrentPage()}
             >
               {t('menu.selectAll')}
-              <MenubarShortcut>{isMacOS() ? '⌘A' : 'Ctrl+A'}</MenubarShortcut>
+              <MenubarShortcut>{isMac ? '⌘A' : 'Ctrl+A'}</MenubarShortcut>
             </MenubarItem>
           </MenubarContent>
         </MenubarMenu>

ui/components/SettingsDialog.tsx

@@ -19,7 +19,7 @@ import {
   AlertTriangleIcon,
 } from 'lucide-react'
 import { useTheme } from 'next-themes'
-import { useEffect, useMemo, useRef, useState, type ReactNode } from 'react'
+import { Fragment, useEffect, useMemo, useRef, useState, type ReactNode } from 'react'
 import { useTranslation } from 'react-i18next'
 
 import {
@@ -39,6 +39,7 @@ import {
 import { Button } from '@/components/ui/button'
 import { Dialog, DialogContent, DialogTitle, DialogDescription } from '@/components/ui/dialog'
 import { Input } from '@/components/ui/input'
+import { Kbd } from '@/components/ui/kbd'
 import { Label } from '@/components/ui/label'
 import { ScrollArea } from '@/components/ui/scroll-area'
 import {
@@ -69,6 +70,7 @@ import { supportedLanguages } from '@/lib/i18n'
 import {
   areShortcutsEqual,
   formatShortcut,
+  formatModifierCombination,
   getPlatform,
   isKeyBlocked,
   isModifierKey,
@@ -688,6 +690,8 @@ const SHORTCUT_ITEMS = [
     key: 'decreaseBrushSize',
     labelKey: 'settings.shortcutDecreaseBrushSize',
   },
+  { key: 'undo', labelKey: 'menu.undo' },
+  { key: 'redo', labelKey: 'menu.redo' },
 ] as const
 
 function KeybindsPane() {
@@ -699,6 +703,7 @@ function KeybindsPane() {
   const [recordingKey, setRecordingKey] = useState<string | null>(null)
   const [error, setError] = useState<string | null>(null)
   const [isSaved, setIsSaved] = useState(false)
+  const [liveShortcut, setLiveShortcut] = useState<string | null>(null)
   const isMac = useMemo(() => getPlatform() === 'mac', [])
 
   // Optimized conflict detection
@@ -723,21 +728,27 @@ function KeybindsPane() {
   useEffect(() => {
     if (!recordingKey) {
       setError(null)
+      setLiveShortcut(null)
       return
     }
 
+    setError(null)
+    setLiveShortcut(null)
     const handleKeyDown = (e: KeyboardEvent) => {
       e.preventDefault()
       e.stopPropagation()
+      setError(null)
 
-      // Early exit for modifier-only events
+      // Early exit for modifier-only events - but update preview!
       if (isModifierKey(e.key)) {
+        setLiveShortcut(formatModifierCombination(e, isMac))
         return
       }
 
       // Allow Escape to cancel recording
       if (e.key === 'Escape') {
         setRecordingKey(null)
+        setLiveShortcut(null)
         return
       }
 
@@ -750,29 +761,30 @@ function KeybindsPane() {
       const shortcut = formatShortcut(e, isMac)
       if (!shortcut) return
 
-      // Conflict detection (now non-blocking)
-      const existingAction = Object.entries(pendingShortcuts).find(
-        ([action, val]) => val === shortcut && action !== recordingKey,
-      )
-
-      if (existingAction) {
-        // Just a hint, we still allow it
-        setError(t('settings.shortcutConflict'))
-      }
-
       setPendingShortcuts((prev) => ({ ...prev, [recordingKey]: shortcut }))
       setRecordingKey(null)
       setIsSaved(false)
+      setLiveShortcut(null)
+    }
+
+    const handleKeyUp = (e: KeyboardEvent) => {
+      if (isModifierKey(e.key)) {
+        const combo = formatModifierCombination(e, isMac)
+        setLiveShortcut(combo || null)
+      }
     }
 
     const handleClickOutside = () => {
       setRecordingKey(null)
+      setLiveShortcut(null)
     }
 
     window.addEventListener('keydown', handleKeyDown, { capture: true })
+    window.addEventListener('keyup', handleKeyUp, { capture: true })
     window.addEventListener('click', handleClickOutside, { capture: true })
     return () => {
       window.removeEventListener('keydown', handleKeyDown, { capture: true })
+      window.removeEventListener('keyup', handleKeyUp, { capture: true })
       window.removeEventListener('click', handleClickOutside, {
         capture: true,
       })
@@ -813,6 +825,17 @@ function KeybindsPane() {
     setResetConfirmOpen(false)
   }
 
+  const renderShortcutKeys = (shortcutStr: string, kbdClass?: string) => {
+    const parts = shortcutStr.split('+')
+
+    return parts.map((part, i) => (
+      <Fragment key={i}>
+        <Kbd className={kbdClass}>{part}</Kbd>
+        {i < parts.length - 1 && <span className='text-muted-foreground'>+</span>}
+      </Fragment>
+    ))
+  }
+
   return (
     <div className='flex h-full flex-col gap-6'>
       <div className='grow space-y-6 overflow-y-auto pr-2'>
@@ -820,30 +843,53 @@ function KeybindsPane() {
           <div className='divide-y divide-border overflow-hidden rounded-xl border border-border bg-card'>
             {SHORTCUT_ITEMS.map((item) => {
               const currentVal = pendingShortcuts[item.key]
-              const hasConflict = (conflictCounts.get(currentVal) || 0) > 1
+              const hasConflict = currentVal && (conflictCounts.get(currentVal) || 0) > 1
+              const conflictingItem = hasConflict
+                ? SHORTCUT_ITEMS.find(
+                    (s) => s.key !== item.key && pendingShortcuts[s.key] === currentVal,
+                  )
+                : null
 
               return (
-                <div key={item.key} className='flex items-center justify-between px-4 py-3'>
+                <div key={item.key} className='flex items-center justify-between px-4 py-2'>
                   <div className='flex items-center gap-2'>
                     <span className='text-sm'>{t(item.labelKey)}</span>
                     {hasConflict && (
-                      <div title={t('settings.shortcutConflict')}>
+                      <div
+                        title={`${t('settings.shortcutConflict')}${
+                          conflictingItem ? `: ${t(conflictingItem.labelKey)}` : ''
+                        }`}
+                      >
                         <AlertTriangleIcon className='size-3.5 text-amber-500' />
                       </div>
                     )}
                   </div>
                   <Button
-                    variant={recordingKey === item.key ? 'default' : 'outline'}
+                    variant={recordingKey === item.key ? 'secondary' : 'ghost'}
                     size='sm'
                     onClick={(e) => {
                       e.stopPropagation()
                       setRecordingKey(item.key)
                     }}
-                    className='h-8 min-w-16 font-mono uppercase'
+                    className='group h-8 w-fit px-2 font-mono uppercase'
                   >
-                    {recordingKey === item.key
-                      ? error || t('settings.shortcutPressKey')
-                      : currentVal || 'NONE'}
+                    <div className='flex items-center gap-1'>
+                      {recordingKey === item.key ? (
+                        error ? (
+                          <span className='text-xs text-destructive'>{error}</span>
+                        ) : liveShortcut ? (
+                          renderShortcutKeys(liveShortcut)
+                        ) : (
+                          <span className='text-xs text-muted-foreground italic'>
+                            {t('settings.shortcutPressKey')}
+                          </span>
+                        )
+                      ) : currentVal ? (
+                        renderShortcutKeys(currentVal, 'bg-background')
+                      ) : (
+                        <span className='text-xs text-muted-foreground'>NONE</span>
+                      )}
+                    </div>
                   </Button>
                 </div>
               )

ui/components/ui/kbd.tsx

@@ -0,0 +1,21 @@
+import * as React from 'react'
+
+import { cn } from '@/lib/utils'
+
+export interface KbdProps extends React.HTMLAttributes<HTMLElement> {}
+
+const Kbd = React.forwardRef<HTMLElement, KbdProps>(({ className, ...props }, ref) => {
+  return (
+    <kbd
+      ref={ref}
+      className={cn(
+        'pointer-events-none inline-flex h-5 min-w-[1.25rem] items-center justify-center rounded border border-b-2 bg-muted px-1.5 font-mono text-[10px] leading-none font-medium text-muted-foreground opacity-100 shadow-sm transition-all duration-200 select-none',
+        className,
+      )}
+      {...props}
+    />
+  )
+})
+Kbd.displayName = 'Kbd'
+
+export { Kbd }

ui/hooks/useKeyboardShortcuts.ts

@@ -10,24 +10,47 @@ import { usePreferencesStore } from '@/lib/stores/preferencesStore'
 export function useKeyboardShortcuts() {
   const setMode = useEditorUiStore((state) => state.setMode)
   const setBrushConfig = usePreferencesStore((state) => state.setBrushConfig)
+  const shortcuts = usePreferencesStore((state) => state.shortcuts)
   const isMac = useMemo(() => getPlatform() === 'mac', [])
 
+  // Optimized tool mapping - built once and updated only when shortcuts change
+  const TOOL_MAP = useMemo(
+    (): Record<string, import('@/lib/types').ToolMode> => ({
+      [shortcuts.select]: 'select',
+      [shortcuts.block]: 'block',
+      [shortcuts.brush]: 'brush',
+      [shortcuts.eraser]: 'eraser',
+      [shortcuts.repairBrush]: 'repairBrush',
+    }),
+    [shortcuts],
+  )
+
   useEffect(() => {
     const handleKeyDown = (event: KeyboardEvent) => {
-      const target = event.target as HTMLElement
+      const target = event.target
       const inTextField =
-        target.tagName === 'INPUT' || target.tagName === 'TEXTAREA' || target.isContentEditable
+        target instanceof HTMLElement &&
+        (target.tagName === 'INPUT' || target.tagName === 'TEXTAREA' || target.isContentEditable)
 
-      // Undo / Redo — work globally, including from within text fields (the
-      // browser's native `z` on an input would only undo keystrokes, not
-      // scene ops). Cmd on macOS, Ctrl elsewhere.
+      // Undo / Redo — these work globally, including from within text fields,
+      // as scene-level history should usually take precedence over native
+      // browser text-undo.
+      const shortcut = formatShortcut(event, isMac)
       const mod = isMac ? event.metaKey : event.ctrlKey
-      if (mod && (event.key === 'z' || event.key === 'Z')) {
+
+      if (shortcut === shortcuts.undo) {
         event.preventDefault()
-        if (event.shiftKey) void redoOp()
-        else void undoOp()
+        void undoOp()
         return
       }
+
+      if (shortcut === shortcuts.redo) {
+        event.preventDefault()
+        void redoOp()
+        return
+      }
+
+      // Legacy fallback: Redo on Ctrl+Y / Cmd+Y
       if (mod && (event.key === 'y' || event.key === 'Y')) {
         event.preventDefault()
         void redoOp()
@@ -50,27 +73,15 @@ export function useKeyboardShortcuts() {
         return
       }
 
-      const shortcut = formatShortcut(event, isMac)
-      if (!shortcut) return
-
-      // Pull latest shortcuts from store to avoid re-binding this listener
-      const shortcuts = usePreferencesStore.getState().shortcuts
-
-      // Tool Switching
-      if (shortcut === shortcuts.select) {
-        setMode('select')
-      } else if (shortcut === shortcuts.block) {
-        setMode('block')
-      } else if (shortcut === shortcuts.brush) {
-        setMode('brush')
-      } else if (shortcut === shortcuts.eraser) {
-        setMode('eraser')
-      } else if (shortcut === shortcuts.repairBrush) {
-        setMode('repairBrush')
+      // Tool Switching - O(1) direct matching
+      const matchingTool = shortcut ? TOOL_MAP[shortcut] : undefined
+      if (matchingTool) {
+        setMode(matchingTool)
+        return
       }
 
       // Brush Size
-      else if (shortcut === shortcuts.increaseBrushSize) {
+      if (shortcut === shortcuts.increaseBrushSize) {
         const currentSize = usePreferencesStore.getState().brushConfig.size
         setBrushConfig({ size: Math.min(128, currentSize + 4) })
       } else if (shortcut === shortcuts.decreaseBrushSize) {
@@ -82,5 +93,5 @@ export function useKeyboardShortcuts() {
     window.addEventListener('keydown', handleKeyDown)
     return () => window.removeEventListener('keydown', handleKeyDown)
     // eslint-disable-next-line react-hooks/exhaustive-deps
-  }, [isMac, setMode])
+  }, [isMac, setMode, TOOL_MAP, shortcuts])
 }

ui/lib/shortcutUtils.ts

@@ -37,6 +37,18 @@ const MODIFIER_NAMES = new Set([
   'Command',
 ])
 
+/**
+ * Generates a string representing only the modifiers currently held.
+ */
+export function formatModifierCombination(event: ShortcutEvent, isMac: boolean): string {
+  const parts: string[] = []
+  if (event.ctrlKey) parts.push('Ctrl')
+  if (event.altKey) parts.push(isMac ? 'Opt' : 'Alt')
+  if (event.shiftKey) parts.push('Shift')
+  if (event.metaKey) parts.push(isMac ? 'Cmd' : 'Win')
+  return parts.join('+')
+}
+
 /**
  * Generates a standardized shortcut string from a keyboard event.
  * Format: [Ctrl+][Alt/Opt+][Shift+][Cmd/Win+]Key
@@ -58,7 +70,8 @@ export function formatShortcut(event: ShortcutEvent, isMac: boolean): string {
   }
 
   // Standardize single characters to uppercase (V, B, [)
-  const displayKey = key.length === 1 ? key.toUpperCase() : key
+  let displayKey = key.length === 1 ? key.toUpperCase() : key
+  if (displayKey === ' ') displayKey = 'Space'
   parts.push(displayKey)
 
   return parts.join('+')
@@ -108,3 +121,36 @@ export function areShortcutsEqual(a: Record<string, string>, b: Record<string, s
 export function isModifierKey(key: string): boolean {
   return MODIFIER_NAMES.has(key)
 }
+
+/**
+ * Formats a standardized shortcut string (e.g., 'Cmd+Shift+Z') for UI display.
+ * On Mac, it uses standard symbols (⌘, ⇧, ⌥, ⌃).
+ * On other platforms, it returns the string as-is with '+' separators.
+ */
+export function formatShortcutForDisplay(shortcut: string, isMac: boolean): string {
+  if (!shortcut) return ''
+
+  const parts = shortcut.split('+')
+  if (!isMac) return parts.join('+')
+
+  // Mac symbol mapping
+  const symbols: Record<string, string> = {
+    Ctrl: '⌃',
+    Opt: '⌥',
+    Shift: '⇧',
+    Cmd: '⌘',
+  }
+
+  // Map parts to symbols if they exist, otherwise keep the key as is
+  const mappedParts = parts.map((part) => symbols[part] || part)
+
+  // Standard Mac menu order: Control, Option, Shift, Command
+  // We sort based on this order for consistency in the UI
+  const ORDER = ['⌃', '⌥', '⇧', '⌘']
+  const modifiers = mappedParts.filter((p) => ORDER.includes(p))
+  const key = mappedParts.find((p) => !ORDER.includes(p))
+
+  modifiers.sort((a, b) => ORDER.indexOf(a) - ORDER.indexOf(b))
+
+  return modifiers.join('') + (key || '')
+}

ui/lib/stores/preferencesStore.ts

@@ -3,6 +3,8 @@
 import { create } from 'zustand'
 import { persist } from 'zustand/middleware'
 
+import { getPlatform } from '@/lib/shortcutUtils'
+
 type PreferencesState = {
   brushConfig: {
     size: number
@@ -21,6 +23,8 @@ type PreferencesState = {
     repairBrush: string
     increaseBrushSize: string
     decreaseBrushSize: string
+    undo: string
+    redo: string
   }
   setShortcuts: (shortcuts: Partial<PreferencesState['shortcuts']>) => void
   resetShortcuts: () => void
@@ -40,6 +44,8 @@ const initialPreferences = {
     repairBrush: 'R',
     increaseBrushSize: ']',
     decreaseBrushSize: '[',
+    undo: getPlatform() === 'mac' ? 'Cmd+Z' : 'Ctrl+Z',
+    redo: getPlatform() === 'mac' ? 'Cmd+Shift+Z' : 'Ctrl+Shift+Z',
   },
 }
 
@@ -73,7 +79,7 @@ export const usePreferencesStore = create<PreferencesState>()(
     }),
     {
       name: 'koharu-config',
-      version: 4,
+      version: 5,
       migrate: (persisted: any, version: number) => {
         if (version < 2 && persisted) {
           delete persisted.localLlm
@@ -92,6 +98,15 @@ export const usePreferencesStore = create<PreferencesState>()(
             }
           }
         }
+        if (version < 5 && persisted?.shortcuts) {
+          const isMac = getPlatform() === 'mac'
+          if (!persisted.shortcuts.undo) {
+            persisted.shortcuts.undo = isMac ? 'Cmd+Z' : 'Ctrl+Z'
+          }
+          if (!persisted.shortcuts.redo) {
+            persisted.shortcuts.redo = isMac ? 'Cmd+Shift+Z' : 'Ctrl+Shift+Z'
+          }
+        }
         return persisted
       },
       partialize: (state) => ({