[Backsearch P1] Property Boundary layer doesnt work on mobile

[lin-d-hop]

Description

When I turn on the Property Boundary layer on mobile (Android) I am unable to select any properties. I press on a polygon after zooming in and nothing is highlighted. Because of this I cannot use the LandReg backsearch on mobile

Acceptance Criteria

I am able to select a property with the property boundary layer turned on on mobile The LandREg backsearch process works as it does now on Desktop

[ms0ur1s]

Updated mapbox-gl-draw to v1.4.3 to solve passive event listener error on touch devices.

Console Error

mapbox-gl-draw.js:1 unable to preventdefault inside passive event listener invocation

GitHub Issue

https://github.com/mapbox/mapbox-gl-draw/issues/1019

This wasn't directly the problem but with noise it created it might well have been.

[ms0ur1s]

The main issue appears to be that the onClick event on the Feature component, within MapProperties.js, doesn't work on handheld / touch devices, despite this generally being the case. To complicate matters the Feature component has no prop for onTouchStart or any touch event, unless you count drag which isn't really device specific, therefore adding onTouchStart to the component has no effect.

There does happen to be an open PR on the react-mapbox-gl repo that addresses touch events in a wider context, but what happens with this we'll see. https://github.com/alex3165/react-mapbox-gl/pull/693/files

I managed to circumvent this by adding an onTouchStart event to the surrounding Layer component, but this required the Layer to be included in the loop handling the array of Feature types (basic, detailed and highlighted). However, this has the consequence of creating a Layer per Feature, rather than a Layer per Feature type, which seems excessive, hacky and definitely semantically incorrect. It did work, though, without throwing any errors or seeming to impeding user experience. I will check this a little more thoroughly. What's your opinion on this solution @King-Mob and @rogup?

Snippet from MapProperties.js

if (property.date_proprietor_added)
      detailedPropertyFeatures.push(
        <Layer
          key={property.poly_id}
          type={"fill"}
          paint={{
            "fill-opacity": 0.15,
            "fill-color": "green",
            "fill-outline-color": "green",
          }}
          onTouchStart={() => onClickNewProperty(property)}
        >
          <Feature
            coordinates={[property.coordinates]}
            key={property.coordinates[0][0]}
            onClick={() => onClickNewProperty(property)}
          />
        </Layer>
      );
    else

As an alternative to this I tried wrapping the Feature component in a div and applying the onTouchStart event to this, but the direct relationship between Layer and Feature is such that any interference between them causes an error.

I'll park this for now, until I receive some feedback.

[rogup]

hey @ms0ur1s your investigation into this looks great. I had a quick look at the issue that you linked (https://github.com/mapbox/mapbox-gl-draw/issues/1019) and looks like lots of people have been experiencing a similar problem.

It looks like they have implemented a fix, which is in the latest mapbox-gl-draw v1.4.3. Have you tried updating the package and testing if that works?

If that doesn't work, I would also try adding this package https://github.com/zzarcon/default-passive-events, which someone suggested as a fix in the comments.

I think it would be worth trying these couple of quick fixes out before attempting any big workarounds in our code. I agree with your intuition that having a layer for every feature is messy and probably something we want to avoid.

[rogup]

Doh, I see you've already tried updating to v1.43, I misread your original comment! But might be worth trying the 'default-passive-events' package which someone mentioned solved the issue on mobile

Note this also affects drawn polygons and lines

[ms0ur1s]

Thanks @rogup, I realise now my first comment doesn't mention that the upgrade to v1.4.3 solved the passive event listener issue, apologies, my bad. There are no longer any errors but, I'm not sure how interconnected this error was with onClick event problem, now, as the problem persists. I'll give 'default-passive-events' package a go 🤞.

[rogup]

@ms0ur1s I just had one more look at the github issues and think I've spotted it.

It's the same problem as this 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.... I think if you use the above specific commit of the repo in our package.json it should work, and we can wait on a proper fix before we upgrade the package again! We can work out how to do this together in our call tomorrow if you'd like?

[ms0ur1s]

@ms0ur1s I just had one more look at the github issues and think I've spotted it.

It's the same problem as this 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.... I think if you use the above specific commit of the repo in our package.json it should work, and we can wait on a proper fix before we upgrade the package again! We can work out how to do this together in our call tomorrow if you'd like?

Thanks @rogup , I'll give this a look and give you a shout if I get stuck.

[ms0ur1s]

Second thoughts, @rogup, I'll wait to chat to you about this one, as the commit above seems to refer to mapbox-gl-draw v1.4.3, which is what I'd initially updated to. Sorry man, I'm a little confused on this one.

[rogup]

Hey @ms0ur1s In the commit on the PR I made, I changed the version to a specific commit in the mapbox-gl-draw Git repo https://github.com/DigitalCommons/land-explorer-front-end/pull/298

Are you able to check it out the 'fix-touch-screen-mapbox' branch I made and give it a test?

[ms0ur1s]

Ah, I see. Nice one @rogup, I have change the reference in my package.json and give that a try.

[ms0ur1s]

Solved by @rogup in #298?

[ms0ur1s]

Closing as just tested the branch attached to #298 fix-touch-screen-mapbox and it works locally for me on a mobile simulator.

[rogup]

Hey @ms0ur1s this has been closed but the PR is still unmerged. I think the process for this would be to merge the PR, then put this issue ticket into QA so that Lynne can test out the fix on a real mobile device on the staging site. We generally only close tickets once the fix has been delivered to production

[ms0ur1s]

Cool, nice one @rogup. I've re-opened this ticket and will kick it into QA once #298 is merged in.