Skip to content
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

Annotation Tool Hover #404

Merged
merged 15 commits into from
Sep 18, 2023
Merged

Annotation Tool Hover #404

merged 15 commits into from
Sep 18, 2023

Conversation

PaulHax
Copy link
Collaborator

@PaulHax PaulHax commented Sep 5, 2023

Hover popup appears 250ms after cursor rests on invisible vtkWidgetRepresentation.

Current look:
image

Old look:
image

@netlify
Copy link

netlify bot commented Sep 5, 2023

Deploy Preview for volview-dev ready!

Built without sensitive environment variables

Name Link
🔨 Latest commit 43be39a
🔍 Latest deploy log https://app.netlify.com/sites/volview-dev/deploys/6508b210b71b6400087c0d06
😎 Deploy Preview https://deploy-preview-404--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 site configuration.

@floryst
Copy link
Collaborator

floryst commented Sep 5, 2023

  • Maybe we should switch from a bounding box test to picking the closest tool. I cannot see the label for a polygon inside another polygon.
  • Having the label be associated with the mouse position seems a little distracting. What do you think about a fixed offset from the tool's bounding box?

@PaulHax
Copy link
Collaborator Author

PaulHax commented Sep 5, 2023

Maybe we should switch from a bounding box test to picking the closest tool. I cannot see the label for a polygon inside another polygon.

I'll try getting distances to each line to sort among tools the pointer is inside. Getting fancy. First pass I had earler was to show popover when hovering over handles or lines, same target area as context menu. Was probably going to do that approach that for Ruler, as bounding box could be a bit much for the diagonal ruler? Maybe we go with that hardware picker approach for all tools for now and refine our "2D picking" approach later?

Having the label be associated with the mouse position seems a little distracting. What do you think about a fixed offset from the tool's bounding box?

Argeed. Can't escape that needy popover. 🤣 Did not do the offset from bounding box cuz if camera is zoomed in and annotation bounds is out of frame, won't see popover. Was common situation in catheter based modality project i worked on. How about a debouce on mouse movement before showing popover?

@PaulHax
Copy link
Collaborator Author

PaulHax commented Sep 5, 2023

Thanks for checking on this early. Good points!

@PaulHax PaulHax changed the title Tool hover Annotation Tool Hover Sep 15, 2023
@PaulHax PaulHax marked this pull request as ready for review September 15, 2023 14:26
Copy link
Collaborator

@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.

Looks good so far! I presume you are going to update this for the rectangle and ruler?

src/components/tools/AnnotationInfo.vue Outdated Show resolved Hide resolved
src/components/tools/BoundingRectangle.vue Outdated Show resolved Hide resolved
src/components/tools/polygon/PolygonTool.vue Outdated Show resolved Hide resolved
src/vtk/PolygonWidget/state.ts Outdated Show resolved Hide resolved
@floryst
Copy link
Collaborator

floryst commented Sep 15, 2023

One other thing: we will do some user testing to see how people feel about the delay in popup. I don't have a better idea at the moment.

Remove bounds changes to Polygon state.

Remove array for watch.
@PaulHax
Copy link
Collaborator Author

PaulHax commented Sep 18, 2023

Added hover support to Ruler and Rectangle.

Yes, hover feel is not great. But do need something to get "Label" of annnotation ASAP. Alternatives:

  • Could enable hover popup with toggle switch. (inspect mode?) Maybe no deboounce if we did this.
  • Inspect tool?
  • Right click context menu shows label type.

@floryst
Copy link
Collaborator

floryst commented Sep 18, 2023

I like the right click context menu approach the most, especially since I have some half-written code snippet somewhere that allows you to switch the label of an annotation via the context menu.

The select tool I plan on working on can be the inspect tool you're referring to. In select mode, hovering can show the label (debounced or not).

@floryst
Copy link
Collaborator

floryst commented Sep 18, 2023

Overall, LGTM. The icon spacing for the context menu seems kind of big, which makes entries without icons (e.g. the polygon tool's "Delete Point") look out of place.

@PaulHax
Copy link
Collaborator Author

PaulHax commented Sep 18, 2023

Added blank icon to fix "Delete Point".

Added a "Select" tool that enables the hover popup.

Included usePopperState, thanks!

@floryst
Copy link
Collaborator

floryst commented Sep 18, 2023

LGTM!

@floryst floryst merged commit 58d215f into Kitware:main Sep 18, 2023
7 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