fix(AIRMap): include map rotation in zoom computing & fix flyovers maps zoom constrains [take II]

[jan-kozinski]

Does any other open PR do the same thing?

No

What issue is this PR fixing?

#4880

The zoom limit algorithm does not take map rotation into account and also it does not work for flyover maps. Flyover maps are not typed and not documented.

How did you test this PR?

iPhone 14 Pro simulator, I logged the zoom values with NSLog. The new algorithm produced same zoom values as the old one did when map was not rotated. When the map was rotated, the old algorithm tended to be unpredictable.

[jan-kozinski]

@salah-ghanim this is take two of closed PR #4892

[salah-ghanim]

@jan-kozinski thanks a lot for re-submitting the pull request.

I added couple of comments that I think need to be addressed before merging the code. specifically I believe there is a way to implement max/min zoom based on having a setter function for both properties and calculating how much that would be in meters (based on screen dimensions).

let me know if you can take care of that or if I should try to do it myself.

[jan-kozinski]

@salah-ghanim Hi, I can't find the comments, perhaps you didn't submit the review yet? :D I'll do my best to address them, though I'm not sure how to compute the distance in meters based on screen dimensions.

[salah-ghanim]

@jan-kozinsk you should be able to see all comments in the "files changed" tab of the pull request.

[jan-kozinski]

Screen.Recording.2023-12-18.at.15.47.52.mov

[jan-kozinski]

Hi @salah-ghanim I still can't find the comments, even under the files changed tab :/. I've included a recording in my previous mesage

[ahinkle]

@salah-ghanim I don't see your comments either. Would be happy to help with this PR :)

[salah-ghanim]

@jan-kozinski @ahinkle strange that you guys don't see the feedback, I see it even in guest mode:
#4905 (review)

anyhow I also included screenshots of the comments so that you see my feedback

[salah-ghanim]

@jan-kozinski thank you so much for the new pull request, but I realised that:

1. we're still doing calculations each time mapRegion changes when we could avoid that by using MKMapView's constraints functionality.
2. we're adding "magic code" / opinionated that maps zoom level to visible meters that I personally can't confirm to match especially knowing that there are multiple device screen sizes out there so, seeing:
   CLLocationDistance minDistance = pow(2, (26 - maxZoomLevel));

doesn't set with me well.

but good news I see the way forward and it's much simpler that what we're trying to do.

1. we should expose new Properties for iOS zoom that exactly match iOS APIs: MKMapCameraZoomRange (min, max) in meters and make them available to JS / react-native users.

if such values are provided you can set the MapKit camera zoom range using those and avoid the use of the current zoom logic

2. disable the current zoom logic for satellite maps completely.
3. use Apple APIs for zoom whenever the min / max params are provided and stop trying to control the zoom level on each regionChange ... apple will ensure that the camera is limited since you provide the limitation.

by "current logic" I mean the current one in the library not the one in this pull request.

maybe to summarise again:

1. disable old logic for satellite map to avoid the bug we know about.
2. introduce new Props for iOS maps that match exactly MKMapView.cameraRange: https://developer.apple.com/documentation/mapkit/mkmapview/3114301-camerazoomrange?language=objc
3. if cameraRange is used, disable the current "reactive / onRegionChange" logic because iOS internals should take care of the limitation.

[salah-ghanim]

@jan-kozinski thanks for the new update some tiny feedback:

1. reactiveZoomConstraintsEnabled I would rename it to: legacyZoomConstraintslEnabled
2. in the doc, please don't mention satellite as a reason why minCenterCoordinateDistance / maxCenterCoordinateDistance .. rather this is the new APIs and should be used, please mark old zoom values for deprecations.
   please make sure to note that this is only relevant for iOS MapKit

once done I think we can finally merge :)

[jan-kozinski]

Done @salah-ghanim. I didn't use the @deprecated marker for old zoom levels properties in Mapview.tsx, as those properties don't have any alternative for google maps. Though I added a note that it is deprecated on Apple Maps

[react-native-maps-bot]

🎉 This PR is included in version 1.10.2 🎉

The release is available on:

• npm package (@latest dist-tag)
• GitHub release

Your semantic-release bot 📦🚀

[ahinkle]

@jan-kozinski,

I'm still seeing this bug on the hybridFlyover map type. Any ideas there?

Screen.Recording.2024-02-10.at.9.17.23.PM.mov

[jan-kozinski]

Hi @ahinkle I think you're using Expo. If so, you have to rebuild your project, not only update the library and start