mapbox / mapbox-plugins-android

Mapbox Android Plugins are a collection of libraries that extend our other SDKs, helping you design powerful mapping features while the plugins handle most of the heavy lifting.
https://www.mapbox.com/android-docs/plugins/overview/
BSD 2-Clause "Simplified" License
241 stars 119 forks source link

AnnotationManager drag event also firing a long-click on the map #1162

Open samcrawford opened 3 years ago

samcrawford commented 3 years ago

I'm using the latest 9.6.0 Mapbox SDK and the latest annotation plugin too. I'm creating some circles on my map (using CircleManager) and have setup a drag listener for them. This all works fine, but when dragging a circle it also seems to fire a long-click event on the map as well.

I note that dragging a circle does not fire a regular click event on the map. It only fires a long-click event.

This minimal code example reproduces the issue:

@Override
public void onMapReady(@NonNull final MapboxMap mapboxMap) {
    MainActivity.this.mapboxMap = mapboxMap;

    mapboxMap.setStyle(
            createStyleBuilder(STYLE_DEFAULT),
            style -> {
                initCircleManager(style);
                initMapClicks();
            });
}

private void initCircleManager(Style style) {
    circleManager = new CircleManager(mapView, mapboxMap, style);

    // snip - add a circle here, mark it as draggable

    circleManager.addDragListener(new OnCircleDragListener() {
        @Override
        public void onAnnotationDragStarted(Circle annotation) {
        }

        @Override
        public void onAnnotationDrag(Circle annotation) {
        }

        @Override
        public void onAnnotationDragFinished(Circle annotation) {
            System.out.println("Circle dragged to "+annotation.getLatLng());
        }
    });
}

private void initMapClicks() {
    mapboxMap.addOnMapLongClickListener(point -> {
        System.out.println("Map long-clicked at "+point);
    });
}

This results in the following output:

I/System.out: Circle dragged to LatLng [latitude=51.48172356366564, longitude=0.013026118189969793, altitude=0.0]
I/System.out: Map long-clicked at LatLng [latitude=51.48312269584446, longitude=0.01208949814909488, altitude=0.0]

My expectation is that dragging the circle should not result in any events being fired at the map level.

Am I doing something wrong or is this a bug? I can't find any issue that matches this.

samcrawford commented 3 years ago

For the benefit of anyone else hitting this, I've worked around this with the following horrible hack. This updates a global timestamp whilst the drag is taking place, and then checks to see if that timestamp is less than 500ms old when a map long-click event fires. If less than 500ms has elapsed since the last drag and the long-click, then the long-click is ignored.

private long lastDragTimestamp;

@Override
public void onMapReady(@NonNull final MapboxMap mapboxMap) {
    MainActivity.this.mapboxMap = mapboxMap;

    mapboxMap.setStyle(
            createStyleBuilder(STYLE_DEFAULT),
            style -> {
                initCircleManager(style);
                initMapClicks();
            });
}

private void initCircleManager(Style style) {
    circleManager = new CircleManager(mapView, mapboxMap, style);

    circleManager.addDragListener(new OnCircleDragListener() {
        @Override
        public void onAnnotationDragStarted(Circle annotation) {
        }

        @Override
        public void onAnnotationDrag(Circle annotation) {
            lastDragTimestamp = System.currentTimeMillis();
        }

        @Override
        public void onAnnotationDragFinished(Circle annotation) {
            lastDragTimestamp = System.currentTimeMillis();
            System.out.println("Circle dragged to "+annotation.getLatLng());
        }
    });
}

private void initMapClicks() {
    mapboxMap.addOnMapLongClickListener(point -> {

        // Workaround for https://github.com/mapbox/mapbox-plugins-android/issues/1162
        if (System.currentTimeMillis() - lastDragTimestamp < 500L) {
            System.out.println("Ignoring bogus long-click");
            return true;
        }

        System.out.println("Map long-clicked at "+point);
    });
}
NasH5169 commented 3 years ago

A simple boolean does the trick instead of timestamp.

Edit: Sorry you're right. If the drag/drop is done quickly, the long click event will be fired. So the timestamp method is mandatory to workaround this issue.

Ph0tonic commented 3 years ago

Just by curiosity, which version of the mapbox-plugins-android are you using ? It seems to me that this problem has been fixed in v0.9.0 by #1141.

samcrawford commented 3 years ago

Just by curiosity, which version of the mapbox-plugins-android are you using ? It seems to me that this problem has been fixed in v0.9.0 by #1124.

I was using v0.9.0, it still occurred for me with this version.

Ph0tonic commented 3 years ago

After watching your code with more attention, I might have found something. Your are adding your mapLongClickListener after your CircleManager. This might lead the mapLongClickListener to be triggered before your draggableListener which is added via the CircleManager. Could you try to inverse the two lines like this :

initMapClicks();
initCircleManager(style);

I'm not sure that those changes will solve your problem, but it is worth to give it a try.

Ph0tonic commented 3 years ago

After some more digging, I think that this might happen due to the way mapViewhandles it's event here https://github.com/mapbox/mapbox-gl-native-android/blob/main/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/maps/MapView.java.

If you have a look at the method onTouchEvent, we first call the mapboxEventHandler and then the one of the CircleManager. Maybe we could do a PR to change the order or add an extra parameter to configure the order of execution of the handles.

samcrawford commented 3 years ago

After watching your code with more attention, I might have found something. Your are adding your mapLongClickListener after your CircleManager. This might lead the mapLongClickListener to be triggered before your draggableListener which is added via the CircleManager. Could you try to inverse the two lines like this :

initMapClicks();
initCircleManager(style);

I'm not sure that those changes will solve your problem, but it is worth to give it a try.

Thanks for investigating! I've tried your suggested change and I'm afraid it makes no difference. The long-click event on the map is still fired when I drag the circle marker.