iOS google map fixes

[Agontuk]

This is an attempt to fix several issues with google maps in iOS. Mainly -

• Fix onMapReady not getting called after first time
• Fix initialRegion lat/lng delta not setting properly
• Fix setRegion method getting called even when map is not ready
• Prevent onRegionChange/ onRegionChangeComplete event until initialRegion or region is set.

[alvelig]

This would have conflicts with #1950. Can you please also review that PR and say if you can rebase on it and it does not have contradictions with your point

[Agontuk]

@alvelig, I've asked the author of PR 1950 to try it out. For this PR, when onLayout is called, map won't be ready on native side. So this.state.isReady will be false and the block won't be executed. When map is finally ready, _onMapReady will be called which'll set the initialRegion or region. That way region will be set correctly. Hope I clearly explained this.

[alvelig]

Just a question: is it possible to do without setting initialRegion or region at all? I use it without setting region, calling animateToRegion when I need to change region and actually don't care about the initialRegion.

[Agontuk]

hmm, it'll be an issue if neither initialRegion nor region is set by user. Then region change events will not be called for this PR. Just curious, if you don't set either props, your map will point to a random location with being zoomed out. Is this a valid use case ?

Edit: while in this topic, is it also a possible use case where user will set both initialRegion & region ?

[FaBeyyy]

Hoping for a quick merge need this!

[alvelig]

@Agontuk setting both initialRegion and region is ok, I don't see any problems.

According to what I expect, the map should have automatically set region according to region settings (showing the country selected). But I'm not quite sure about that.

[M3po]

I tried this and i am still having the problem if i set the region and try to move the map it resets to region coordinate (while it is undefined it works ok).

[Agontuk]

@M3po, that's expected I think. If you use region prop, you have to keep it in a state and update the state when it changes. If you don't want to keep it in a state, just use initialRegion.

[Agontuk]

There's one thing to note, if this gets merged, user won't be able to get region change events if they set neither initialRegion nor region. So we may need to warn user to pass at least one prop.

[rborn]

LGTM

@Agontuk related to the event triggering only of there is an initialRegion or region set, I get your point, but doesn't the map actually sets one "internally" by default - 0,0 ?
So in this case can't we detect this?

[Agontuk]

@rborn, I'll take a look. I think some codes are not needed anymore after the latest commit.

[rborn]

LGTM

@alvelig 🐽

[Agontuk]

Ok, now it's possible to receive region change events without providing initialRegion or region. Only downside I found is that if user sets region prop, it'll trigger onRegionChangeComplete twice on first load.

[rborn]

@Agontuk like every time it changes the region or only the first time from nothing to something

[Agontuk]

only first time, when map first loads with a provided region.

[rborn]

If it's too hard to fix that, maybe we could just go ahead with it as I think is an edge case. @alvelig what do you think?

[Agontuk]

I added logger to check why it's happening and found that region slightly changes between two events (ex. delta from 0.008999999999999999 to 0.00899993600424053). Most probably gmap is internally adjusting the camera and triggering the change event, but not sure though.

[rborn]

Oh yes, this happens to me as well, and if you make the region to depend on status will create a crazy loop.
I fixed it with a workaround but it's pretty weird.

[Agontuk]

@rborn, yeah if user use setState on onRegionChange event to update the region, it'll go on a crazy loop. Because react batches the state changes and does not update it immediately, so map will try to reset to the previous region over and over. So it would be a bad practice to use setState on onRegionChange event.

[rborn]

Ok, let's wait for @alvelig and see if we merge :)

[rborn]

@Agontuk I just tested your branch in an app that requires multiple maps and it seems to work really nice. Great job! 🤗

[rborn]

@alvelig 🐽

[alvelig]

Meeeeerge!!!

[rborn]

@christopherdro what about a new push to npm? 🤗

[willbattel]

My Google Map on iOS is never calling onRegionChangeComplete, but does call onRegionChange. Could it be related to one of the issues solved with this PR?

I'm waiting to create an issue since it may be related to this. I don't need onRegionChange, since updating the region state every frame is ill-advised, but I really need onRegionChangeComplete to work or else the map continuously snaps back to its initial region.

[alvelig]

@wbattel4607 as far as I understand, no. Have a look at issue#311, if not, create a new issue, but please, make a project to reproduce your problem. So that we can git clone it and see.

[willbattel]

@alvelig turns out I was just being an idiot. My IDE had crashed before I saved the Map component's JSX attribute from onRegionChange to onRegionChangeComplete so it wasn't set to point at the correct function.

On another note, do you know when we might see these fixes pushed through npm in a new version?

[alvelig]

That's one of the points we ask people to make a reproducible demo project before creating an issue 🤣

Patience is our best friend