document.blockingElements polyfill with inert

[valdrinkoshi]

Implementation of document.blockingElements stack polyfill.

document.$blockingElements manages a stack of elements that inert the interaction outside them.

• the stack can be updated with the methods push(elem), remove(elem), pop(elem)
• the top element (document.$blockingElements.top) is the interactive part of the document
• has(elem) returns if the element is a blocking element

This implementation uses the WICG/inert polyfill to disable interactions on the rest of the document. The algorithm will:

• search for the path of the element to block up to document.body
• set inert to all the siblings of each parent, skipping the parents and the element's distributed content (if any)

Why not listening to events that trigger focus change?

Another approach could be to listen for events that trigger focus change (e.g. focus, blur, keydown) and prevent those if focus moves out of the blocking element (it's the idea behind GoogleChrome/inert-polyfill and iron-overlay-behavior).

Wrapping the focus requires to find all the focusable nodes within the top blocking element, eventually sort by tabindex, in order to find first and last focusable node (e.g. see PR PolymerElements/iron-overlay-behavior#200 which works for ShadowDom v0).

This approach doesn't allow the focus to move outside the window (e.g. to the browser's url bar, dev console if opened, etc.), and is less robust when used with assistive technology (e.g. android talkback allows to move focus with swipe on screen, Apple Voiceover allows to move focus with special keyboard combinations).

Performance

Performance is dependent on the inert polyfill performance. The polyfill tries to invoke inert only if strictly needed (e.g. avoid setting it twice when updating the top blocking element).

At each toggle, scripting + rendering + painting totals to ~50ms (first toggle), ~35ms (next toggles)

The heaviest parts are:

• unconditional paint caused by changes to cursor-event css property (see issue) => once fixed we should gain ~20-25ms
• addition of the inert style nodes in shadow roots (done once for inert element's shadow root, see polyfill's implementation) => can be fixed only by native implementation of inert, should be a gain of at least ~5ms (cost of adding a node)

The results have been obtained by toggling the deepest x-trap-focus inside nested x-b (Chrome v52 stable for MacOs -> http://localhost:8080/components/blockingElements/demo/ce.html?ce=v0)

[cdata]

The tools team is leaning in the direction of only using const on top-level things (modules and symbols, for example), and preferring let for most things that have a relatively confined scope. What do you think of potentially standardizing on this style?

At the risk of starting a 🔥 /cc @justinfagnani @rictic

[justinfagnani]

Leaning, pending final disposition of a gentlemanly duel of ideas between the tools team :)

I personally prefer let for local variables.

[valdrinkoshi]

synced offline, and the agreement is to use const when using a variable that won't be reassigned, and let for variables that will be reused within the scope.

[cdata]

👍

[valdrinkoshi]

@cdata @justinfagnani feedback incorporated, PTAL.

Everyone, I've updated the description of the PR with more info about performance & other options considered, PTAL :)

[justinfagnani]

FYI and in case you care, I'm going to advocate in tools that for Symbols we don't use SCREAMING_CASE, but _privateCamelCase names. I think it'll read better in the class bodies, and I also in general disagree with all caps for const, especially if const is the default.

compare:

BlockingElements[TOP_CHANGED_FN](element, oldTop, this[ALREADY_INERT_ELEMS]);

and

BlockingElements[_topChangedFn](element, oldTop, this[_alreadyInertElements]);

[valdrinkoshi]

You're right, it's less of a punch in the eye! Will update :)

[valdrinkoshi]

@justinfagnani updated the Symbol names to be lower case, much more readable indeed.
I've also removed the all getter in favor of has() method, since I believe the only APIs needed are to know if an element is a blocking element, and if it is the top blocking element.

[valdrinkoshi]

@bicknellr FYI

[bicknellr]

An array might be nicer here; they're already stacks (push, pop, includes, splice(i, 1)) so you wouldn't have to do stuff like line 113.

[bicknellr]

Also, you wouldn't have to track _topElement.

[valdrinkoshi]

It was an Array, but then got converted to Set, see #1 (comment)

[bicknellr]

@cdata @justinfagnani So the collective decision on how to make a stack was to not use the built-in stack? 😆

[valdrinkoshi]

Set is much faster that Array in searching if it contains an element, and that's the main use in here for _blockingElements, did some tests in chrome and there is a x5 - x10 difference code - demo (check the console)

[valdrinkoshi]

ergo: it is painfully slow to compute the last element :x http://jsbin.com/yukizak/3/edit?html,js,output
this needs to be changed

[cdata]

The syntax for this loop might be able to be tightened up a bit?

let current = element;
do {
  // ...
} while (current = current.parentNode || current.host)

[cdata]

Actually, there are multiple conditions, so maybe my proposal would just be uglier..