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

ShapeSource onpress call 2 times #97

Closed
eklauberg opened this issue Jun 7, 2019 · 16 comments · Fixed by #137
Closed

ShapeSource onpress call 2 times #97

eklauberg opened this issue Jun 7, 2019 · 16 comments · Fixed by #137

Comments

@eklauberg
Copy link

Describe the bug
The onpress function of ShapeSource is called 2 times

To Reproduce

async function onPress(e, coleta, setDados) {
  console.log(11);
}

function Point({
  features, coleta, setDados
}) {
  return (
    <MapboxGL.ShapeSource
      cluster
      clusterRadius={35}
      clusterMaxZoom={14}
      id="postes"
      shape={{ type: 'FeatureCollection', features }}
      onPress={e => console.log(11)}
    >
      <MapboxGL.SymbolLayer
        id="pointCount"
        style={styles.clusterCount}
      />

Screenshots
image

Versions (please complete the following information):

  • Platfrom: [Android]
  • SDK Version [28]
  • React Native Version [0.59.5]
@mattijsf
Copy link
Member

mattijsf commented Jun 7, 2019

Does it only happen when clustering is enabled?

@smol-honk
Copy link

I've also noticed that the onPress in UserLocation calls twice with each press.

@carsonaaberle
Copy link

It is being called twice as for me on Android only for MapView onPress. I am on 7.0.0-rc-2

@makirby
Copy link
Contributor

makirby commented Jun 11, 2019

We came under the same problem. I pinned it down to duplicate events being dispatched once by the ShapeSource and once by the UIManager.

This patch fixes it but I've haven't had time yet to check if it creates any other side effects.

diff --git a/node_modules/@react-native-mapbox/maps/android/rctmgl/src/main/java/com/mapbox/rctmgl/components/AbstractEventEmitter.java b/node_modules/@react-native-mapbox/maps/android/rctmgl/src/main/java/com/mapbox/rctmgl/components/AbstractEventEmitter.java
index 7fa9007..d8a093c 100644
--- a/node_modules/@react-native-mapbox/maps/android/rctmgl/src/main/java/com/mapbox/rctmgl/components/AbstractEventEmitter.java
+++ b/node_modules/@react-native-mapbox/maps/android/rctmgl/src/main/java/com/mapbox/rctmgl/components/AbstractEventEmitter.java
@@ -42,12 +42,13 @@ abstract public class AbstractEventEmitter<T extends ViewGroup> extends ViewGrou
             return;
         }
 
-        mRateLimitedEvents.put(eventCacheKey, System.currentTimeMillis());
-        mEventDispatcher.dispatchEvent(new AbstractEvent(event.getID(), event.getKey(), event.toJSON()));
-
         RCTEventEmitter emitter = EventEmitter.getViewEmitter(mRCTAppContext);
+        mRateLimitedEvents.put(eventCacheKey, System.currentTimeMillis());
+        // try to get view dispatcher, otherwise use the UImanager emitter
         if (emitter != null) {
             emitter.receiveEvent(event.getID(), event.getKey(), event.toJSON());
+        } else {
+            mEventDispatcher.dispatchEvent(new AbstractEvent(event.getID(), event.getKey(), event.toJSON()));
         }
     }
 

@mfazekas
Copy link
Contributor

Looks like to me it's a result of merge error:

One change mEventDispatcher.dispatchEvent( was nitaliano/react-native-mapbox-gl#1508

react-native-mapbox-gl/maps@7458dd1#diff-67b563fd89b6669ee1bc084451e62e57R41

other was from expression changes:
nitaliano/react-native-mapbox-gl@fc8901f#diff-67b563fd89b6669ee1bc084451e62e57R42

I guess the first change should win, so we should keep mEventDispatcher.dispatchEvent(new AbstractEvent(event.getID(), event.getKey(), event.toJSON())); and remove emitter related lines.

@makirby
Copy link
Contributor

makirby commented Jun 13, 2019

Yeah that looks like what's happened. I'll get it into a PR later today.

edit: actually after some testing it looks like the emitter is still needed for certain events

@kristfal
Copy link
Member

kristfal commented Jun 13, 2019 via email

@mfazekas
Copy link
Contributor

edit: actually after some testing it looks like the emitter is still needed for certain events

I've implemented the fix in #137. Let me know if you're seeing double events/missing events now.

@bwandersOA
Copy link

All - Thanks for working a solution to this one and actively maintaining this repo! We are holding off on releasing our Android app until we work this fix in. When might we expect the new release to be tagged and up on npm?

@makirby
Copy link
Contributor

makirby commented Jun 21, 2019

Apologies for not submitting a fix, after removing the emitter.receiveEvent we noticed some callbacks on the Map component were no longer firing. One of which was onDidFinishLoadingStyle. Has anyone else noticed the same?

@kristfal
Copy link
Member

kristfal commented Jun 21, 2019

@makirby is this in production env, or only in development mode after a reload of the JS side?

@makirby
Copy link
Contributor

makirby commented Jun 26, 2019

I could still get it to happen in production, albeit more intermittently.
I suspected it was something to do with throttling so did a rebuild with the didStartRenderingFrame & didFinishRenderingFrame listeners removed, with those removed the onDidFinishLoadingStyle events would then fire every time, in dev and production.

@mfazekas
Copy link
Contributor

@makirby might or might not related to #177

@makirby
Copy link
Contributor

makirby commented Jun 27, 2019

@mfazekas yeah that looks like the cause

@mattijsf
Copy link
Member

@mfazekas Thanks for pointing it out. Yes it was caused by the same problem, just added a commit to the PR that fixes the problem for these map related events.

@Beamanator
Copy link

Hmmm I'm trying to get a ShapeSource's onPress to work, but it's not running the function at all! Any advice?

Some important packages I'm using:

"@react-native-mapbox-gl/maps": "^7.0.4",
"react": "^16.9.0",
"react-native": "^0.60.5",

Platfrom: [Android]
SDK Version [28]

I've been using very similar code to this example but including onPress={(e) => console.log("shapesource press", e)} prop in the ShapeSource component.

I am not getting any errors or anything, and no console log messages either. Is anyone else running into a similar issue with the above packages?

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 a pull request may close this issue.

9 participants