Reimplement peek behaviour
What does this MR do and why?
Reimplement peek behaviour
This reimplements the sidebar peek behaviour that was originally added in !116914 (merged).
This implementation uses a document-wide mousemove
listener, whereas
the previous implementation relied on the mouseenter/leave
events
fired on the sidebar element itself and a small hover area.
The old approach
The main problem with the previous implementation was that sometimes the sidebar would oscillate between peeked open and closed if the cursor was stationary.
This was sometimes easy to reproduce, and sometimes hard. The reason comes down to a race between the JavaScript timer and the CSS transition duration.
The UI Events specification says of the mouseenter
event:
A user agent MUST also dispatch this event when the element or one of its descendants moves to be underneath the primary pointing device.
It turns out that browsers do not obey this as you might expect on elements that are undergoing transitions under a stationary cursor (I suspect this applies to animations as well). Presumably this is for performance reasons.
What they actually do in this case is dispatch the event approximately after the transition has ended. Each browser does this slightly differently in my testing, and none of them seem deterministic in exactly when the event is dispatched.
This means that, if the sidebar's opening transition is cancelled before
it is complete (e.g., the sidebar instead starts its closing transition)
the mouseenter
does not get dispatched even if the transition did move
the element underneath the stationary cursor.
If the cursor is moving at the same time as the transitioning element, the event is dispatched as expected on the first frame the cursor enters it.
So, the race was between the opening sidebar transition and the closing
timer. If the transition ended early enough, the mouseenter
event
would be dispatched, and the bug wouldn't happen. If the cursor moved
during the transition and was over the transitioning sidebar, the
mouseenter
event would be dispatched, as expected.
The bug occurred only when the cursor was stationary, and the opening
transition took too long, causing it to be cancelled by the closing
transition, meaning no mouseenter
event was dispatched to keep the
sidebar open.
The new approach
The approach taken here is to rely on a document-wide mousemove
listener. This has two benefits:
- The bug with the previous approach is avoided since no DOM is involved in determining where the cursor is.
- Additional regions are trivial to add, for instance, moving the cursor far enough away instantly closes the sidebar, rather than having to wait for the closing timer. It is possible to other regions, for instance, one which shows a hint or affordance of the peek behaviour, but does not start the timer to open the sidebar (this is not implemented here, but is trivial to do so).
It does have one drawback, which is that it's not coupled to the DOM. This means that if the sidebar changes width, or has some descendant that overflows the sidebar, the sidebar might get closed even though the cursor is over it. This could be avoided in a few ways, but isn't done here.
This also formalises and extracts the peek state machine logic into its own Vue component. This means that the document event listener is guaranteed to be added and cleaned up when switching between toggled open/closed states. The timers are also encapsulated within that component, and the whole thing can be more thoroughly tested in isolation.
stateDiagram-v2
[*] --> STATE_CLOSED
STATE_CLOSED --> STATE_WILL_OPEN : x < 8px
STATE_WILL_OPEN --> STATE_OPEN : after 200ms
STATE_WILL_OPEN --> STATE_CLOSED : x >= 8px
STATE_WILL_OPEN --> STATE_CLOSED : cursor leaves window
STATE_OPEN --> STATE_WILL_CLOSE : 8px <= x < 2 * sidebar_width
STATE_OPEN --> STATE_WILL_CLOSE : cursor leaves window
STATE_WILL_CLOSE --> STATE_CLOSED : after 500ms
STATE_OPEN --> STATE_CLOSED : x >= 2 * sidebar_width
Iterating on regions/timings
This MR hard codes a particular set of regions and timings. We might want to tweak these in future, and to make that a bit easier, I have a branch based in an earlier draft of this which adds some controls to change these variables in real time:
simplescreenrecorder-2023-04-28_18.14.43
To try this out:
git fetch origin dev-fix-sidebar-peek
git checkout dev-fix-sidebar-peek
This test branch also adds another region, as suggested by @aregnery, which triggers the hint/affordance, but doesn't initiate the opening timer. Here's the state chart for this branch:
stateDiagram-v2
[*] --> STATE_CLOSED
STATE_CLOSED --> STATE_WILL_OPEN : x < 8px
STATE_WILL_OPEN --> STATE_OPEN : after 200ms
STATE_WILL_OPEN --> STATE_CLOSED : x >= 16px
STATE_WILL_OPEN --> STATE_CLOSED : cursor leaves window
STATE_OPEN --> STATE_WILL_CLOSE : 16px <= x < 2 * sidebar_width
STATE_OPEN --> STATE_WILL_CLOSE : cursor leaves window
STATE_WILL_CLOSE --> STATE_CLOSED : after 500ms
STATE_OPEN --> STATE_CLOSED : x >= 2 * sidebar_width
STATE_CLOSED --> STATE_HINT : 8px <= x < 16px
STATE_WILL_OPEN --> STATE_HINT : 8px <= x < 16px
STATE_HINT --> STATE_WILL_OPEN : x < 8px
STATE_HINT --> STATE_CLOSED : x >= 16px
Screenshots or screen recordings
header | header |
---|---|
simplescreenrecorder-2023-04-28_18.02.45 | simplescreenrecorder-2023-04-27_15.24.17 |
How to set up and validate locally
- Enable the sidebar.
- Collapse the sidebar either with the keyboard shortcut Ctrl+\ or ⌘+\, or with the button to the left of the page breadcrumbs.
- Move the cursor around!
MR acceptance checklist
This checklist encourages us to confirm any changes have been analyzed to reduce risks in quality, performance, reliability, security, and maintainability.
-
I have evaluated the MR acceptance checklist for this MR.