search

rnmapbox / maps

A Mapbox react native module for creating custom maps
MIT License
2.27k stars 849 forks source link

Diff between MapView's getVisibleBounds() method on Android and iOS #2202

Closed alexanderoskin closed 2 years ago

alexanderoskin commented 2 years ago

New Feature

Return the same value from MapView's getVisibleBounds() method and event.properties.visibleBounds property of the onRegionDidChange event on Android and iOS.

Why

Before v10 MapView's getVisibleBounds() method returned the same value as bbox array like [1.1, 1.1, 1.1, 1.1] on Android and iOS. The same with the event.properties.visibleBounds property of the onRegionDidChange event.

In v10 MapView's getVisibleBounds() method and event.properties.visibleBounds property of the onRegionDidChange event return different values. On Android it's polygon array [[1.1, 1.1], [1.1, 1.1], [1.1, 1.1], [[1.1, 1.1]]. On iOS it's bbox array [1.1, 1.1, 1.1, 1.1].

In case of the onRegionDidChange event on Android implementation here https://github.com/rnmapbox/maps/blob/10fb73810ca84562a8d3b1c4b040e35829bfd45d/android/rctmgl/src/main/java-v10/com/mapbox/rctmgl/components/mapview/RCTMGLMapView.kt#L581 on iOS here https://github.com/rnmapbox/maps/blob/10fb73810ca84562a8d3b1c4b040e35829bfd45d/ios/RCTMGL-v10/RCTMGLMapView.swift#L349

In v10.0.0-beta.24 getVisibleBounds() method and onRegionDidChange event was implemented on Android to return polygon array.

Is there any reason why difference in the method and event return on Android and iOS? Why on Android these method and event return polygon array [[1.1, 1.1], [1.1, 1.1], [1.1, 1.1], [[1.1, 1.1]] instead of bbox array [1.1, 1.1, 1.1, 1.1] as on iOS and as before v10?

Maybe it's better that within one library the same method and event will return the same value regardless of platform (Android or iOS).

Official mapbox-gl-js package for the web return bbox array [1.1, 1.1, 1.1, 1.1], on iOS return bbox array [1.1, 1.1, 1.1, 1.1]. So maybe will be better if on Android these method and event will return the same bbox array [1.1, 1.1, 1.1, 1.1] and not polygon array?

mfazekas commented 2 years ago

@alexanderoskin thanks for the bug report, yes I agree android should return bbox array too

alexanderoskin commented 2 years ago

@mfazekas The same issue again. On beta44 was fixed. Today we have updated to beta54 and onRegionDidChange event return 8 values array instead of 4 values array as was in beta44 and was correct.