DigitalCommons / land-explorer-front-end

React app for the Land Explorer front end
http://landexplorer.cc
GNU Affero General Public License v3.0
2 stars 0 forks source link

LX-293 Check out version of mapbox-gl-draw with fix for touch screens #298

Closed rogup closed 8 months ago

rogup commented 11 months ago

What? Why?

Closes #293

Tapping on drawn polygons and the property boundary layer doesn't work on touch screens.

We're hitting the problem described here https://github.com/mapbox/mapbox-gl-draw/pull/869

The issue was fixed by https://github.com/mapbox/mapbox-gl-draw/pull/1146 in this commit https://github.com/mapbox/mapbox-gl-draw/commit/84e35b7aa308c69a44b483296baa6ac1c6dc9db8 However, they reverted the fix because it broke editing of drawings on mobile https://github.com/mapbox/mapbox-gl-draw/pull/1158

But we don't allow drawings on mobile anyway, so this doesn't matter to us.

So we will use this specific commit of the mapbox-gl-draw repo, and we can wait on a proper fix before we upgrade the package again

What should we test?

Release notes

Deployment notes

Documentation updates

rogup commented 11 months ago

@ms0ur1s since we don't have as long to pair, thought I would just push the proposed changes I mentioned for you to look over

I have no idea if this will break other functionality, so probably needs a bit of general testing with drawing lines/polygons and messing with the boundary layer. But it seems to fix the problem when I test it using the mobile emulator in Google Chrome Inspect tools :)

lin-d-hop commented 10 months ago

Is this issue awaiting anything before moving on to review/qa? Ping @rogup @ms0ur1s

ms0ur1s commented 10 months ago

@lin-d-hop, I've just tested it locally, as well, on a mobile simulator and seems to work. I think it can go into review.

lin-d-hop commented 9 months ago

@rogup @ms0ur1s is this one just waiting on a review?

ms0ur1s commented 9 months ago

@rogup @ms0ur1s is this one just waiting on a review?

I tested this locally as well, I think it can be merged in.

lin-d-hop commented 8 months ago

@rogup @ms0ur1s Any reason this still is awaiting merge?

rogup commented 8 months ago

Not sure, I've just merged it

lin-d-hop commented 8 months ago

This is great! Nice work team!