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

Context menu on samples list stays open #1304

Closed
axelboc opened this issue Jul 11, 2024 · 6 comments
Closed

Context menu on samples list stays open #1304

axelboc opened this issue Jul 11, 2024 · 6 comments

Comments

@axelboc
Copy link
Collaborator

axelboc commented Jul 11, 2024

This is really unexpected and quite annoying especially since it stays open even after navigating away and back to the samples list page.

Peek 2024-07-11 09-40

@marcus-oscarsson
Copy link
Member

Yes, and its a bit awkward that it is animated from the bottom up in my opinion.

@marcus-oscarsson
Copy link
Member

@jbflo Is it something you think you can have look at when you get some time ?

@walesch-yan
Copy link
Collaborator

The context menu on the canvas element in the data collection section shows the exact same behavior. From my understanding, the only way to close them is by clicking on the element that spawned them (i.e. sample, resp. canvas) or its direct parent element. Clicking anywhere on the screen to close them would be a more intuitive solution.

@jbflo
Copy link
Member

jbflo commented Jul 12, 2024

Maybe we should check again with react-contexify or reat contexMenu that have the necessary events handlers to show or Hide on body click, but in the pass they had some bugs when dealing with long Menu Height. So we had to create this Context Menu component where Show or Hide state are from redux, But forgot to handle the hide when component is Unmounted. Please have a look at the
Related PR for a fix

@axelboc
Copy link
Collaborator Author

axelboc commented Jul 12, 2024

Concerning UX, the context menu should ideally be closed right after selecting a menu item. This is what you expect from a typical OS context menu. Unmount is a good start though, I'm not against merging that to mitigate the issue when navigating away and back.

Concerning implementation, if the underlying library can't handle the above easily out of the box, then the state should ideally be managed locally for efficiency and to avoid cluttering global state. But this could be refactored later on, of course.

@axelboc
Copy link
Collaborator Author

axelboc commented Aug 13, 2024

Resolved in #1321

@axelboc axelboc closed this as completed Aug 13, 2024
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

No branches or pull requests

4 participants