fix(canvas): Save button no-op due to leaked click event
The Toolbar and Sidebar Save buttons wired onClick={onSave} directly, so
React passed the MouseEvent as the first argument. handleSave treats its
first arg as a designIdOverride, corrupting design_id and making the save
fail silently. Ctrl+S worked because it calls handleSave() with no args.
Wrap both handlers as onClick={() => onSave()} so no event leaks through.
Add regression tests asserting onSave is called with zero arguments.
Fixes #186
ha-relevant: no
This commit is contained in:
@@ -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
|
||||
/>
|
||||
|
||||
@@ -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]" />
|
||||
|
||||
@@ -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'))
|
||||
|
||||
@@ -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()
|
||||
})
|
||||
})
|
||||
Reference in New Issue
Block a user