rnmapbox / maps

A Mapbox react native module for creating custom maps
MIT License
2.19k stars 834 forks source link

[Bug]: `onPress` not working on Touchable Child component of MarkerView [iOS] #2751

Closed santhosh-umapathi-appymango closed 1 year ago

santhosh-umapathi-appymango commented 1 year ago

Mapbox Implementation

Mapbox

Mapbox Version

v10

Platform

iOS

@rnmapbox/maps version

10.0.0-rc.5

Standalone component to reproduce

import React from 'react';
import { View } from 'react-native';
import Mapbox from '@rnmapbox/maps';

const App = () => {

const markers = [
  { coordinate: [12.338, 45.4385] },
  { coordinate: [10.338, 47.4385] },
  { coordinate: [8.338, 43.4385] },
];

return (
<Mapbox.MapView style={styles.map}>
    <Mapbox.Camera zoomLevel={5} centerCoordinate={[12.338, 45.4385]} />
    {/* Rendering Markers */}
    {markers.map(marker => (
    <Mapbox.MarkerView coordinate={marker.coordinate} allowOverlap>
        <TouchableOpacity onPress={()=>console.log("pressed", marker.coordinate)}>
           <View style={{width: 10, height: 10, backgroundColor: 'red'}} />
        </TouchableOpacity>
    </Mapbox.MarkerView>
    ))}
</Mapbox.MapView>
)}

Observed behavior and steps to reproduce

Touchable Child component inside MarkerView, onPress is not working on iOS. But works on Android. Seems like Map layer is disabling the Markers onPress events

Expected behavior

Touchable Child component inside MarkerView, onPress to work on iOS

Notes / preliminary analysis

No response

Additional links and references

No response

mfazekas commented 1 year ago

@santhosh-umapathi-appymango please include a complete component. markers, onPress is not defined.

santhosh-umapathi-appymango commented 1 year ago

Sure, @mfazekas i have updated the code to a working example. Thanks for your time

mfazekas commented 1 year ago

@santhosh-umapathi-appymango I wasn't able to reproduce the issue. I've trired on iOS simulator. And both feedback and both events worked fine.

Used the following component to try to reproduce:

import React from 'react';
import { View, TouchableOpacity } from 'react-native';
import Mapbox from '@rnmapbox/maps';

const App = () => {
  const markers = [
    { coordinate: [12.338, 45.4385], key: 'k1' },
    { coordinate: [10.338, 47.4385], key: 'k2' },
    { coordinate: [8.338, 43.4385], key: 'k3' },
  ];

  return (
    <Mapbox.MapView style={{ flex: 1 }}>
      <Mapbox.Camera zoomLevel={5} centerCoordinate={[12.338, 45.4385]} />
      {/* Rendering Markers */}
      {markers.map((marker) => (
        <Mapbox.MarkerView
          coordinate={marker.coordinate}
          allowOverlap
          key={marker.key}
        >
          <TouchableOpacity
            onPress={() => console.log('pressed', marker.coordinate)}
          >
            <View style={{ width: 10, height: 10, backgroundColor: 'red' }} />
          </TouchableOpacity>
        </Mapbox.MarkerView>
      ))}
    </Mapbox.MapView>
  );
};

export default App;
 LOG  pressed [10.338, 47.4385]
 LOG  pressed [12.338, 45.4385]
 LOG  pressed [10.338, 47.4385]
 LOG  pressed [12.338, 45.4385]
 LOG  pressed [10.338, 47.4385]
 LOG  pressed [12.338, 45.4385]
 LOG  pressed [10.338, 47.4385]
 LOG  pressed [10.338, 47.4385]
 LOG  pressed [10.338, 47.4385]

image

mfazekas commented 1 year ago

Closing as cannot reproduce

anfinil commented 1 year ago

The same issue for me

trevorarmstrong commented 1 year ago

Yeah can we reactivate this, it doesn't work for me either.

If I take the sample @mfazekas provided above and pop it into the example app it does work. However if I make my app just that same snippet press events don't work.

@mfazekas I'm using 10.0.0-rc.10 and react native 0.70.6. Are there any other deps that might be important for getting this functionality to work?

mfazekas commented 1 year ago

@trevorarmstrong thanks much for verifying the example app. I think our example app is rn 69.7. Will try in new app if I have time.

Santhosh-Umapathi commented 1 year ago

@mfazekas , @trevorarmstrong , Yes I forgot to update the versions I was using.

