Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

shadow dom (open) support #217

Open
mihaiav opened this issue Apr 2, 2020 · 8 comments
Open

shadow dom (open) support #217

mihaiav opened this issue Apr 2, 2020 · 8 comments

Comments

@mihaiav
Copy link

mihaiav commented Apr 2, 2020

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

@mihaiav mihaiav changed the title shadow dom (open) shadow dom (open) support Apr 2, 2020
@anawhj
Copy link
Collaborator

anawhj commented Apr 2, 2020

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.

@mihaiav
Copy link
Author

mihaiav commented Apr 2, 2020

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> 

@anawhj
Copy link
Collaborator

anawhj commented Apr 2, 2020

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

@mihaiav
Copy link
Author

mihaiav commented Apr 3, 2020

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.

@anawhj
Copy link
Collaborator

anawhj commented Apr 5, 2020

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. :)

@mihaiav
Copy link
Author

mihaiav commented Apr 12, 2020

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;
}

@anawhj
Copy link
Collaborator

anawhj commented Apr 14, 2020

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.

@uplusion23
Copy link

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?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants