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

Overlay - Element events (click) pass through to Map #6948

Open
ghost opened this issue Jun 23, 2017 · 22 comments
Open

Overlay - Element events (click) pass through to Map #6948

ghost opened this issue Jun 23, 2017 · 22 comments

Comments

@ghost
Copy link

ghost commented Jun 23, 2017

I have a popup in React made as an Overlay. The problem is, events meant for this React component (clicking a button) pass through to the map, which means sometimes features get (de)selected because the buttons happen to be over them.

Setting the overlay with stopEvent: true causes the React component to not receive any click events at all, while false causes the map to be also clicked as well.

Is this intended behavior? Does 'stopEvent' prevent the overlay element from having any events?

@bartvde
Copy link
Member

bartvde commented Jun 23, 2017

yeah I've dealt with this as well, there are multiple threads on it such as:

https://stackoverflow.com/questions/35454014/clicks-on-reactjs-components-not-firing-on-an-openlayers-overlay

I've also used the approach of setting interactions to not active when the popup is open such as here:

https://github.com/boundlessgeo/sdk/blob/3ee2e70c6c35d3390be4804058ee9c0521da7806/src/components/EditPopup.js#L223:L228

See also: #4400

@tschaub
Copy link
Member

tschaub commented Jun 24, 2017

I think we'd agree that setting stopEvent: true should only cause the map to ignore events that target the overlay. We should be able to handle this in the library, so I think it is a bug (instead of intended behavior) that other listeners can't respond to events on the overlay.

@ghost
Copy link
Author

ghost commented Jun 26, 2017

For my particular use-case my workaround involved setting stopEvent: true and doing something like this inside my React component (which I took from @bartvde's answer in that linked SO question):

if (this.closeButtonRef.button.button.onclick === null) {
   this.closeButtonRef.button.button.onclick = this.setFeaturePopupVisibleAsFalse;
}

This is because I was using material-ui which comes with its own buttons. A normal button will likely involve this.buttonElement.onclick = this.testFunc or something similar.
I also had to bind this in componentWillReceiveProps instead of the constructor or componentDidMount since the element in question relied on a prop triggering before appearing.
This looks hacky at best, and I had to do this for every action button on that popup. Still, hoping this helps anyone who stumbles with it, while there isn't a fix yet.

EDIT: And for those who use Redux, you'd have to use dispatch directly inside the function, you can't use those you set in mapDispatchToProps.

@bartvde
Copy link
Member

bartvde commented Jun 26, 2017

Yep would be great if we could handle this in the library instead, I agree those workaround are wonky ;-)

@marcjansen
Copy link
Member

We have deployed such wonky workarounds as well, I would also favor a library solution.

@bassarisse
Copy link

Any news on this? Looking for a better solution, at least without having to patch each component =/

@hwbllmnn
Copy link

hwbllmnn commented Sep 5, 2017

@tschaub @ahocevar I'd be willing to work on a solution for this. Can you give me a few pointers on how to approach this? Thanks

@marcel-dancak
Copy link

Sill no official solution, so I will add another workaround for React. You can enable it simple on overlay's root element, and onClick events should be working almost normally in all nested elements. It works by converting mouseup events to click events with overridden stopPropagation method, so ol cannot stop it.

const convertToClick = (e) => {
  const evt = new MouseEvent('click', { bubbles: true })
  evt.stopPropagation = () => {}
  e.target.dispatchEvent(evt)
}
<div onMouseUp={convertToClick}>
  <button onClick={handleClick}>x</button>
  ...
</div>

(For simple use cases, it is easier to directly use onMouseUp instead of onClick)

@benupham
Copy link

I have been having trouble with event propagation and OpenLayers as well. I'm not sure if this is relevant to React, or the best solution, but I wanted to point out (no pun intended) that the click event that the ol map listens for it "pointerdown" not "click" -- at least in some cases. I was stopping propagation on "click" on an overlay, which wasn't working, but stopping propagation on "pointerdown" does. Similar for "mousemove" -- ol listens for "pointermove".

@dnyanp
Copy link

dnyanp commented Oct 5, 2018

Any update on this? Is it fixed?

edellucien pushed a commit to geops/react-spatial that referenced this issue Feb 5, 2019
fix safari popup click event which targets the map instead
openlayers/openlayers#6948
@stale
Copy link

stale bot commented May 22, 2019

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@walkermatt
Copy link
Contributor

I've recently encountered this when testing ol-geocoder and ol-layerswitcher components which don't use React but do remove or recreate the elements that the events originate from.

I think the issue is that once the elements have been removed the check to detect whether an event originated from an element that is a child of ol-overlaycontainer-stopevent no longer holds as the elements are no longer in the DOM.