"react-native": "0.71.1" iOS Simulator: 16.4

Hope this helps to pinpoint the issue.

mfazekas commented 1 year ago

Folks are you using mapbox v10 implementation? Do you have

$RNMapboxMapsImpl = 'mapbox'

in your podfile?

I've tried with a new project with 0.71.1 and click was working for me there as well

anfinil commented 1 year ago

RNMapboxMapsImpl

yes, I'm using v10 mapbox ("@rnmapbox/maps@^10.0.0-rc.7") and "react-native": "0.71.5". I assume that problem in Mapbox Maps for iOS version 10.12.1, before it was 10.11.

Podfile has $RNMapboxMapsImpl = 'mapbox'

mfazekas commented 1 year ago

I have

% cat ios/Podfile.lock| grep MapboxMaps
  - MapboxMaps (10.12.0):
    - MapboxMaps (~> 10.12.0)
    - MapboxMaps (~> 10.12.0)
    - MapboxMaps
  MapboxMaps: a9978d13d1cba0403c5bdd2ab3b71418924075d2
anfinil commented 1 year ago

cat ios/Podfile.lock| grep MapboxMaps

cat ios/Podfile.lock| grep MapboxMaps
- MapboxMaps (10.12.1):
- MapboxMaps (~> 10.12.0)
- MapboxMaps (~> 10.12.0)
- MapboxMaps
MapboxMaps: a673c0923f645dc6f90e40bdc034b8567d8dead4

can you try to update pod and run example?

santhosh-umapathi-appymango commented 1 year ago

@mfazekas , could you paste your podfile contents please ?

mfazekas commented 1 year ago

@anfinil it's working fine for me with 10.2.1 as well, I do get touch feedback and:

 LOG  pressed [12.338, 45.4385]
 LOG  pressed [8.338, 43.4385]
% cat ios/Podfile.lock| grep MapboxMaps
  - MapboxMaps (10.12.1):
    - MapboxMaps (~> 10.12.0)
    - MapboxMaps (~> 10.12.0)
    - MapboxMaps
  MapboxMaps: a673c0923f645dc6f90e40bdc034b8567d8dead4
    "react-native": "0.71.1"

@Santhosh-Umapathi my pod file is:

require_relative '../node_modules/react-native/scripts/react_native_pods'
require_relative '../node_modules/@react-native-community/cli-platform-ios/native_modules'

platform :ios, min_ios_version_supported
prepare_react_native_project!

$RNMapboxMapsImpl = 'mapbox'

# If you are using a `react-native-flipper` your iOS build will fail when `NO_FLIPPER=1` is set.
# because `react-native-flipper` depends on (FlipperKit,...) that will be excluded
#
# To fix this you can also exclude `react-native-flipper` using a `react-native.config.js`
# ```js
# module.exports = {
#   dependencies: {
#     ...(process.env.NO_FLIPPER ? { 'react-native-flipper': { platforms: { ios: null } } } : {}),
# ```
flipper_config = ENV['NO_FLIPPER'] == "1" ? FlipperConfiguration.disabled : FlipperConfiguration.enabled

linkage = ENV['USE_FRAMEWORKS']
if linkage != nil
  Pod::UI.puts "Configuring Pod with #{linkage}ally linked Frameworks".green
  use_frameworks! :linkage => linkage.to_sym
end

target 'rnrnmapboxts' do
  config = use_native_modules!

  # Flags change depending on the env values.
  flags = get_default_flags()

  use_react_native!(
    :path => config[:reactNativePath],
    # Hermes is now enabled by default. Disable by setting this flag to false.
    # Upcoming versions of React Native may rely on get_default_flags(), but
    # we make it explicit here to aid in the React Native upgrade process.
    :hermes_enabled => flags[:hermes_enabled],
    :fabric_enabled => flags[:fabric_enabled],
    # Enables Flipper.
    #
    # Note that if you have use_frameworks! enabled, Flipper will not work and
    # you should disable the next line.
    :flipper_configuration => flipper_config,
    # An absolute path to your application root.
    :app_path => "#{Pod::Config.instance.installation_root}/.."
  )

  target 'rnrnmapboxtsTests' do
    inherit! :complete
    # Pods for testing
  end

  pre_install do |installer|
    $RNMapboxMaps.pre_install(installer)
  end

  post_install do |installer|
    $RNMapboxMaps.post_install(installer)
    react_native_post_install(
      installer,
      # Set `mac_catalyst_enabled` to `true` in order to apply patches
      # necessary for Mac Catalyst builds
      :mac_catalyst_enabled => false
    )
    __apply_Xcode_12_5_M1_post_install_workaround(installer)
  end
