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

Annotations: Change remove button to a close button #84

Merged

Conversation

martinRenou
Copy link
Member

Fix #63

We don't see the mouse in the screencast, I'm:

  • Opening the annotation in the 3D view using the circle handler
  • Closing it with the close button (the cross)
  • Reopening it
  • Removing it with the delete button (the bin)
Screencast.from.2023-01-02.17-50-28.webm

@github-actions
Copy link
Contributor

github-actions bot commented Jan 2, 2023

Binder 👈 Launch a Binder on branch martinRenou/jupytercad/improve_open_close_remove_annotation

@trungleduc
Copy link
Member

It works great! Thanks @martinRenou. Since I'm a fan of Windows 😃 , I'm feeling more consistent using the bar as the minimize icon and keeping the cross as the delete icon. What do you think?

icon

@martinRenou
Copy link
Member Author

I'm feeling more consistent using the bar as the minimize icon and keeping the cross as the delete icon. What do you think?

It's indeed more consistent with the cross for removing the 3D objects in the tree view 👍🏽

I'd be more confident with a dialog asking for delete confirmation if we use this UI. With the previous implementation my first impression was that the cross would just close the annotation and allow me to show it again later, not delete it, a delete confirmation prompt would have helped me.

Did you change the icons locally? Could you push it as a commit on my branch?

@trungleduc trungleduc force-pushed the improve_open_close_remove_annotation branch from bec3907 to 5cc2eaf Compare January 3, 2023 14:10
@trungleduc trungleduc merged commit c26dbcf into jupytercad:main Jan 3, 2023
@martinRenou martinRenou deleted the improve_open_close_remove_annotation branch January 3, 2023 15:48
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.

Make annotation deletion more explicit
2 participants