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

Add onPress for polygons and polylines on iOS and Android #760

Merged
merged 2 commits into from
Nov 10, 2016

Conversation

frankrowe
Copy link
Contributor

Adds onPress handlers for polygons and polylines.

  • On Android, uses the native setOnPolygonClickListener and setOnPolylineClickListener
  • On iOS, loops overlays in handleMapTap and finds if the tap is inside a polygon, or if the closest polyline is within 10 pixels of the tap.

@markudevelop
Copy link
Contributor

markudevelop commented Nov 3, 2016

Oh this is awesome why not merged yet? :)
I need this as well

@markudevelop
Copy link
Contributor

@felipecsl Hi, can you please take a look? 👍

@felipecsl
Copy link
Contributor

felipecsl commented Nov 7, 2016

Android code looks mostly good, just some nits 👍

Copy link
Contributor

@felipecsl felipecsl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just some minor changes

Map map = MapBuilder.of(
"onPress", MapBuilder.of("registrationName", "onPress")
);
return map;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

return MapBuilder.of() directly, no need to declare a variable 👍

Map map = MapBuilder.of(
"onPress", MapBuilder.of("registrationName", "onPress")
);
return map;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same

@Override
public void onPolygonClick(Polygon polygon) {
WritableMap event;
event = makeClickEventData(polygon.getPoints().get(0));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

WritableMap event = makeClickEventData(polygon.getPoints().get(0));

@Override
public void onPolylineClick(Polyline polyline) {
WritableMap event;
event = makeClickEventData(polyline.getPoints().get(0));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same

@frankrowe
Copy link
Contributor Author

Cool, android changes made. Let me know if any iOS changes are needed. I have tested this in my own applications, but not extensively.

@felipecsl
Copy link
Contributor

android changes look good to me. someone else should review the iOS ones (i cant :))

@markudevelop
Copy link
Contributor

@felipecsl Thanks you.
not sure who we can ping about the iOS part any ideas?

@felipecsl
Copy link
Contributor

maybe @spikebrehm can help

@markudevelop
Copy link
Contributor

@spikebrehm Hi, any chance you can take a look at this? I really need this thing, thank you so much!

@spikebrehm
Copy link

Thanks @frankrowe, this looks good!

Can you please update the example app with example usage of this feature? I'll go ahead and merge now, and if you can please follow up with another PR with the examples, I would appreciate it 🍻.

@spikebrehm spikebrehm merged commit 67f51b1 into react-native-maps:master Nov 10, 2016
@frankrowe
Copy link
Contributor Author

Great! I will add an example as soon as I can.

@markudevelop
Copy link
Contributor

@frankrowe it should be used like that?

<MapView.Polygon
   onPress={(e) => {
    console.log(e, 'pressed polygon');
}}/>

I can't make it work for some reason

@frankrowe
Copy link
Contributor Author

Hey @voidale yes that is correct usage. I added examples in PR #793 can you check if that works?

@npineda
Copy link

npineda commented Nov 18, 2016

I installed the update from github(not npm since the version hasn't been updated) and onPress is working on Android. Using Genymotion emulator Android API 23

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.

5 participants