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

[RN][iOS][google] Set region only when view has width&height #785

Merged
merged 1 commit into from
Nov 10, 2016

Conversation

gilbox
Copy link
Contributor

@gilbox gilbox commented Nov 10, 2016

  • Fix type issue in AIRMapManager
  • Setup Gemfile in example/ios dir to avoid problems with different versions of cocoapods
  • Update examples-setup.md to use bundler
  • Change MapView so that we only set the native region prop when there is a width and height. GoogleMaps iOS requires the width and height to properly calculate the map zoom level.

@spikebrehm @lelandrichardson

- Fix type issue in AIRMapManager
- Setup Gemfile in example/ios dir to avoid problems with different versions of cocoapods
- Update examples-setup.md to use bundler
- Change MapView so that we only set the native region prop when there is a width and height. GoogleMaps iOS requires the width and height to properly calculate the map zoom level.
Copy link

@spikebrehm spikebrehm left a comment

Choose a reason for hiding this comment

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

LGTM! Thanks

const { region, initialRegion } = this.props;
if (region && this.state.isReady) {
this.map.setNativeProps({ region });
} else if (initialRegion && this.state.isReady) {

Choose a reason for hiding this comment

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

Why is this no longer needed?

Choose a reason for hiding this comment

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

Because of the change to _onLayout()?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

For google+ios, this will cause a region calculation that is incorrect. For mapkit+ios it will cause the map to redundantly set the region because it will happen in _onLayout anyway.


```
cd example
npm install
cd ios
pod install
bundle install
bundle exec pod install

Choose a reason for hiding this comment

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

Do we really need both? Can it just be the second?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@spikebrehm unless there is a way to add a command to run pod install from the Gemfile I think we need both.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

oh are you saying that bundle exec runs bundle install automatically? I don't know much about bundler tbh

Choose a reason for hiding this comment

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

oh no nevermind brainfart

@spikebrehm spikebrehm merged commit 498896f into master Nov 10, 2016
@spikebrehm spikebrehm deleted the gil/set-region-after-layout branch November 10, 2016 20:46
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.

None yet

2 participants