Merge pull request #188 from Pouzor/refactor/settings-modal

refactor(settings): move sidebar settings into a modal

frontend/src/App.tsx

@@ -20,6 +20,7 @@ import { LoginPage } from '@/components/LoginPage'
 import { NodeModal } from '@/components/modals/NodeModal'
 import { EdgeModal } from '@/components/modals/EdgeModal'
 import { ScanConfigModal } from '@/components/modals/ScanConfigModal'
+import { SettingsModal } from '@/components/modals/SettingsModal'
 import { ZigbeeImportModal } from '@/components/zigbee/ZigbeeImportModal'
 import { GroupRectModal, type GroupRectFormData } from '@/components/modals/GroupRectModal'
 import { TextModal, type TextFormData } from '@/components/modals/TextModal'
@@ -69,6 +70,7 @@ export default function App() {
   const [pendingConnection, setPendingConnection] = useState<Connection | null>(null)
   const [editEdgeId, setEditEdgeId] = useState<string | null>(null)
   const [scanConfigOpen, setScanConfigOpen] = useState(false)
+  const [settingsOpen, setSettingsOpen] = useState(false)
   const [exportModalOpen, setExportModalOpen] = useState(false)
   const [zigbeeImportOpen, setZigbeeImportOpen] = useState(false)
 
@@ -608,6 +610,7 @@ export default function App() {
             onScan={() => setScanConfigOpen(true)}
             onZigbeeImport={() => setZigbeeImportOpen(true)}
             onSave={handleSave}
+            onOpenSettings={() => setSettingsOpen(true)}
             forceView={sidebarForceView}
             onOpenPending={openPendingModal}
           />
@@ -805,6 +808,10 @@ export default function App() {
         />
         <ShortcutsModal open={shortcutsOpen} onClose={() => setShortcutsOpen(false)} />
 
+        {!STANDALONE && (
+          <SettingsModal open={settingsOpen} onClose={() => setSettingsOpen(false)} />
+        )}
+
         <PendingDevicesModal
           open={pendingModalOpen}
           onClose={() => setPendingModalOpen(false)}

frontend/src/components/modals/SettingsModal.tsx

@@ -0,0 +1,128 @@
+import { useState, useEffect } from 'react'
+import { Dialog, DialogContent, DialogHeader, DialogTitle, DialogFooter } from '@/components/ui/dialog'
+import { Button } from '@/components/ui/button'
+import { settingsApi } from '@/api/client'
+import { toast } from 'sonner'
+import {
+  type AlignmentSettings,
+  readAlignmentSettings,
+  writeAlignmentSettings,
+  subscribeAlignmentSettings,
+} from '@/utils/alignmentSettings'
+
+interface SettingsModalProps {
+  open: boolean
+  onClose: () => void
+}
+
+export function SettingsModal({ open, onClose }: SettingsModalProps) {
+  const [interval, setIntervalValue] = useState(60)
+  const [saving, setSaving] = useState(false)
+  const [alignment, setAlignment] = useState<AlignmentSettings>(readAlignmentSettings)
+
+  useEffect(() => {
+    if (!open) return
+    settingsApi.get()
+      .then((res) => setIntervalValue(res.data.interval_seconds))
+      .catch(() => {/* use default */})
+  }, [open])
+
+  useEffect(() => subscribeAlignmentSettings(setAlignment), [])
+
+  const updateAlignment = (patch: Partial<AlignmentSettings>) => {
+    const next = { ...alignment, ...patch }
+    setAlignment(next)
+    writeAlignmentSettings(next)
+  }
+
+  const handleSave = async () => {
+    setSaving(true)
+    try {
+      await settingsApi.save({ interval_seconds: interval })
+      toast.success('Settings saved')
+      onClose()
+    } catch {
+      toast.error('Failed to save settings')
+    } finally {
+      setSaving(false)
+    }
+  }
+
+  return (
+    <Dialog open={open} onOpenChange={(v) => !v && onClose()}>
+      <DialogContent className="bg-[#161b22] border-border max-w-md">
+        <DialogHeader>
+          <DialogTitle className="text-foreground">Settings</DialogTitle>
+        </DialogHeader>
+
+        <div className="space-y-5 py-2">
+          {/* Status checker */}
+          <div className="space-y-1.5">
+            <label className="text-xs text-muted-foreground">Status check interval (s)</label>
+            <div className="flex items-center gap-2">
+              <input
+                type="number"
+                min={10}
+                max={3600}
+                value={interval}
+                onChange={(e) => { const v = Number(e.target.value); if (!isNaN(v)) setIntervalValue(v) }}
+                className="w-24 px-2 py-1 rounded-md text-xs font-mono bg-[#0d1117] border border-border text-foreground focus:outline-none focus:border-[#00d4ff]"
+              />
+              <span className="text-xs text-muted-foreground">seconds</span>
+            </div>
+            <p className="text-[10px] text-muted-foreground leading-tight">
+              How often node health is polled (ping, HTTP, SSH…)
+            </p>
+          </div>
+
+          {/* Canvas */}
+          <div className="pt-3 border-t border-border space-y-3">
+            <span className="text-xs font-medium text-muted-foreground uppercase tracking-wider">Canvas</span>
+
+            <label className="flex items-center justify-between gap-2 cursor-pointer">
+              <span className="text-xs text-foreground">Snap to nodes</span>
+              <input
+                type="checkbox"
+                checked={alignment.enabled}
+                onChange={(e) => updateAlignment({ enabled: e.target.checked })}
+                className="cursor-pointer accent-[#00d4ff]"
+                aria-label="Toggle alignment guides"
+              />
+            </label>
+
+            <div className={alignment.enabled ? 'space-y-1.5' : 'space-y-1.5 opacity-50 pointer-events-none'}>
+              <label className="text-xs text-muted-foreground">Snap distance</label>
+              <div className="flex items-center gap-2">
+                <input
+                  type="range"
+                  min={2}
+                  max={16}
+                  step={1}
+                  value={alignment.threshold}
+                  onChange={(e) => updateAlignment({ threshold: Number(e.target.value) })}
+                  className="flex-1 cursor-pointer accent-[#00d4ff]"
+                  aria-label="Alignment snap threshold"
+                />
+                <span className="font-mono text-[11px] text-foreground w-8 text-right">{alignment.threshold}px</span>
+              </div>
+              <p className="text-[10px] text-muted-foreground leading-tight">
+                Distance at which dragged nodes snap to neighbours. Hold Alt while dragging to disable.
+              </p>
+            </div>
+          </div>
+        </div>
+
+        <DialogFooter className="gap-2">
+          <Button variant="ghost" onClick={onClose}>Cancel</Button>
+          <Button
+            onClick={handleSave}
+            disabled={saving}
+            style={{ background: '#00d4ff', color: '#0d1117' }}
+          >
+            {saving ? 'Saving…' : 'Save'}
+          </Button>
+        </DialogFooter>
+      </DialogContent>
+    </Dialog>
+  )
+}

frontend/src/components/modals/__tests__/SettingsModal.test.tsx

@@ -0,0 +1,74 @@
+import { describe, it, expect, vi, beforeEach } from 'vitest'
+import { render, screen, fireEvent, waitFor } from '@testing-library/react'
+import { SettingsModal } from '../SettingsModal'
+
+vi.mock('sonner', () => ({ toast: { success: vi.fn(), error: vi.fn() } }))
+vi.mock('@/api/client', () => ({
+  settingsApi: {
+    get: vi.fn(),
+    save: vi.fn(),
+  },
+}))
+
+import { settingsApi } from '@/api/client'
+import { toast } from 'sonner'
+
+describe('SettingsModal', () => {
+  beforeEach(() => {
+    vi.clearAllMocks()
+    vi.mocked(settingsApi.get).mockResolvedValue({ data: { interval_seconds: 60 } } as never)
+    vi.mocked(settingsApi.save).mockResolvedValue({ data: { interval_seconds: 60 } } as never)
+    vi.mocked(toast.success).mockReset()
+    vi.mocked(toast.error).mockReset()
+  })
+
+  it('loads interval from API when opened', async () => {
+    render(<SettingsModal open onClose={vi.fn()} />)
+    await waitFor(() => expect(settingsApi.get).toHaveBeenCalledOnce())
+    expect(screen.getByText('Status check interval (s)')).toBeDefined()
+  })
+
+  it('does not fetch when closed', () => {
+    render(<SettingsModal open={false} onClose={vi.fn()} />)
+    expect(settingsApi.get).not.toHaveBeenCalled()
+  })
+
+  it('displays interval loaded from API', async () => {
+    vi.mocked(settingsApi.get).mockResolvedValue({ data: { interval_seconds: 120 } } as never)
+    render(<SettingsModal open onClose={vi.fn()} />)
+    expect(await screen.findByDisplayValue('120')).toBeDefined()
+  })
+
+  it('saves interval and closes on Save click', async () => {
+    const onClose = vi.fn()
+    render(<SettingsModal open onClose={onClose} />)
+    const input = await screen.findByDisplayValue('60')
+    fireEvent.change(input, { target: { value: '180' } })
+    fireEvent.click(screen.getByRole('button', { name: 'Save' }))
+    await waitFor(() => {
+      expect(settingsApi.save).toHaveBeenCalledWith({ interval_seconds: 180 })
+      expect(toast.success).toHaveBeenCalledWith('Settings saved')
+      expect(onClose).toHaveBeenCalled()
+    })
+  })
+
+  it('shows error toast and stays open when save fails', async () => {
+    vi.mocked(settingsApi.save).mockRejectedValue(new Error('network'))
+    const onClose = vi.fn()
+    render(<SettingsModal open onClose={onClose} />)
+    await screen.findByDisplayValue('60')
+    fireEvent.click(screen.getByRole('button', { name: 'Save' }))
+    await waitFor(() => {
+      expect(toast.error).toHaveBeenCalledWith('Failed to save settings')
+    })
+    expect(onClose).not.toHaveBeenCalled()
+  })
+
+  it('calls onClose on Cancel', async () => {
+    const onClose = vi.fn()
+    render(<SettingsModal open onClose={onClose} />)
+    await screen.findByDisplayValue('60')
+    fireEvent.click(screen.getByRole('button', { name: 'Cancel' }))
+    expect(onClose).toHaveBeenCalledOnce()
+  })
+})

frontend/src/components/panels/Sidebar.tsx

@@ -5,22 +5,16 @@ import { Tooltip, TooltipContent, TooltipTrigger } from '@/components/ui/tooltip
 import { useCanvasStore } from '@/stores/canvasStore'
 import { useDesignStore } from '@/stores/designStore'
 import { useAuthStore } from '@/stores/authStore'
-import { designsApi, scanApi, settingsApi } from '@/api/client'
+import { designsApi, scanApi } from '@/api/client'
 import { resolveDesignIcon, DEFAULT_DESIGN_ICON } from '@/utils/designIcons'
 import { DesignModal, type DesignFormData } from '@/components/modals/DesignModal'
 import type { Design } from '@/types'
 import { toast } from 'sonner'
 import { useLatestRelease } from '@/hooks/useLatestRelease'
-import {
-  type AlignmentSettings,
-  readAlignmentSettings,
-  writeAlignmentSettings,
-  subscribeAlignmentSettings,
-} from '@/utils/alignmentSettings'
 
 const STANDALONE = import.meta.env.VITE_STANDALONE === 'true'
 
-type SidebarView = 'canvas' | 'history' | 'settings'
+type SidebarView = 'canvas' | 'history'
 
 const PENDING_TRIGGERS: { kind: 'pending' | 'hidden'; icon: typeof ScanLine; label: string }[] = [
   { kind: 'pending', icon: ScanLine, label: 'Pending Devices' },
@@ -45,11 +39,12 @@ interface SidebarProps {
   onScan: () => void
   onZigbeeImport: () => void
   onSave: () => void
+  onOpenSettings: () => void
   forceView?: SidebarView
   onOpenPending: (deviceId?: string, status?: 'pending' | 'hidden') => void
 }
 
-export function Sidebar({ onAddNode, onAddGroupRect, onAddText, onScan, onZigbeeImport, onSave, forceView, onOpenPending }: SidebarProps) {
+export function Sidebar({ onAddNode, onAddGroupRect, onAddText, onScan, onZigbeeImport, onSave, onOpenSettings, forceView, onOpenPending }: SidebarProps) {
   const [collapsed, setCollapsed] = useState(false)
   const [activeView, setActiveView] = useState<SidebarView>(forceView ?? 'canvas')
   const [prevForceView, setPrevForceView] = useState(forceView)
@@ -225,7 +220,6 @@ export function Sidebar({ onAddNode, onAddGroupRect, onAddText, onScan, onZigbee
       {!collapsed && activeView !== 'canvas' && (
         <div className="flex-1 min-h-0 overflow-y-auto border-t border-border">
           {activeView === 'history' && <ScanHistoryPanel />}
-          {activeView === 'settings' && <SettingsPanel />}
         </div>
       )}
 
@@ -279,8 +273,7 @@ export function Sidebar({ onAddNode, onAddGroupRect, onAddText, onScan, onZigbee
             icon={Settings}
             label="Settings"
             collapsed={collapsed}
-            active={activeView === 'settings'}
-            onClick={() => setActiveView((v) => v === 'settings' ? 'canvas' : 'settings')}
+            onClick={onOpenSettings}
           />
         )}
         {!STANDALONE && (
@@ -440,105 +433,6 @@ function ScanHistoryPanel() {
   )
 }
 
-function SettingsPanel() {
-  const [interval, setIntervalValue] = useState(60)
-  const [saving, setSaving] = useState(false)
-  const [alignment, setAlignment] = useState<AlignmentSettings>(readAlignmentSettings)
-
-  useEffect(() => {
-    settingsApi.get()
-      .then((res) => setIntervalValue(res.data.interval_seconds))
-      .catch(() => {/* use default */})
-  }, [])
-
-  useEffect(() => subscribeAlignmentSettings(setAlignment), [])
-
-  const updateAlignment = (patch: Partial<AlignmentSettings>) => {
-    const next = { ...alignment, ...patch }
-    setAlignment(next)
-    writeAlignmentSettings(next)
-  }
-
-  const handleSave = async () => {
-    setSaving(true)
-    try {
-      await settingsApi.save({ interval_seconds: interval })
-      toast.success('Settings saved')
-    } catch {
-      toast.error('Failed to save settings')
-    } finally {
-      setSaving(false)
-    }
-  }
-
-  return (
-    <div className="p-3 space-y-4">
-      <span className="text-xs font-medium text-muted-foreground uppercase tracking-wider">Settings</span>
-
-      <div className="space-y-1.5">
-        <label className="text-xs text-muted-foreground">Status check interval (s)</label>
-        <div className="flex items-center gap-2">
-          <input
-            type="number"
-            min={10}
-            max={3600}
-            value={interval}
-            onChange={(e) => { const v = Number(e.target.value); if (!isNaN(v)) setIntervalValue(v) }}
-            className="w-24 px-2 py-1 rounded-md text-xs font-mono bg-[#0d1117] border border-border text-foreground focus:outline-none focus:border-[#00d4ff]"
-          />
-          <span className="text-xs text-muted-foreground">seconds</span>
-        </div>
-        <p className="text-[10px] text-muted-foreground leading-tight">
-          How often node health is polled (ping, HTTP, SSH…)
-        </p>
-      </div>
-
-      <button
-        onClick={handleSave}
-        disabled={saving}
-        className="w-full py-1.5 rounded-md text-xs font-medium bg-[#00d4ff]/10 text-[#00d4ff] border border-[#00d4ff]/30 hover:bg-[#00d4ff]/20 transition-colors disabled:opacity-50"
-      >
-        {saving ? 'Saving…' : 'Save'}
-      </button>
-
-      <div className="pt-3 border-t border-border space-y-3">
-        <span className="text-xs font-medium text-muted-foreground uppercase tracking-wider">Canvas</span>
-
-        <label className="flex items-center justify-between gap-2 cursor-pointer">
-          <span className="text-xs text-foreground">Snap to nodes</span>
-          <input
-            type="checkbox"
-            checked={alignment.enabled}
-            onChange={(e) => updateAlignment({ enabled: e.target.checked })}
-            className="cursor-pointer accent-[#00d4ff]"
-            aria-label="Toggle alignment guides"
-          />
-        </label>
-
-        <div className={alignment.enabled ? 'space-y-1.5' : 'space-y-1.5 opacity-50 pointer-events-none'}>
-          <label className="text-xs text-muted-foreground">Snap distance</label>
-          <div className="flex items-center gap-2">
-            <input
-              type="range"
-              min={2}
-              max={16}
-              step={1}
-              value={alignment.threshold}
-              onChange={(e) => updateAlignment({ threshold: Number(e.target.value) })}
-              className="flex-1 cursor-pointer accent-[#00d4ff]"
-              aria-label="Alignment snap threshold"
-            />
-            <span className="font-mono text-[11px] text-foreground w-8 text-right">{alignment.threshold}px</span>
-          </div>
-          <p className="text-[10px] text-muted-foreground leading-tight">
-            Distance at which dragged nodes snap to neighbours. Hold Alt while dragging to disable.
-          </p>
-        </div>
-      </div>
-    </div>
-  )
-}
-
 function VersionBadge() {
   const current = __APP_VERSION__
   const { latest, hasUpdate } = useLatestRelease(current)

frontend/src/components/panels/__tests__/SettingsPanel.test.tsx

@@ -1,104 +0,0 @@
-import { describe, it, expect, vi, beforeEach } from 'vitest'
-import { render, screen, fireEvent, waitFor } from '@testing-library/react'
-import { Sidebar } from '../Sidebar'
-import * as canvasStore from '@/stores/canvasStore'
-import { TooltipProvider } from '@/components/ui/tooltip'
-
-vi.mock('@/stores/canvasStore')
-vi.mock('sonner', () => ({ toast: { success: vi.fn(), error: vi.fn() } }))
-vi.mock('@/api/client', () => ({
-  scanApi: {
-    trigger: vi.fn(),
-    pending: vi.fn().mockResolvedValue({ data: [] }),
-    hidden: vi.fn().mockResolvedValue({ data: [] }),
-    runs: vi.fn().mockResolvedValue({ data: [] }),
-    getConfig: vi.fn().mockResolvedValue({ data: { ranges: [] } }),
-  },
-  settingsApi: {
-    get: vi.fn(),
-    save: vi.fn(),
-  },
-}))
-
-import { settingsApi } from '@/api/client'
-import { toast } from 'sonner'
-
-function renderSidebar() {
-  vi.mocked(canvasStore.useCanvasStore).mockReturnValue({
-    nodes: [],
-    hasUnsavedChanges: false,
-    hideIp: false,
-    toggleHideIp: vi.fn(),
-    addNode: vi.fn(),
-    scanEventTs: 0,
-  } as unknown as ReturnType<typeof canvasStore.useCanvasStore>)
-
-  return render(
-    <TooltipProvider>
-      <Sidebar
-        onAddNode={vi.fn()}
-        onAddGroupRect={vi.fn()}
-        onScan={vi.fn()}
-        onSave={vi.fn()}
-        onNodeApproved={vi.fn()}
-      />
-    </TooltipProvider>
-  )
-}
-
-describe('SettingsPanel', () => {
-  beforeEach(() => {
-    vi.mocked(settingsApi.get).mockResolvedValue({ data: { interval_seconds: 60 } } as never)
-    vi.mocked(settingsApi.save).mockResolvedValue({ data: { interval_seconds: 60 } } as never)
-    vi.mocked(toast.success).mockReset()
-    vi.mocked(toast.error).mockReset()
-  })
-
-  it('opens when Settings item is clicked', async () => {
-    renderSidebar()
-    fireEvent.click(screen.getByRole('button', { name: 'Settings' }))
-    await waitFor(() => {
-      expect(settingsApi.get).toHaveBeenCalledOnce()
-    })
-    expect(screen.getByText('Status check interval (s)')).toBeDefined()
-  })
-
-  it('displays interval loaded from API', async () => {
-    vi.mocked(settingsApi.get).mockResolvedValue({ data: { interval_seconds: 120 } } as never)
-    renderSidebar()
-    fireEvent.click(screen.getByRole('button', { name: 'Settings' }))
-    const input = await screen.findByDisplayValue('120')
-    expect(input).toBeDefined()
-  })
-
-  it('saves interval via settingsApi on Save click', async () => {
-    renderSidebar()
-    fireEvent.click(screen.getByRole('button', { name: 'Settings' }))
-    const input = await screen.findByDisplayValue('60')
-    fireEvent.change(input, { target: { value: '180' } })
-    fireEvent.click(screen.getByRole('button', { name: 'Save' }))
-    await waitFor(() => {
-      expect(settingsApi.save).toHaveBeenCalledWith({ interval_seconds: 180 })
-      expect(toast.success).toHaveBeenCalledWith('Settings saved')
-    })
-  })
-
-  it('shows error toast when save fails', async () => {
-    vi.mocked(settingsApi.save).mockRejectedValue(new Error('network'))
-    renderSidebar()
-    fireEvent.click(screen.getByRole('button', { name: 'Settings' }))
-    await screen.findByDisplayValue('60')
-    fireEvent.click(screen.getByRole('button', { name: 'Save' }))
-    await waitFor(() => {
-      expect(toast.error).toHaveBeenCalledWith('Failed to save settings')
-    })
-  })
-
-  it('closes panel when Settings is clicked again', async () => {
-    renderSidebar()
-    fireEvent.click(screen.getByRole('button', { name: 'Settings' }))
-    await screen.findByText('Status check interval (s)')
-    fireEvent.click(screen.getByRole('button', { name: 'Settings' }))
-    expect(screen.queryByText('Status check interval (s)')).toBeNull()
-  })
-})

frontend/src/components/panels/__tests__/Sidebar.test.tsx

@@ -73,6 +73,7 @@ const defaultProps = {
   onScan: vi.fn(),
   onZigbeeImport: vi.fn(),
   onSave: vi.fn(),
+  onOpenSettings: vi.fn(),
   onOpenPending: vi.fn(),
 }
 
@@ -257,14 +258,10 @@ describe('Sidebar', () => {
     await waitFor(() => expect(screen.queryByText('No scans yet')).not.toBeInTheDocument())
   })
 
-  it('toggles Settings panel on Settings click', async () => {
+  it('calls onOpenSettings when Settings is clicked', () => {
     render(<Sidebar {...defaultProps} />)
-    fireEvent.click(screen.getByText('Settings'))
-    await waitFor(() =>
-      expect(screen.getByText('Status check interval (s)')).toBeInTheDocument(),
-    )
     fireEvent.click(screen.getByRole('button', { name: 'Settings' }))
-    expect(screen.queryByText('Status check interval (s)')).not.toBeInTheDocument()
+    expect(defaultProps.onOpenSettings).toHaveBeenCalledOnce()
   })
 
   // ── Logout ─────────────────────────────────────────────────────────────────