LiveBy / react-leaflet-control

Dumb React component for creating Controls
ISC License
88 stars 24 forks source link

Disable click propagation on control component #9

Closed kellyi closed 7 years ago

kellyi commented 7 years ago

Hi! Thanks again for creating and maintaining this library. I was setting up a sandbox project using this component and encountered a bug whereby clicks (and double clicks) on the control component pass through to the map and can cause the map to pan or zoom when the user's trying to click on the control.

You can see the bug in action in this small demo project I set up:

https://github.com/kellyi/react-container/tree/research/react-leaflet-control-bug

screen shot 2016-11-28 at 8 46 29 pm

...in which double clicking on the "Demo Control" component causes the map to zoom. I noticed it when trying to add some buttons to a control and seeing clicks rapid enough in succession to register as double clicks would zoom in on the map.

I did some investigation and noticed that for custom controls Leaflet seems to require explicitly calling DomEvent.disableClickPropagation(...) with the custom control as an argument in order to get that behavior: https://github.com/Leaflet/Leaflet/issues/1343#issuecomment-99494636

This PR updates the onAdd methods for the dist and lib Control.Dumb.js files to explicitly disable click propagation.

Testing Instructions To test that this works you can:

cd app
npm install
npm start

...with a recent (4.x/6.x) version of Node. The app will load in webpack-dev-server on port 7171, and you can double click on the "Demo Control" text to see the bug happening

onAdd: function (map) {
    var _controlDiv = _leaflet.DomUtil.create('div', this.options.className);
    _leaflet.DomEvent.disableClickPropagation(_controlDiv);
    return _controlDiv;
},

Webpack-dev-server should patch the change into the project and hot-reload the app in the browser. From there you can try double-clicking on the "Demo Control" text and verify that instead of zooming in on the map, it is highlighting the text instead:

screen shot 2016-11-28 at 8 55 04 pm

Thanks again for maintaining this project! Let me know if you'd like me to make any additional changes or anything for this PR.

jgimbel commented 7 years ago

Released with version 1.2.0