downshift version: v2.0.20What you did:
Render Downshift within a shadow root using the web components API.
What happened:
Clicking on items do not select or register at all.
Reproduction repository:
https://codesandbox.io/s/kk674yvm0o
Problem description:
This is totally related to #200, and it has been fixed. However to make it work with a shadowRoot (document fragment) is currently very cumbersome. Again, see the example: https://codesandbox.io/s/kk674yvm0o
Suggested solution:
Current, addEventListener and removeEventListener rely on environment.document to be present, which works within an iframe. For shadowRoots, we could try environment.document || environment.ownerDocument.
Hi @Rendez!
Thanks for the issue. If that fixes the issue then you're welcome to make a pull request for it. I would like to have a test for this though. It looks like jsdom does not yet support web components, so we'll probably need to make it part of the E2E test suite with Cypress and storybook.
Thanks!
Thanks for the suggestion on js-dom. I will make it happen though might take a couple of days, cheers!
Alright, so I started looking at the source and couldn't find a strong-enough reason to do this elegantly.
I wouldn't modify the current approach, since the environment has to be a window-like object, and ShadowRoot simply isn't one. In other words, the current implementation covers iframe and window usages perfectly, but it's also normal to want to adapt it for web components.
GIven the API surface we need, add(remove)EventListener and document, there are two possible ways to approach it:
environment.document internally that "assumes" ownerDocument is what we need when document isn't found within the environment object.environment object from the outside, which can be done in a custom way depending on what we need, thus, not being bound to ShadowRoot and its particular object's prototype.IMO, the first approach would offer a targeted solution, but offer less guarantees. Therefore, I've come up with a more refined and elegant way to approach it from the outside (as in 2.):
https://codesandbox.io/s/j42nwojwzy
const isProxySupported =
typeof Proxy === 'function' &&
// https://github.com/facebook/react/issues/12011
!Object.isSealed(new Proxy({}, {}))
/**
* @param environment ShadowRoot|Window|Object
*/
const createEnvironment = environment => {
// assume first a non-ShadowRoot object
if ('document' in environment) {
return environment
}
// es6-way (support lacking on IE and older Safari versions)
if (isProxySupported) {
// this is the most elegant way
return new Proxy(environment, {
get: function(obj, prop) {
if (prop === 'document') {
// ownerDocument would be the closest choice in relation to an iframe 'window'
return obj.ownerDocument
}
// is there in which Reflect.apply would be helpful here?
return obj.ownerDocument[prop].bind(obj)
},
})
}
// es5-way
return Object.create(Object.prototype, {
document: {
configurable: false,
get() {
return environment.ownerDocument
},
},
addEventListener: {
configurable: false,
get() {
return environment.ownerDocument.addEventListener.bind(environment)
},
},
removeEventListener: {
configurable: false,
get() {
return environment.ownerDocument.removeEventListener.bind(environment)
},
},
})
}
Let me know your opinion on this, and if looks like a good use-case solution we could document it somehow for the future (perhaps on how to use environment with an iframe vs shadow-root).
Hmmm.... I think that rather than build-in support for this I'd rather document how to make this work with the existing API. The reason being that it's not too difficult to do it yourself and 99% of people don't have this use case.
Would you like to make a pull request to document how to do this?
That is exactly what I was trying to say. I'll open the PR :)
@Rendez I have a question about this. A link to your solution is provided on the main doc page but I'm confused as to why it has to be as complicated as it is. Why do you need to use a Proxy for this? Why can't you just return an object that has the three properties that downshift expects that do what you need. Why doesn't this work?:
function createDownshiftEnvironment(shadowRoot) {
return {
document: shadowRoot,
addEventListener: shadowRoot.addEventListener.bind(shadowRoot),
removeEventListener: shadowRoot.removeEventListener.bind(shadowRoot)
};
}
@TazmanianD that works too, I was being extra safe and also playful in the example above.
@Rendez I've been playing around with this some more and I think there's a bug in your proxy and by extension mine as well. You're assigning environment.document to shadowRoot.ownerDocument but I think that's a mistake. The downshift library uses environment.document to get at the active element as well as call getElementById but those properties aren't on ownerDocument, they're on shadowRoot itself. I think where you've got
document: ownerDocument,
It really should be
document: shadowRoot,
Does that seem right to you?
And actually I think this may be true for the addEventListener calls as well. Wouldn't it be more appropriate to add those listeners to the shadow root directly?
About the add/removeEventListener, yes, it would be more correct to assign them to the shadowRoot that's right, more similar to window. However I do have all of the prototype properties you mention within ownerDocument. I do not however have body in the shadowRoot prototype. Also, do you know if the activeElement of shadowRoot is ever anything but null?
To be honest I haven't tested what I wrote above outside my one example in a project, and for that it worked perfectly and continues to work (even after body and activeElement became necessary)
Can you show me how you're creating your shadowRoot, in which your shadowRoot's ownerDocument doesn't have getElementById etc? (I might be wrong about the assumption for your case)
I found that your version did not work correctly although the only case where I could see that was that the dropdown wasn't scrolling automatically when you use the arrow keys to navigate items. The ownerDocument does have getElementById but it was always returning null for some reason but calling it on the shadow root does work.
And yes, the activeElement on the shadow root did seem to be the actual element that has focus while the activeElement on the ownerDocument was the I think the shadow root itself or the host element..I forget exactly which.
I went ahead and updated my comment above to show what I ended up using it does seem to work for me.
Most helpful comment
I found that your version did not work correctly although the only case where I could see that was that the dropdown wasn't scrolling automatically when you use the arrow keys to navigate items. The ownerDocument does have
getElementByIdbut it was always returning null for some reason but calling it on the shadow root does work.And yes, the activeElement on the shadow root did seem to be the actual element that has focus while the activeElement on the ownerDocument was the I think the shadow root itself or the host element..I forget exactly which.
I went ahead and updated my comment above to show what I ended up using it does seem to work for me.