end
anfinil commented 1 year ago

@anfinil it's working fine for me with 10.2.1 as well, I do get touch feedback and:

 LOG  pressed [12.338, 45.4385]
 LOG  pressed [8.338, 43.4385]
% cat ios/Podfile.lock| grep MapboxMaps
  - MapboxMaps (10.12.1):
    - MapboxMaps (~> 10.12.0)
    - MapboxMaps (~> 10.12.0)
    - MapboxMaps
  MapboxMaps: a673c0923f645dc6f90e40bdc034b8567d8dead4
    "react-native": "0.71.1"

@Santhosh-Umapathi my pod file is:

require_relative '../node_modules/react-native/scripts/react_native_pods'
require_relative '../node_modules/@react-native-community/cli-platform-ios/native_modules'

platform :ios, min_ios_version_supported
prepare_react_native_project!

$RNMapboxMapsImpl = 'mapbox'

# If you are using a `react-native-flipper` your iOS build will fail when `NO_FLIPPER=1` is set.
# because `react-native-flipper` depends on (FlipperKit,...) that will be excluded
#
# To fix this you can also exclude `react-native-flipper` using a `react-native.config.js`
# ```js
# module.exports = {
#   dependencies: {
#     ...(process.env.NO_FLIPPER ? { 'react-native-flipper': { platforms: { ios: null } } } : {}),
# ```
flipper_config = ENV['NO_FLIPPER'] == "1" ? FlipperConfiguration.disabled : FlipperConfiguration.enabled

linkage = ENV['USE_FRAMEWORKS']
if linkage != nil
Pod::UI.puts "Configuring Pod with #{linkage}ally linked Frameworks".green
use_frameworks! :linkage => linkage.to_sym
end

target 'rnrnmapboxts' do
config = use_native_modules!

# Flags change depending on the env values.
flags = get_default_flags()

use_react_native!(
  :path => config[:reactNativePath],
  # Hermes is now enabled by default. Disable by setting this flag to false.
  # Upcoming versions of React Native may rely on get_default_flags(), but
  # we make it explicit here to aid in the React Native upgrade process.
  :hermes_enabled => flags[:hermes_enabled],
  :fabric_enabled => flags[:fabric_enabled],
  # Enables Flipper.
  #
  # Note that if you have use_frameworks! enabled, Flipper will not work and
  # you should disable the next line.
  :flipper_configuration => flipper_config,
  # An absolute path to your application root.
  :app_path => "#{Pod::Config.instance.installation_root}/.."
)

target 'rnrnmapboxtsTests' do
  inherit! :complete
  # Pods for testing
end

pre_install do |installer|
  $RNMapboxMaps.pre_install(installer)
end

post_install do |installer|
  $RNMapboxMaps.post_install(installer)
  react_native_post_install(
    installer,
    # Set `mac_catalyst_enabled` to `true` in order to apply patches
    # necessary for Mac Catalyst builds
    :mac_catalyst_enabled => false
  )
  __apply_Xcode_12_5_M1_post_install_workaround(installer)
end
end

can you push the example project to github? I'll try to test on my environment and check the difference.

santhosh-umapathi-appymango commented 1 year ago

@mfazekas , @anfinil , in my case, i have the below setup, I need to use use_frameworks as required by RN Firebase for push notifications setup. Could that be the reason ?

# Push Notification Setup
use_frameworks! :linkage => :static # With Hermes enabled
$RNFirebaseAsStaticFramework = true
# Mapbox Config
$RNMapboxMapsImpl = 'mapbox'
mfazekas commented 1 year ago

@anfinil check out this repo: https://github.com/mfazekas/rnmapbox-examples/

anfinil commented 1 year ago

I've updated library to "^10.0.0-rc.11" and now it works.

trevorarmstrong commented 1 year ago

Does not work for us still, I'm going to have to strip our app down to the bare bones and start building it back up to try to figure out what's wrong here

trevorarmstrong commented 1 year ago

@mfazekas for us it's the react navigation drawer, if we render our component inside a drawer screen no press handlers are fired. Worked fine on v8.5.0.

@santhosh-umapathi-appymango any chance you're using a react navigation drawer?

santhosh-umapathi-appymango commented 1 year ago

@trevorarmstrong , I am just using the following from react navigation. Not the Navigation drawer.

