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

Avoid accidentally closing (sub-)menu too early #1725

Closed
mtlynch opened this issue Jan 22, 2024 · 2 comments · Fixed by #1734
Closed

Avoid accidentally closing (sub-)menu too early #1725

mtlynch opened this issue Jan 22, 2024 · 2 comments · Fixed by #1734
Assignees
Labels
enhancement New feature or request small

Comments

@mtlynch
Copy link
Collaborator

mtlynch commented Jan 22, 2024

One of the things I noticed in a lot of the videos for https://github.com/tiny-pilot/tinypilot-pro/pull/1169 was that there are a lot of instances where Jason was navigating the menu and the menu closed too fast because he moved his cursor just off the menu.

We should explore increasing the timeout so that the menu stays visible for an extra 100-200ms after the cursor leaves the menu area.

@mtlynch mtlynch added enhancement New feature or request small labels Jan 22, 2024
@jotaen4tinypilot
Copy link
Contributor

jotaen4tinypilot commented Jan 23, 2024

There is a closely related usability issue, which likely might (or rather: should) be addressed by the same solution (i.e., the proposed delay).

When hovering over a menu item to open a sub-menu, and then moving the cursor diagonally into the sub-menu, it might occur that the sub-menu closes due to the trajectory of the cursor.

If the cursor follows the red trajectory, it briefly leaves the “Networking” menu item, which causes the sub-menu to close immediately. Only if you follow the green trajectory, the sub-menu stays open.

(Sorry, by the way, for psychoanalyzing you here @jdeanwallace, but I noticed in your screen recordings that you do a very good job to carefully follow that green trajectory 🤣)

Screenshot 2024-01-23 at 19 03 32
Screen.Recording.2024-01-23.at.19.03.38.mov

(The timing in the video is a bit exaggerated.)

@jotaen4tinypilot jotaen4tinypilot changed the title Wait longer to hide the menu when the cursor moves out of it Avoid accidentally closing (sub-)menu too early Jan 23, 2024
@jotaen4tinypilot
Copy link
Contributor

For reference, macOS also uses the delay technique in their native UIs:

Screen.Recording.2024-01-23.at.19.18.39.mov

@jotaen4tinypilot jotaen4tinypilot self-assigned this Jan 31, 2024
jotaen4tinypilot added a commit that referenced this issue Feb 1, 2024
Resolves #1725.

This PR introduces an on-close delay for (sub-)menus, which prevents the
menu from closing too early in case the user moves out their mouse
cursor from the respective menu item.

I’ve described some subtle problems and corner-cases below. Let me know
how the current timing parameters feel for you, so that we can tweak
them if need be.

## Now


https://github.com/tiny-pilot/tinypilot/assets/83721279/6a30adc2-b75d-4f82-a2ca-d078b5378a21

## Before


https://github.com/tiny-pilot/tinypilot/assets/83721279/98e2cf31-c25e-4aa6-bb96-06a866617be2

([Photo of TinyPilot user, captured throughout this screen
recording](https://github.com/tiny-pilot/tinypilot/assets/83721279/29c769ef-bb8e-47af-ab43-2732d091d880))

# Notes

- I don’t think we need `-webkit` and friends anymore for compatibility.
It seems that the `transition` directive should [be supported for long
enough by now](https://caniuse.com/?search=transition).
- I moved the comment about the “System” menu down one line, just above
the respective `<li>` element. I think it should have been there in the
first place.

## Delaying the transition

I slightly delayed the fade-out in the initial part of the transition,
otherwise the menu would start to disappear immediately. This would feel
a bit weird, because if you move the mouse cursor back quickly, the menu
seems to fade in again out of nowhere. The initial delaying alleviates
this effect.

The video below demonstrates the feeling with the default, linear
transition, i.e., without a [`cubic-bezier` easing
function](https://developer.mozilla.org/en-US/docs/Web/CSS/easing-function).
(Alternatively, we could also use a `linear` easing function, but that
still has poorer browser support right now, since it’s newer.)

By the way, see [this tool for a visualisation what `cubic-bezier(0.5,
0, 1, 0.25)` looks like
mathematically](https://cubic-bezier.com/#.5,0,1,.25).


https://github.com/tiny-pilot/tinypilot/assets/83721279/6b419939-e462-4d3d-a1af-17347f04f3f9

## Transition duration

I tried to find a good compromise between keeping the menu open long
enough so that it prevents the accidental closing scenario, yet still
quick enough so that it doesn’t feel sluggish and sticky.

The video below demonstrates the feeling when the transition duration is
too long.


https://github.com/tiny-pilot/tinypilot/assets/83721279/ce2e7b81-b88e-4b5f-806f-156ed15b7d9f

## Flicker prevention

Chrome and Firefox seem to prevent transitions on page load
automatically. Safari (and maybe other browser too) don’t do this,
however, so you would see the menus briefly flickering when loading the
page. Therefore, we have to workaround that with a CSS/JS hack.

I was only able to test on MacOS with Chrome, Firefox and Safari.
Unfortunately, there is always a slight risk with these kind of hacks
that browsers do weird things. This hack technique doesn’t seem uncommon
at least.

The video below demonstrates how it looks without such a workaround (in
Safari).


https://github.com/tiny-pilot/tinypilot/assets/83721279/22535fd5-d5f0-4db0-b444-3052b3ad9fe0

<a data-ca-tag
href="https://codeapprove.com/pr/tiny-pilot/tinypilot/1734"><img
src="https://codeapprove.com/external/github-tag-allbg.png" alt="Review
on CodeApprove" /></a>

---------

Co-authored-by: Jan Heuermann <jan@jotaen.net>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request small
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants