Pomax / react-onclickoutside

An onClickOutside wrapper for React components
MIT License
1.83k stars 187 forks source link

make it work with shadow dom #323

Closed cameronbraid closed 3 years ago

cameronbraid commented 5 years ago

This seems to fix shadow dom issues.

The basic issue is that with a dom structure like :

In the above case the mousedown listener is registered on document and when clicking on div#clicker the handler is called with an event with target = div#shadow-host where as we really want it to start at div#wrapper

So the solution is to add the event listeners to both document and #shadow-root

However this causes the handler is triggered twice, once with event.target = div#clicker, then once with event.target=div#shadow-host. So we tag the event and only handle it once, from div#clicker

Then when walking up the dom in findHighest we have to continue via the host element so that we traverse the host document as well

cameronbraid commented 5 years ago

I've opened this as a draft PR since I really just wanted to get a discussion going on the correct solution for this

Pomax commented 5 years ago

fair enough - I'm not sure this is a solvable issue in this way since the shadow dom is supposed to be genuinely "hidden" from the page, so as far as the browser can tell the page, there's nothing "in" the shadow DOM, the event simply started in the (custom) element that is found in the regular DOM.

What's the concrete use case we're trying to solve for here, though? Because React itself doesn't use shadow DOM, that's more a custom elements thing (which I would love to see get so easy to use they basically make React obsolete =D).

cameronbraid commented 5 years ago

My primary case is css isolation.

By rendering into a shadow DOM I am albe to use my component in contexts where I have no control over the styling of the page. For example if I use semantic ui and a third party use bootstrap there will be no css clashes when embedding my control

I am using a component (react-datepicker) that uses onclickoutside to hide the calendar popup when you click outside the popup. However when used within a shadow dom, clicks within the calendar popup always close the popup due to the above mentioned reason.

newying61 commented 5 years ago

@cameronbraid Got the same issue in shadowDOM. Another solution maybe:

Inside the returned array the first one is the real target. when getting current here, we can replace the event.target with the first element in the array.

In older browsers, if we use the webcomponentsjs polyfill, shadowRoot is a virtual element in memory, it seems addEventListener is not working sometimes, haven't chase why.

Pomax commented 5 years ago

Looking at Event.composed is a good call, because it lets us access Event.composedPath, which is probably the exact thing we want to be looking at in this case, rather than having to do a "while not at top" loop.

As for older browsers: each version of this HOC is for modern browsers "at the time of release" so if it doesn't work in IE11, too bad for IE (anyone designing for IE11 would know they need to load dom4 shims already anyway), but if it doesn't work in Edge, Firefox, Chrome, or Safari, that might need some extra work. However, as far as I know none of those require any kind of shimming/polyfilling.

newying61 commented 5 years ago

@Pomax @cameronbraid I've raised another pull request #324 . Please have a look.

The polyfill is for custom web components API. Not for this lib. This lib works quite well in Chrome and other modern browsers. :)

mbIkola commented 5 years ago

Faced same issue when using react-datepicker in shadowRoot. Patch applied as "temporary workaround":

diff --git a/src/index.js b/src/index.js
index 3e66bf8..3ec0e85 100644
--- a/src/index.js
+++ b/src/index.js
@@ -131,6 +131,8 @@ export default function onClickOutsideHOC(WrappedComponent, config) {
      * for clicks and touches outside of this element.
      */
     enableOnClickOutside = () => {
+      const document = this.componentNode && this.componentNode.getRootNode ? this.componentNode.getRootNode() : document;
+
       if (typeof document === 'undefined' || enabledInstances[this._uid]) {
         return;
       }
@@ -178,6 +180,8 @@ export default function onClickOutsideHOC(WrappedComponent, config) {
      * for clicks and touches outside of this element.
      */
     disableOnClickOutside = () => {
+      const document = this.componentNode && this.componentNode.getRootNode ? this.componentNode.getRootNode() : document;
+
       delete enabledInstances[this._uid];
       const fn = handlersMap[this._uid];

quite similar to solution in this PR, but a bit simpler :) I can't use composedPath, cuz in my env shadowTree is attached in closed mode.

Pomax commented 3 years ago

closing due to inactivity (mine, mostly, and unless someone is going to fund me working on this again, it's unlikely that'll change)