Skip to content

Reimplement peek behaviour

Mark Florian requested to merge fix-sidebar-peek into master

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

  1. Enable the sidebar.
  2. Collapse the sidebar either with the keyboard shortcut Ctrl+\ or +\, or with the button to the left of the page breadcrumbs.
  3. 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.

Edited by Mark Florian

Merge request reports

Loading