Shadow DOM support #196

cdata · 2019-05-01T00:22:24Z

Shadow DOM support

This change enables custom elements to coordinate with the polyfill in order to apply it to discrete scopes of the composed tree (such as their shadow roots). Highlights include:

The polyfill is now exported to window as a function that can be invoked on any Document|ShadowRoot
The polyfill is automatically, synchronously applied to the top-level document when its script runs, preserving the existing behavior
Code that wishes to use the polyfill in a ShadowRoot can apply it as needed
- Shadow hosts that have the polyfill applied are decorated with a data-js-focus-visible attribute
A focus-visible-polyfill-ready event is now dispatched on window to support lazy loading scenarios (see discussion in Is DOM ready necessary? #193)

Example usage as a mixin

In the course of implementing this proposal, I also crafted a mixin to demonstrate how a custom element should coordinate with the polyfill in order to take advantage of :focus-visible-like capabilities.

I am not proposing to include the mixin in the main project, but it is linked here for future reference:

https://github.com/cdata/focus-visible/blob/fc33395294dd69ad8af70e822e6176a8893816e2/src/custom-element-mixin.js#L1-L49

Compared to #112

There is an older WIP proposal (#112) up for consideration that attempts to add transparent support for arbitrary shadow roots. This approach would ultimately require that focus and blur be observed at every shadow boundary throughout the composed tree in order to work transparently.

The design in the proposal before you expects a Shadow DOM-using component author to opt-in to the polyfill. This allows the polyfill to be applied to Shadow DOM-using components that need it, while avoiding the cost of observing changes in shadow roots that do not know about or do not want to use the polyfill.

Examples

For the purposes of the following examples:

:focus elements are styled with purple color and border-color
:focus-visible elements receive the default focus ring
Elements with shadow roots have delegatesFocus: true configured
Custom elements apply the mixin referenced above to coordinate with the polyfill

This change w/ pointer traversal	This change w/ keyboard traversal

Shadow root styling problem

Without this change, it is not possible to use the polyfill from within a shadow root. In the following example, you can see that the top-level document elements can be styled to hide the focus ring when focusing with pointer events, but focusable elements inside of a shadow root still display a focus ring:

Failure to style shadow root-encapsulated elements

Fixes #193
Fixes #28
Fixes #112

This change enables custom elements to coordinate with the polyfill in order to apply it to discrete scopes of the composed tree (such as shadow roots).

alice · 2019-05-01T03:31:25Z

src/focus-visible.js

-    loaded = false;
-    document.addEventListener('DOMContentLoaded', load, false);
-    window.addEventListener('load', load, false);
+    // We detect that a node is a ShadowRoot by ensuring that it is a


Putting this comment inside the previous if block is breaking my brain a little bit, though I can see why you did. Maybe switch the order of these if/else statements? It would be marginally less efficient for the "normal" case, I guess, but more readable.

Or, perhaps a switch statement over scope.nodeType ?

Thanks @alice I'll try to rework this so that it is less of an eyesore

I switched the order of the conditions around. The comments are hopefully more cleanly formatted now.

alice · 2019-05-01T03:46:58Z

test/specs/shadow-dom.js

+    return matchesMouse(false);
+  });
+
+  describe('focusable elements in shadow', () => {


Seems like this test is failing in CI for some reason.

Sorry about that, will take a look

alice · 2019-05-01T03:49:19Z

Generally looking really good (that gif blew my mind), not sure what's going on with that test though.

alice · 2019-05-08T22:21:01Z

Just had a quick look into the test failure. Looks like it's failing in Firefox due to some interaction between the shady dom polyfill and the focus-visible polyfill.

cdata · 2019-05-12T21:46:03Z

I have addressed the test failures. The following changes were necessary:

In some browsers, we accessed the content of the inline <template> in the Shadow DOM test fixture too early. Deferring until DOMContentLoaded fixed the issue.
I had (mistakenly) assumed that the polyfill did not need to be applied at shadowRoot boundaries when the ShadyCSS polyfill was in effect. But, it turns out the polyfill does encapsulate focus events at the virtual shady root boundaries that it creates, so the :focus-visible polyfill should still be applied as normal.

alice · 2019-05-14T15:06:10Z

Aww, no commit history? We squash merge anyway. I can't tell what's changed since I last looked :\

cdata · 2019-05-14T15:45:20Z

Sorry, I will try to recreate the history

alice · 2019-05-14T16:19:19Z

Thanks and sorry :\

cdata · 2019-05-14T18:01:10Z

Nothing a little git gymnastics couldn't handle 👍

I split the change out into two commits. It looks like the Node.js 10 build had three test timeouts this time around. I'm not sure if that is actionable...

alice · 2019-05-15T09:04:18Z

src/focus-visible.js

-  } else if (scope.nodeType === Node.DOCUMENT_FRAGMENT_NODE && scope.host) {
+  // We detect that a node is a ShadowRoot by ensuring that it is a
+  // DocumentFragment and also has a host property. This check covers native
+  // implementation and polyfill implementation transparently. If we only cared


I like the added counterfactual here, thank you!

alice · 2019-05-15T09:06:24Z

test/fixtures/shadow-dom.html

-      if (self.ShadyCSS == null || self.ShadyCSS.nativeShadow) {
        applyFocusVisiblePolyfill(el.shadowRoot);
-      }
+      }, { once: true });


