-
Notifications
You must be signed in to change notification settings - Fork 61
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Simplify tool code #421
Simplify tool code #421
Conversation
floryst
commented
Sep 15, 2023
- Some code refactoring for easier usage
- useFrameOfReference encapsulates the frame of reference handling
- usePlacingAnnotationTool provides a more readable API for managing the placing tool
- added finishedTools to all annotation tools
- simplified the point visibility code to make it more obvious as to what it's doing
- removed the movePoint state from the polygon store
✅ Deploy Preview for volview-dev ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
I've decided against making one giant composable for the Tool and Widget components. I think it's an unnecessary abstraction to deal with for now if the only goal is code deduplication. Maybe we can revisit it once we've added more tools, as reducing the bundle size may become a driving reason. My primary goal is to ensure readability of the tool logic while keeping flexibility. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good cleanup. Small bug then good by me.
Guess we should remove this too https://github.com/Kitware/VolView/blob/main/src/types/polygon.ts#L11 |
Updated to address comments. |
This state should not be stored inside the tool.
419d964
to
45be61a
Compare