"@react-navigation/bottom-tabs": "^6.5.3",
"@react-navigation/native": "^6.1.2",
"@react-navigation/native-stack": "^6.9.8"
santhosh-umapathi-appymango commented 1 year ago

@anfinil , upgrading the lib to ^10.0.0-rc.11 does not work in my case, I doubt navigation can be the reason for me as well. Need to check it as a nav free component.

trevorarmstrong commented 1 year ago

Def seems react navigation related, which means it's going to be hit a lot once this goes to a wider release.

I've given up on MarkerView and ported my stuff to PointAnnotation, but that has it's own problems (broken shadows, janky refresh logic). Would love to go back to MarkerView when it's working again.

mfazekas commented 1 year ago

Our example app uses @react-navigation/native and @react-navigation/native-stack and we don't see the issue. It would be great if someone could post simplified app to reproduce.

sascha-kaleidoscode commented 1 year ago

Not a solution, but a possible workaround if you already include react-native-gesture-handle in your project. I switched from a Pressable component to a TapGestureHandler and it seems to be stable.

anfinil commented 1 year ago

@anfinil , upgrading the lib to ^10.0.0-rc.11 does not work in my case, I doubt navigation can be the reason for me as well. Need to check it as a nav free component.

Updating to rc.13 breaks things again. ) Update to MapboxMaps 10.12.2 (was 10.12.1) didn't help. Replacing TouchableOpacity inner component with TapGestureHandler helps.

santhosh-umapathi-appymango commented 1 year ago

As others mentioned, making use of TapGestureHandlerfixed the situation. Thanks @sascha-kaleidoscode

here is how I used it.

 <Mapbox.MarkerView {...{id, coordinate, allowOverlap}}>
      <TapGestureHandler onBegan={onPress}>
        <View>
          <Icon
            name="MarkerFilled"
            size={size}
          />
        </View>
      </TapGestureHandler>
    </Mapbox.MarkerView>
MichaelDanielTom commented 1 year ago

Any update on if there is intention to fix this, whether it is an issue within MapBox's iOS sdk that they will fix, or one that this library would make?

mfazekas commented 1 year ago

@MichaelDanielTom see - https://github.com/rnmapbox/maps/issues/2751#issuecomment-1493315694 and https://github.com/rnmapbox/maps/issues/2751#issuecomment-1500795156

I wasn't able to reproduce the issue. Please try to figure out under what conditions does the issue happens. Then we can look into it, thanks for the understanding.

Ajmal0197 commented 1 year ago

Solution:

  const doubleTapRef = useRef();

    <MarkerView ...>
      <TapGestureHandler
        waitFor={doubleTapRef}
        onActivated={() => {
          console.log("SINGLE TAP");
        }}
      >
        <TapGestureHandler
          maxDelayMs={250}
          ref={doubleTapRef}
          numberOfTaps={2}
          onActivated={() => {
            console.log("DOUBLE TAP");
          }}
        >
          <Animated.View
            style={{ width: 100, height: 100, backgroundColor: "red" }}
          />
        </TapGestureHandler>
      </TapGestureHandler>
    </MarkerView>
Nachox07 commented 11 months ago

I have the same issue with the TouchableOpacity, and the workaround from @Ajmal0197 worked, thanks. In my case, I need to know when the marker is pressed. Otherwise, the event is mistakenly dispatched when the zooming gesture is performed. The versions I'm using for several core libraries of my app are:

"react-native": "0.72.4",
"@rnmapbox/maps": "10.0.15",
"react-native-reanimated": "3.3.0",
"@react-navigation/native": "^6.1.7",
"@react-navigation/native-stack": "^6.9.13"

The simplest marker implementation I tried:

import Mapbox from '@rnmapbox/maps';
import React from 'react';
import { View } from 'react-native';

import { TouchableOpacity } from 'react-native';
import { Geo } from '../../../types/geo';

type MarkerProps = {
  id: string;
  point: Geo.Point;
};

const Marker = ({ id, point }: MarkerProps) => {
  return (
    <Mapbox.MarkerView id={id} coordinate={point}>
      <TouchableOpacity onPress={() => console.log('pressed')}>
        <View style={{ width: 10, height: 10, backgroundColor: 'red' }} />
      </TouchableOpacity>
    </Mapbox.MarkerView>
  );
};

export default Marker;

Regarding the MapView, I tried a simple one without any props attached. If you need more details, I can prepare a more detailed example.