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

fixed region/initialRegion null overrides of this.props #1715

Merged

Conversation

atkit
Copy link
Contributor

@atkit atkit commented Oct 19, 2017

I noticed that
props.region and props.initialRegion are always come as null. (even if set for <MapView />)

This change gives ability to set region and initialRegion to something that come from the top.

Also it causes big performance issue on RN side when these props are null, (I'll try to mitigate that with different PR)

@atkit atkit force-pushed the fix-region-intial-region branch from 5e4834a to ecfe964 Compare October 20, 2017 08:02
aaronbuchanan added a commit to aaronbuchanan/react-native-maps that referenced this pull request Oct 20, 2017
onBadge pushed a commit to onBadge/react-native-maps that referenced this pull request Oct 31, 2017
onBadge pushed a commit to onBadge/react-native-maps that referenced this pull request Oct 31, 2017
@larryranches
Copy link

Can this get merged soon? This bug fix would help a lot as i'm also having issues with #1742

@atkit
Copy link
Contributor Author

atkit commented Nov 2, 2017

probably someone from AirBnb can merge it

@larryranches
Copy link

@atkit I did test your fixes and have confirmed that this fixes #1742. Great Work!

@gpeal I'm new to this library, how often is the release cycle or PRs get approved and merged?

@aaronbuchanan
Copy link

@larryranches in my experience it can take quite a while for approved changes to make it in, I had to maintain my own fork to speed things up

@atkit atkit force-pushed the fix-region-intial-region branch from ecfe964 to e5d9bce Compare November 20, 2017 08:41
@rborn
Copy link
Collaborator

rborn commented Dec 12, 2017

@atkit could you please update the PR (so the conflict goes away) maybe we can merge this? 💪
Thanks.

@rborn
Copy link
Collaborator

rborn commented Dec 14, 2017

LGTM @alvelig

@rborn
Copy link
Collaborator

rborn commented Dec 14, 2017

@christopherdro

@christopherdro
Copy link
Collaborator

Should be just move the spread down to the bottom?

@rborn
Copy link
Collaborator

rborn commented Dec 14, 2017

@christopherdro I think yes, because otherwise the default will overwrite the spread.

@christopherdro christopherdro merged commit 19c30b2 into react-native-maps:master Dec 14, 2017
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