I think a fix might be to check if the target is in the page in ol/PluggableMap~PluggableMap#handleMapBrowserEvent like so:

--- a/src/ol/PluggableMap.js
+++ b/src/ol/PluggableMap.js
@@ -950,8 +950,23 @@ class PluggableMap extends BaseObject {
       return;
     }
     let target = mapBrowserEvent.originalEvent.target;
+    if (!document.body.contains(target)) {
+        return;
+    }
     while (target instanceof HTMLElement) {

Happy to submit a PR if acceptable.

@walkermatt
Copy link
Contributor

Demo app to reproduce the issue. If you open the console you should see singleclick events logged when clicking on the N/S button. This is due to the RotateNorthControl control rebuilding the UI once the user clicks the button. The example is a little contrived but it demonstrates the issue. I've found that the above patch ☝️ resolves the issue when applied to a local copy of the repo.

@walkermatt
Copy link
Contributor

Hi @ahocevar, is this issue a candidate for being reopened or would I be better creating a new issue?

@ahocevar
Copy link
Member

ahocevar commented Dec 16, 2019

@walkermatt Thanks for your research and suggested fix! I'm reopening this. Your fix would need some comments because it is not really obvious, but if it also solves the reported issues with React I'd not be opposed.

@walkermatt
Copy link
Contributor

walkermatt commented Dec 19, 2019

@ahocevar I've pulled together an example that uses React to provide the UI for a Control which again reproduces the issue. There is a singleclick handler on the map which is triggered when the show/ hide buttons are clicked (see the console log). My local copy which includes the change to PluggableMap fixes the problem.

Do you think there would be support for adding a React Control example if it was tidied up a little?

@ahocevar
Copy link
Member

ahocevar commented Jan 8, 2020

@walkermatt I think a React Control example would be a nice addition. Thanks in advance!

walkermatt added a commit to walkermatt/ol that referenced this issue Jan 10, 2020
As discussed in openlayers#6948 (comment)

The check to see if the target is within the "page" uses the viewport as
the MapBrowserEventHandler instance adds it's listeners to the viewport.

Using Node.contains appears to have a slight performance benefit
over manually walking the DOM.
@jumpinjackie
Copy link
Contributor

The fix introduced with #10502 and shipped with OpenLayers 6.2 breaks all mouse events in my map viewer. I am in a bit of an inverted situation. I am not trying to use React components as controls/overlays for an OL map. Instead I have a React component that wraps the OL map and in this situation I lose all mouse events with OL 6.2 so I cannot pan, mouse wheel zoom, drag, etc, etc.

The react component is quite complex so I don't know what specifics I should provide to give better context for diagnosing/debugging the issue. But as a starting point to at least demonstrate the problem, I've got a storybook instance on my GH pages repo that demonstrates the problem:

https://jumpinjackie.github.io/mapguide-react-layout/olbug/storybook-static/index.html

Choose any story that showcases the map. You should notice that mouse based actions (pan, mouse wheel zoom, etc) do not work at all.

Compare this with the current master, which is also updated to OL 6.2, but I've monkey-patched out the bad PluggableMap method.

https://jumpinjackie.github.io/mapguide-react-layout/master/storybook-static/index.html

All the mouse based actions work here.

@ahocevar
Copy link
Member

@jumpinjackie, please make sure you include css/ol.css from v6.2.0 with your component. Looks like the pointer-events: none style is missing from the overlay container divs.

jumpinjackie added a commit to jumpinjackie/mapguide-react-layout that referenced this issue Feb 13, 2020
…OL stylesheet (probably circa OL 4.6.5), so it is missing the required pointer-events CSS. Including the actual OL stylesheet from the npm module makes all the mouse events works again (ref: openlayers/openlayers#6948 (comment))
@jumpinjackie
Copy link
Contributor

Thanks @ahocevar that does indeed fix the issue. I had a stale local pre-6.0 OL stylesheet.

ZakarFin added a commit to ZakarFin/oskari-frontend that referenced this issue Apr 2, 2020
@ZakarFin
Copy link

ZakarFin commented Apr 2, 2020

The fix introduced with #10502 and shipped with OpenLayers 6.2 breaks all mouse events in my map viewer.

This was a life-saver comment. I've been debugging our React/OpenLayers app for some time now trying to figure out which library upgrade broke the map. Turns out we haven't included ol.css at all previously using ol 6.1.1 and older. Looks like adding an import for the ol/ol.css fixed the issue.

@Voronar
Copy link

Voronar commented Aug 11, 2020

Is this new React release helps with the issue?
https://reactjs.org/blog/2020/08/10/react-v17-rc.html#changes-to-event-delegation

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

No branches or pull requests