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

Fixes #470. Support legalLabelInsets on Apple Maps #840

Merged
merged 4 commits into from
Dec 6, 2016

Conversation

scarlac
Copy link
Contributor

@scarlac scarlac commented Nov 29, 2016

Fixes #470. This will add support for legalLabelInsets prop on iOS with Apple Maps. Code is adapted from RN core.

This will add support for `legalLabelInsets` prop on iOS with Apple Maps. Code is adapted from RN core.
@scarlac scarlac changed the title Fixes #470 for iOS and Apple Maps Fixes #470. Support legalLabelInsets on Apple Maps Nov 29, 2016
@gilbox
Copy link
Contributor

gilbox commented Nov 29, 2016

Thanks! We should probably avoid -> notation. Would you mind adding an example for this?

@scarlac
Copy link
Contributor Author

scarlac commented Nov 29, 2016

@gilbox Yeah, let's keep it consistent. I've removed it.

@gilbox
Copy link
Contributor

gilbox commented Nov 30, 2016

@scarlac will you be able to add an example?

@scarlac
Copy link
Contributor Author

scarlac commented Nov 30, 2016

@gilbox Ah! I am assuming you mean updating the documentation. Yes, I'll get right on it tomorrow and update this PR :)

@gilbox
Copy link
Contributor

gilbox commented Nov 30, 2016

@scarlac I was talking about adding an example in the example/examples dir. That's how I smoke-test PRs since we don't have specs.

@scarlac
Copy link
Contributor Author

scarlac commented Dec 2, 2016

@gilbox I've added an example to demonstrate the usefulness of the legalLabelInsets. I think it's fairly simple while showing a very realistic scenario of when it could be of use. Personally, I'm using it in an advanced way but opted for simplicity in the example, for the sake of understanding.

Let me know if there's more you need before it's ready for a merging.

@gilbox
Copy link
Contributor

gilbox commented Dec 6, 2016

lgtm but I'd like to get a second opinion re: the async op

@gilbox gilbox merged commit 8e7f854 into react-native-maps:master Dec 6, 2016
@gilbox
Copy link
Contributor

gilbox commented Dec 6, 2016

thanks @scarlac ( ◠‿◠ )

@scarlac
Copy link
Contributor Author

scarlac commented Dec 6, 2016

My pleasure! 👍

@scarlac scarlac deleted the patch-1 branch December 8, 2016 13:50
Exilz pushed a commit to archriss/react-native-maps that referenced this pull request Dec 9, 2016
…eact-native-maps#840)

* Fixes react-native-maps#470 for iOS and Apple Maps

This will add support for `legalLabelInsets` prop on iOS with Apple Maps. Code is adapted from RN core.

* Pluralize updateLegalLabelInset(s) for consistency

* Removed arrow notation

* Added legalLabelInsets example
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.

2 participants