TIL 😲
Since 2016, even!

alice · 2019-05-15T10:25:13Z

Thanks so much for the git gymnastics! Build problems look like they were an infrastructure problem, it passed once I restarted it.

cdata · 2019-05-15T15:30:15Z

Thanks @alice ! BTW I should note before anyone merges this that due to the fix for #196 this should probably be considered a breaking change (albeit a small one). The timing around which the polyfill activates is different now, so it could break any user who is sensitive to that.

cdata · 2019-05-29T22:05:40Z

@robdodson LMK if I can do anything to help move this one forward!

robdodson

Implementation looks good to me, thank you for writing the tests!

I just had a couple of comments regarding the README.

robdodson · 2019-06-04T20:18:13Z

README.md

+
+```javascript
+// Check for the polyfill:
+if (self.applyFocusVisiblePolyfill != null) {


Is self equivalent to window? Not sure if the snippet makes it clear what context this is running in.

Yeah, self is equivalent to window in the document context. I can change this to window.

robdodson · 2019-06-04T20:20:04Z

README.md

+
+```javascript
+window.addEventListener('focus-visible-polyfill-ready',
+    () => self.applyFocusVisiblePolyfill(myComponent.shadowRoot),


Similar to above, what's the expected value of self in this context?

Will change to window.

robdodson · 2019-06-05T19:34:48Z

I think this is good to merge once chris makes the window changes in the README.

cdata · 2019-06-06T20:18:08Z

@robdodson README has been updated per your request 🍻

jakearchibald · 2019-06-07T08:01:44Z

src/focus-visible.js

+try {
+  event = new CustomEvent('focus-visible-polyfill-ready');
+} catch (error) {
+  // IE11 does not support using CustomEvent as a constructor directly:


Does it support new Event()?

Some very light Googling yields this Stack Overflow that suggests "no" and "use CustomEvent instead" 😄

cdata marked this pull request as ready for review May 1, 2019 01:11

cdata changed the title ~~(WIP) Shadow DOM support~~ Shadow DOM support May 1, 2019

cdata force-pushed the shadow-dom-support branch from 62178c9 to f65c666 Compare May 1, 2019 01:46

Support for custom element shadow roots

ba3bd97

This change enables custom elements to coordinate with the polyfill in order to apply it to discrete scopes of the composed tree (such as shadow roots).

cdata force-pushed the shadow-dom-support branch from f65c666 to ba3bd97 Compare May 1, 2019 01:47

alice reviewed May 1, 2019

View reviewed changes

This was referenced May 2, 2019

Focus ring via :focus-visible google/model-viewer#463

Closed

Is DOM ready necessary? #193

Closed

cdata force-pushed the shadow-dom-support branch from f5b2c46 to 8fe026b Compare May 12, 2019 21:35

cdata force-pushed the shadow-dom-support branch from 8fe026b to 994b3b2 Compare May 13, 2019 00:44

Wait until DOMContentLoaded; better comment format

a3f1f3d

cdata force-pushed the shadow-dom-support branch from 994b3b2 to a3f1f3d Compare May 14, 2019 17:10

alice approved these changes May 15, 2019

View reviewed changes

eps1lon mentioned this pull request May 15, 2019

[test] Colocate shadow root test for focus visible with implementation mui/material-ui#15712

Merged

robdodson self-requested a review June 4, 2019 20:15

robdodson reviewed Jun 4, 2019

View reviewed changes

Change 'self' to 'window' in README

1aa299a

robdodson merged commit 44c0db8 into WICG:master Jun 6, 2019

jakearchibald reviewed Jun 7, 2019

View reviewed changes

cdata mentioned this pull request Jun 25, 2019

Selectively hide focus ring via :focus-visible polyfill support google/model-viewer#660

Merged

Pogodaanton mentioned this pull request Jan 15, 2020

Focus-visible support breaking in newer versions of the polyfill microsoft/fast#2591

Closed

Shadow DOM support #196

Shadow DOM support #196

Uh oh!

Conversation

cdata commented May 1, 2019 • edited by robdodson Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Shadow DOM support

Example usage as a mixin

Compared to #112

Examples

Shadow root styling problem

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

alice commented May 1, 2019

Uh oh!

alice commented May 8, 2019

Uh oh!

cdata commented May 12, 2019

Uh oh!

alice commented May 14, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

cdata commented May 14, 2019

Uh oh!

alice commented May 14, 2019

Uh oh!

cdata commented May 14, 2019

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

alice commented May 15, 2019

Uh oh!

cdata commented May 15, 2019

Uh oh!

cdata commented May 29, 2019

Uh oh!

robdodson left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

robdodson commented Jun 5, 2019

Uh oh!

cdata commented Jun 6, 2019

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

cdata commented May 1, 2019 •

edited by robdodson

Loading

alice commented May 14, 2019 •

edited

Loading