fix: stop showing the grab cursor while a drawing tool is armed - #130
KartikLabhshetwar merged 2 commits into
Conversation
Hovering a shape showed the open-hand cursor whichever tool was armed, because the hover check ran before the tool check. With a drawing tool active a press draws a new shape, so the cursor was promising something the click would not do - most visibly with arrows, where the handles are showing at the same time. The grab cursor is now only offered in Select. A drawing tool keeps the crosshair over an existing shape, which is what the press will actually do. Hit testing also stops running on every mouse-moved event while drawing. This became easy to hit in 0.4.1, when tools stopped falling back to Select after each shape (KartikLabhshetwar#108) - before that the editor was usually in Select by the time the pointer came to rest on the shape just drawn. Closes KartikLabhshetwar#129
|
@zergzorg is attempting to deploy a commit to the knox projects Team on Vercel. A member of the Team first needs to authorize it. |
![tembo[bot]](https://avatars.githubusercontent.com/in/1223475?s=60&v=4){kind=small}
| boundaryFrame: boundaryFrame | ||
| ) != nil | ||
| setCursor(hovering ? .openHand : .arrow) | ||
| } else { |
There was a problem hiding this comment.
One small thing to keep in mind: the Text tool is a bit of a hybrid -- clicking an existing text annotation edits it rather than creating a new one (AnnoEditorInteraction.swift:22). So the placement cursor is slightly misleading when hovering over existing text with the Text tool armed. Not a blocker, just worth noting for the manual verification pass.
There was a problem hiding this comment.
Right, and it cuts the other way: with Text armed the crosshair now promises a new box where a click would actually edit the one under the pointer. Fixed in bda3b07 rather than left to the manual pass — hovering existing text with Text armed shows the I-beam, and everywhere else the tool still creates, so the crosshair stands.
Added a .text case to AnnotationCanvasCursor for it. Text is the only tool with this shape, since every other one goes straight to creating a shape in pointerDown.
…eate Text is the one tool that acts on what it lands on: pointerDown routes a click on an existing text annotation into startEditingText rather than createText. The crosshair from the previous commit is therefore just as misleading there as the grab cursor was, only in the other direction. Hovering existing text with Text armed now shows the I-beam. Everywhere else the tool still creates, so the crosshair stands.
|
Hi @KartikLabhshetwar! Following up on this cursor fix and #132 (optional capture on mouse release). Both release builds pass; the only failing check is Vercel deployment authorization. The Text-tool review feedback here is addressed too. Could you take a look when you have a chance? Happy to address any further feedback. Thanks! |
b0a352d
into
KartikLabhshetwar:main
2 participants
Closes #129.
Problem
Hovering a shape you just drew shows the open-hand "grab" cursor, but pressing draws another shape instead of picking the existing one up. #129 hits it with arrows, where the selection handles are visible at the same time, so everything on screen says grab me and the click does the opposite.
Cause
updateCursortested the hover before it tested the tool (AnnotationCanvas.swift:667):So the grab cursor won over every drawing tool.
pointerDownnever agreed with it: only.selectroutes intobeginSelectInteraction, every other tool goes straight to creating a shape (AnnoEditorInteraction.swift:16-32).The mismatch is older than the report, but it was hard to reach before 0.4.1. Until #108 a tool fell back to Select the moment a shape was committed, so by the time the pointer came to rest on what you had just drawn the editor was usually in Select and the cursor was telling the truth. Now that the tool stays armed, resting on your own work is the normal case.
Change
The grab cursor is offered only in Select. A drawing tool keeps the crosshair over an existing shape, which is what the press will actually do.
Hit testing moves inside the Select branch as a side effect, so
hoveredAnnotationstops running on every mouse-moved event while drawing.Text needed the opposite treatment, which the review caught. It is the one tool that acts on what it lands on:
pointerDownsends a click on existing text intostartEditingText, notcreateText. So the crosshair would have been misleading there in the other direction. Hovering existing text with Text armed now shows the I-beam, via a new.textcase onAnnotationCanvasCursor.I deliberately did not make a drawing tool grab the shape under the pointer instead. That would make it impossible to draw a shape overlapping an existing one — exactly what redaction over a busy screenshot needs — and it is not what Figma, Sketch or Preview do either.
Verification
No Xcode on this machine (Command Line Tools only), so no
make build. Type-checked all ofSources/with the project's own Swift settings fromproject.yml—-swift-version 5 -default-isolation MainActor, the fourSWIFT_APPROACHABLE_CONCURRENCYfeatures,MemberImportVisibility,-target arm64-apple-macosx26.0— against a stub of the one SPM dependency: 0 errors, andmain's existing 127 warnings are byte-identical with and without the patch.Manual pass still needed on a machine with Xcode: