Merge pull request #190 from Pouzor/fix/save-button-event-arg

fix(canvas): Save button no-op due to leaked click event

frontend/src/components/panels/Sidebar.tsx

@@ -257,7 +257,7 @@ export function Sidebar({ onAddNode, onAddGroupRect, onAddText, onScan, onZigbee
           icon={Save}
           label="Save Canvas"
           collapsed={collapsed}
-          onClick={onSave}
+          onClick={() => onSave()}
           badge={hasUnsavedChanges}
           accent
         />

frontend/src/components/panels/Toolbar.tsx

@@ -95,7 +95,7 @@ export function Toolbar({ onSave, onAutoLayout, onExport, onChangeStyle, onUndo,
           background: hasUnsavedChanges ? '#00d4ff' : undefined,
           color: hasUnsavedChanges ? '#0d1117' : undefined,
         }}
-        onClick={onSave}
+        onClick={() => onSave()}
       >
         {hasUnsavedChanges && (
           <span className="absolute -top-1 -right-1 w-2 h-2 rounded-full bg-[#e3b341] border border-[#161b22]" />

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

@@ -188,6 +188,15 @@ describe('Sidebar', () => {
     expect(defaultProps.onSave).toHaveBeenCalledOnce()
   })
 
+  // Regression (#186): the click handler must not forward the MouseEvent as an
+  // argument — handleSave treats its first arg as a designIdOverride, so leaking
+  // the event corrupts design_id and the save silently fails.
+  it('calls onSave with no arguments (does not leak the click event)', () => {
+    render(<Sidebar {...defaultProps} />)
+    fireEvent.click(screen.getByText('Save Canvas'))
+    expect(defaultProps.onSave).toHaveBeenCalledWith()
+  })
+
   it('calls onOpenSettings when Settings is clicked', () => {
     render(<Sidebar {...defaultProps} />)
     fireEvent.click(screen.getByText('Settings'))

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

@@ -0,0 +1,59 @@
+import { describe, it, expect, beforeEach, vi } from 'vitest'
+import { render, screen, fireEvent } from '@testing-library/react'
+import { Toolbar } from '../Toolbar'
+import { useCanvasStore } from '@/stores/canvasStore'
+
+// ── Mocks ────────────────────────────────────────────────────────────────────
+
+vi.mock('@/stores/canvasStore')
+
+vi.mock('@/components/ui/Logo', () => ({
+  Logo: () => <div data-testid="logo" />,
+}))
+
+function mockStore(overrides: Partial<ReturnType<typeof useCanvasStore>> = {}) {
+  vi.mocked(useCanvasStore).mockReturnValue({
+    hasUnsavedChanges: false,
+    past: [],
+    future: [],
+    ...overrides,
+  } as ReturnType<typeof useCanvasStore>)
+}
+
+const defaultProps = {
+  onSave: vi.fn(),
+  onAutoLayout: vi.fn(),
+  onExport: vi.fn(),
+  onChangeStyle: vi.fn(),
+  onUndo: vi.fn(),
+  onRedo: vi.fn(),
+  onShortcuts: vi.fn(),
+  onExportMd: vi.fn(),
+  onExportYaml: vi.fn(),
+  onImportYaml: vi.fn(),
+  onViewOnly: vi.fn(),
+}
+
+// ── Tests ─────────────────────────────────────────────────────────────────────
+
+describe('Toolbar', () => {
+  beforeEach(() => {
+    mockStore()
+    vi.clearAllMocks()
+  })
+
+  it('calls onSave when Save is clicked', () => {
+    render(<Toolbar {...defaultProps} />)
+    fireEvent.click(screen.getByText('Save'))
+    expect(defaultProps.onSave).toHaveBeenCalledOnce()
+  })
+
+  // Regression (#186): the click handler must not forward the MouseEvent as an
+  // argument — handleSave treats its first arg as a designIdOverride, so leaking
+  // the event corrupts design_id and the save silently fails.
+  it('calls onSave with no arguments (does not leak the click event)', () => {
+    render(<Toolbar {...defaultProps} />)
+    fireEvent.click(screen.getByText('Save'))
+    expect(defaultProps.onSave).toHaveBeenCalledWith()
+  })
+})