Repository navigation
fix(board): show the hand cursor when panning with Space under any tool - #170
Conversation
The Space and panning cursor rules had the same specificity as the tool cursor rules and came before them, so crosshair or cell won with the sticky, text, shape, frame, connector, pen and eraser tools. Move them after the tool rules. The select tool sets its handle cursor inline, so Space now clears it and select's hover does not set it while Space is held.
|
@gormes-agent review |
ReviewNo blocking code defects found in the four-file diff at The CSS ordering gives Space precedence over tool cursors and panning precedence over Space. Clearing the existing inline cursor on keydown handles a stationary pointer over a selection handle; the hover guard prevents subsequent pointer movement from reinstating it while Space is held. Nonblocking test follow-up
Independently checked
LimitsNo browser computed-style or real keyboard/mouse validation was performed in this review. Go is not installed in this environment, so I did not independently repeat No source changes or merge actions were made. |
oesni
left a comment
oesni
left a comment
There was a problem hiding this comment.
Thanks for your first contribution :)
Could you take a look at the comments?
Removing the inline cursor reset in App.key left both cursor tests passing. The new test compiles App.key from app.js, because importing app.js starts the board, and checks that Space clears a handle cursor without a hover.
|
Thanks for the review. Added 01e8a92: a regression test that starts with an inline handle cursor, sends a Space keydown without a hover, and checks that the cursor clears at once. Removing only |
Problem
Holding Space and dragging pans the board with any tool, but the hand cursor only showed with the select and hand tools.
.layer-dynamic.spaceand.layer-dynamic[data-tool=pen]have the same specificity, and the tool cursor rules came later in app.css, socrosshairorcellwon. With the select tool over a resize or rotate handle,SelectTool.hover()sets an inline cursor, which beats any stylesheet rule.Change
grab) and panning (grabbing) rules now come after the tool cursor rules..layer-dynamic.hover()does not set a handle cursor while Space is held..layer-dynamiccursor rules in app.css. All three fail on main. Importing app.js starts the board, so the keydown test compilesApp.keyfrom the source.The spec (board.md rule 8, board-TECH.md Pan row) defines Space+drag panning but not its cursor, so no spec change.
Verification
go test -count=1 ./internal/board/...passes;TestWebClientran with Node 22..layer-dynamic:Over a selected sticky's resize handle:
nwse-resize, thengrabwith Space, thennwse-resizeagain after release and a pointer move. The image tool was skipped because it returns to select at once. Real keyboard and mouse input was not used.Known limits
setTooldoes not clear an inline cursor, and middle-button pan over a select handle keeps the handle cursor. Both predate this change.Review
Design, implementation and testing were each reviewed by claude-sonnet-5-5 in fresh sessions (author claude-opus-5-5), with snapshots bound by the devreview helper. All verdicts were approve or accept with nonblocking follow-ups, and the approvals were carried to the rebased commit. The keydown test (01e8a92) was added after the bot review; claude-sonnet-5-5 accepted it and carried the implementation approval to that commit. The full task report with verbatim verdicts is local only.