spatial-navigation icon indicating copy to clipboard operation
spatial-navigation copied to clipboard

shadow dom (open) support

Open mihaiav opened this issue 4 years ago • 8 comments

I'm aware the polyfill is not working on closed shadow dom. Is this the case with open shadow dom as well?

mihaiav avatar Apr 02 '20 08:04 mihaiav

I'm not sure but the open shadow dom should work well in the current polyfill. Could you try it and share the result whether it works well, if you don't mind?

If it doesn't, the contribution for it would be definitely valuable for the polyfill.

anawhj avatar Apr 02 '20 08:04 anawhj

I don't think it can work because nowhere in the polyfill I find any reference that checks the shadow root on nodes (e.g. `Element.shadowRoot ). Below is a test that proves the current polyfill is not working.

<!DOCTYPE html>
<html>
   <head>
      <meta charset="UTF-8">
      <style>
         h{
             font-size: larger;
             font-weight: 100;
         }
         button{
         color:white;
         font-size: large;
         background-color: rgb(1, 1, 10);
         width: 200px;
         height: 200px;
         }
         button:focus{
         background-color: rgb(93, 0, 0);
         }
         .defaultButtons {
         display: flex;
         flex-direction: row;
         } 
      </style>
      <script>
         class XCom extends HTMLElement {
         constructor() {
           super();
           // Insert a template with some buttons
           let tmpl = document.createElement('template');
         tmpl.innerHTML = `
         <style>
         button{
         color:white;
         font-size: large;
         background-color: rgb(1, 1, 10);
         width: 200px;
         height: 200px;
         }
         button:focus{
         background-color: rgb(93, 0, 0);
         }
         </style>
         <button>Button in shadow</button>
             <button>Button  in shadow</button> 
             <button>Button  in shadow</button> 
         `;
             // Attach a shadow root to the element.
             let shadowRoot = this.attachShadow({mode: 'open'});
             shadowRoot.appendChild(tmpl.content.cloneNode(true));
         }
         
         }
         
         customElements.define('shadow-element', XCom);
         
      </script>
      <script src="https://wicg.github.io/spatial-navigation/polyfill/spatial-navigation-polyfill.js"></script>
   </head>
   <body>
      <h>Use LEFT / RIGHT arrows to navigate. You may find that the 3 buttons are unreachable because spatial nav polyfill does not support shadow dom</h>
      <div class="defaultButtons">
         <button>Button</button>
         <button>Button</button>
         <button>Button</button>
         <shadow-element></shadow-element>
      </div>
    
   </body>
</html> 

mihaiav avatar Apr 02 '20 11:04 mihaiav

You're definitely right. Sorry for confusing you.

The polyfill hasn't supported the shadow dom. You could consider to implement the shadow dom support inside your application or directly to the polyfill with a pull request.

If the embedded shadow dom is an external component not made by you, the latter approach would be the only way to support it.

If you consider how to add the shadow dom support directly into the polyfill, you could refer to the appropriate methods as follows:

  1. Consider the shadow dom host with 'mode: open' and include its descendants to the candidates https://github.com/WICG/spatial-navigation/blob/master/polyfill/spatial-navigation-polyfill.js#L298

  2. Handle the descendants of the shadow root if one of them becomes the best candidate https://github.com/WICG/spatial-navigation/blob/master/polyfill/spatial-navigation-polyfill.js#L237

anawhj avatar Apr 02 '20 23:04 anawhj

Thanks for guidance! I've tried to fix it in getSpatialNavigationCandidates and focusingController but unfortunately it seems to take a bit more effort so I can't promise a PR any time soon. There are a lot of places where .shadowRoot and .host needs to be handled. Shadow Dom has no 'style' property so a lot of methods require patches (e.g. isBeingRendered, isVisible etc). There are also references to parentElement but Shadow DOM has no parentElement (only .host) so that needs to be checked as well.

mihaiav avatar Apr 03 '20 02:04 mihaiav

Thanks for sharing the info. I agree that it seems to require a bit efforts for handling all the things. Hope to see the contribution from someone in the future. :)

anawhj avatar Apr 05 '20 23:04 anawhj

I've made it work with shadow dom in my app. It's required to replace several native calls(e.g. prototype.parentElement, prototype.contains) with wrappers(parentElementNav, containsNav) that handle the shadow dom. I'm pasting here the gist hopefully they will be helpful for whoever wants/has the time to properly implement them in the polyfill.

window.ShadowRoot.prototype.parentElementNav = function() {
    return this.host
};
window.Element.prototype.parentElementNav = function() {
    if (!this.parentElement && this.parentNode && this.parentNode.host) {
        return this.parentNode.host;
    }
    return this.parentElement;
}

 window.Element.prototype.containsNav = function(n) {
        if (!n) {
            console.log("invalid contains subject");
            throw (n);
        }
        let el = this;
        if (this.shadowRoot) {
            if (this.shadowRoot.mode !== "open") {
                return false;
            }
            el = this.shadowRoot;
        }
        let ok = el.contains(n);
        if (ok) {
            return ok;
        }
        while (true) {
            n = n.getRootNode({
                composed: false
            });
            ok = el.contains(n);
            if (ok || !n.host) {
                return ok;
            }
            if (el.contains(n.host)) {
                return true;
            }
            n = n.host;
        }
  };
window.Element.prototype.childrenNav = function() {
    if (this.shadowRoot) {
        return this.shadowRoot.children;
    }
    return this.children;
}
window.Element.prototype.childElementCountNav = function() {
    if (this.shadowRoot) {
        return this.shadowRoot.childElementCount;
    }
    return this.childElementCount;
}

function activeElementNav(){
    let ac = document.activeElement;
    if (!ac.shadowRoot){
        return ac;
    }
    return ac.shadowRoot.activeElement;
}

function elementFromPointNav(doc, x, y) {
    let el = doc.elementFromPoint(x, y)
    if (el.shadowRoot) {
        return elementFromPointNav(el.shadowRoot, x, y);
    }
    if (el.host) {
        return elementFromPointNav(el.host, x, y);
    }
    return el;
}

mihaiav avatar Apr 12 '20 19:04 mihaiav

Thank you for sharing the major codes. Looks like it works well for handling dom nodes inside shadow dom.

If someone proposes the functionality as a PR considering the existing polyfill structure, I or other people could review it.

anawhj avatar Apr 14 '20 04:04 anawhj

Thanks for guidance! I've tried to fix it in getSpatialNavigationCandidates and focusingController but unfortunately it seems to take a bit more effort so I can't promise a PR any time soon. There are a lot of places where .shadowRoot and .host needs to be handled. Shadow Dom has no 'style' property so a lot of methods require patches (e.g. isBeingRendered, isVisible etc). There are also references to parentElement but Shadow DOM has no parentElement (only .host) so that needs to be checked as well.

I'm currently trying to implement this method, via editing the polyfill itself. Do you happen to have an example, or is your other posted method the best route?

uplusion23 avatar Oct 04 '20 14:10 uplusion23