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

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

Merged
merged 1 commit into from
Mar 6, 2024

Conversation

rogup
Copy link
Collaborator

@rogup rogup commented Dec 15, 2023

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 mapbox/mapbox-gl-draw#869

The issue was fixed by mapbox/mapbox-gl-draw#1146 in this commit mapbox/mapbox-gl-draw@84e35b7
However, they reverted the fix because it broke editing of drawings on mobile mapbox/mapbox-gl-draw#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?

  • On desktop, save a map with a polygon
  • Open this saved map on a mobile device and try to select the polygon by tapping.
  • Also, enable the property boundaries layer and try to select a property

Release notes

Deployment notes

Documentation updates

@rogup
Copy link
Collaborator Author

rogup commented Dec 15, 2023

@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
Copy link
Contributor

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

@ms0ur1s
Copy link
Collaborator

ms0ur1s commented Jan 9, 2024

@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
Copy link
Contributor

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

@ms0ur1s
Copy link
Collaborator

ms0ur1s commented Feb 6, 2024

@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
Copy link
Contributor

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

@rogup rogup merged commit c47fde5 into development Mar 6, 2024
@rogup rogup deleted the fix-touch-screen-mapbox branch March 6, 2024 16:51
@rogup
Copy link
Collaborator Author

rogup commented Mar 6, 2024

Not sure, I've just merged it

@lin-d-hop
Copy link
Contributor

This is great! Nice work team!

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

Successfully merging this pull request may close these issues.

[Backsearch P1] Property Boundary layer doesnt work on mobile
3 participants