Skip to content

Keyboard handling overhaul: Cmd/Ctrl shortcuts trigger tools, single keys mutate BOTH panels' calibration, Esc collisions, pen-undo ordering race #94

Description

@dompm

Both panels register window keydown handlers (ResultPanel.tsx:922-972, SheetPanel.tsx:196-220) with structural problems that produce data corruption from ordinary keystrokes:

1. No modifier guards

Handlers check bare e.key with no metaKey/ctrlKey/altKey checks:

  • Cmd/Ctrl+C (copy) → both panels switch to crop mode. Cmd/Ctrl+V → select tool. Cmd/Ctrl+S → ResultPanel toggles refine-remove mode (ResultPanel.tsx:946) and the browser Save dialog opens (no preventDefault).

2. Shortcuts fire into both panels — and fabricate calibration data

Neither handler scopes to the hovered/focused panel. Pressing M activates measure on both canvases, and handleToolChange('measure') writes data on both sides: a default 6-inch scale on the pattern (ResultPanel.tsx:1358-1365) and on the active sheet (SheetPanel.tsx:399-402). One keystroke intended for one panel silently fabricates a calibration for the other — and a wrong sheet scale corrupts every downstream real-world size computation.

Fix: a single shared useCanvasShortcuts(panel) hook with modifier guards and hover/focus scoping (see the ResultPanel-split refactor issue).

3. Pen-tool Cmd+Z race: one keystroke, two destructive actions

ResultPanel.tsx:952-958 relies on e.stopImmediatePropagation() to block App's global undo — which only works if ResultPanel's listener registered before App's. But App re-registers on selectedPieceIds/undo/redo changes (App.tsx:711 deps) and ResultPanel re-registers on activeTool change (ResultPanel.tsx:990); window listeners run in registration order. Switch to the pen tool (listener now last), place vertices, press Cmd+Z → App's handler runs first and reverts the last committed project op (e.g. deletes the previously created piece), then ResultPanel pops the pen vertex.

4. Esc collisions with modals

No Esc handler stops propagation and panels don't know a modal is open (ResultPanel.tsx:959-972, ShortcutsOverlay.tsx:67-77, MoveConfirmDialog.tsx:17-24). Mid-pen-drawing, open the shortcuts overlay (?), press Esc to close it → ResultPanel discards the in-progress polygon and SheetPanel resets its tool.

5. Delete inside the sheet <select> deletes the piece

App's global guard (App.tsx:678) excludes only HTMLInputElement/HTMLTextAreaElement — not HTMLSelectElement (the panels' own guards do, e.g. ResultPanel.tsx:923). Focus the "Sheet" dropdown in the piece popover, press Delete/Backspace → deletePieces(selectedPieceIds) deletes the piece being edited.

6. Stale closures in the keydown effect

Deps are [activeTool] only (ResultPanel.tsx:990): updateHoverPoint reads unrolledLamp state directly at :804 (inconsistent with the ref at :719), so Shift re-snap uses stale lamp geometry after a profile edit; 'b' at :939 checks a stale isEncoding; keyboard handleToolChange('m') reads a stale project.patternCrop. Related: App's keyboard effect (App.tsx:711) lists deletePiece in deps but uses deletePieces, which closes over isSymmetryEnabled — toggle symmetry then press Delete and the stale handler applies the old symmetry mode.

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions