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

[android] Add parameter to disable the moving on marker press #676

Merged
merged 2 commits into from
Oct 12, 2016

Conversation

mlanter
Copy link
Contributor

@mlanter mlanter commented Oct 11, 2016

On iOS (Apple Maps) clicking a marker doesn't move the map but on Android clicking a marker causes the map to move. This PR adds a property that lets that behavior be disabled.

Test plan (on Android):

  • Verify the callout still appears when clicking a pin
  • Verify the map moves if the property isn't specified
  • Verify the map doesn't move if the property is set to false.

Before:
before

After:
after

@@ -165,6 +165,11 @@ public void setLoadingEnabled(AirMapView view, boolean loadingEnabled) {
view.enableMapLoading(loadingEnabled);
}

@ReactProp(name="moveOnMarkerPress", defaultBoolean = true)
public void setMoveOnMarkerPress(AirMapView view, boolean moveOnPress) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

space between =

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Just to clarify, should I add spaces around the = or remove it? The other methods in the file use both standards.

Copy link
Collaborator

Choose a reason for hiding this comment

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

space around =. I guess I missed the other props that do the same.

I think the general syntax is to have space in between. Would be great if you can adjust the others to match as well.

@christopherdro
Copy link
Collaborator

LGTM! Small nit pick comment

@christopherdro
Copy link
Collaborator

@spikebrehm @lelandrichardson LGTM!

Thoughts on getting this is?

@spikebrehm
Copy link

Beautiful, thank you!

@spikebrehm spikebrehm merged commit 722f5e9 into react-native-maps:master Oct 12, 2016
@spikebrehm
Copy link

LMK if you need me to cut a new release for this.

@christopherdro
Copy link
Collaborator

Would be great with the commit for react native 0.35.

@mlanter mlanter deleted the move_pr branch October 12, 2016 01:55
@saschaknobloch saschaknobloch mentioned this pull request Dec 2, 2016
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.

3 participants