Skip to content

feat(crosshairs): add hold key for temp crosshairs tool#743

Merged
floryst merged 1 commit intoKitware:mainfrom
PaulHax:crosshairs-toggle
May 27, 2025
Merged

feat(crosshairs): add hold key for temp crosshairs tool#743
floryst merged 1 commit intoKitware:mainfrom
PaulHax:crosshairs-toggle

Conversation

@PaulHax
Copy link
Collaborator

@PaulHax PaulHax commented May 21, 2025

No description provided.

@netlify
Copy link

netlify bot commented May 21, 2025

Deploy Preview for volview-dev ready!

Built without sensitive environment variables

Name Link
🔨 Latest commit da5eba0
🔍 Latest deploy log https://app.netlify.com/projects/volview-dev/deploys/6835d99a649c720008e7fcc7
😎 Deploy Preview https://deploy-preview-743--volview-dev.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

@PaulHax PaulHax requested a review from floryst May 21, 2025 22:04
@PaulHax PaulHax force-pushed the crosshairs-toggle branch 2 times, most recently from 370345e to 85d52db Compare May 21, 2025 22:05
@PaulHax PaulHax force-pushed the crosshairs-toggle branch from 85d52db to 6544a11 Compare May 22, 2025 20:01
@floryst
Copy link
Contributor

floryst commented May 26, 2025

I forgot to submit my review.

I'm wondering if we should special-case disable the temp crosshairs when in crosshairs mode. Nothing breaks, so I'm fine leaving it as-is.

@PaulHax PaulHax force-pushed the crosshairs-toggle branch from 6544a11 to cbc586e Compare May 27, 2025 15:23
@PaulHax
Copy link
Collaborator Author

PaulHax commented May 27, 2025

I'm wondering if we should special-case disable the temp crosshairs when in crosshairs mode. Nothing breaks, so I'm fine leaving it as-is.

There was a subtle bug here (dragging was stuck on if crosshairs was already selected and temp crosshairs was toggled). Also a bug with pressing 'p' twice for paint. So added a check to do nothing if tool already active in store/tools/index.ts

@PaulHax PaulHax force-pushed the crosshairs-toggle branch from cbc586e to da5eba0 Compare May 27, 2025 15:26
Copy link
Contributor

@floryst floryst left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@floryst floryst added this pull request to the merge queue May 27, 2025
Merged via the queue into Kitware:main with commit 1d78dab May 27, 2025
9 of 11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants