Fix zoom animation for v3.32 #559
Conversation
Instead, add a new method fromLatLngToContainerPixel() to geo service.
src/google_map.js
Outdated
| } else { | ||
| this_.updateCounter_++; | ||
| this_._onBoundsChanged(map, maps); | ||
| if (mapsVersion < 32) { |
There was a problem hiding this comment.
Can we move 32 to a const variable?
| const ptx = overlayProjection.fromLatLngToDivPixel( | ||
| new maps.LatLng(ne.lat(), sw.lng()) | ||
| overlayProjection.fromContainerPixelToLatLng({ x: 0, y: 0 }) |
There was a problem hiding this comment.
Could you elaborate why setting { x:0, y:0 } fixes the animation problem?
There was a problem hiding this comment.
Good question, this is subtle. There are two ways of getting the lat-lng coordinates of the map's top-left corner.
-
Using
map.getBounds()gives you the final bounds. That's because it is based onmap.getCenter()andmap.getZoom()which give you the final center and the final zoom. In other words, if you callmap.setZoom()or click on the zoom control,map.getZoom()immediately reflects the new zoom, and so doesmap.getBounds(). You would use this if you were, say, fetching data from a server based on the map bounds. -
Using
overlayProjection.fromContainerPixelToLatLng({ x: 0, y: 0})gives you the bounds for the current frame. We use this because we are drawing each frame.
There was a problem hiding this comment.
@stephenfarrar Great, thanks for the answer. That makes sense, if map.getBounds gives you the final bounds, that is exactly what is making the markers animate from the corners.
src/google_map.js
Outdated
| @@ -566,6 +566,9 @@ export default class GoogleMap extends Component { | |||
|
|
|||
| this._setLayers(this.props.layerTypes); | |||
|
|
|||
| const versionMatch = maps.version.match(/^3\.(\d+)\./); | |||
| const mapsVersion = versionMatch ? Number(versionMatch[1]) : 99; | |||
There was a problem hiding this comment.
Could we make this code more readable? What is 99? or versionMatch[1]? (I understand ofc but its not so readable)
src/google_map.js
Outdated
| @@ -34,6 +34,9 @@ const K_GOOGLE_TILE_SIZE = 256; | |||
| const K_IDLE_TIMEOUT = 100; | |||
| const K_IDLE_CLICK_TIMEOUT = 300; | |||
| const DEFAULT_MIN_ZOOM = 3; | |||
| // Starting with version 3.32, the maps API calls `draw()` each frame during | |||
| // a zoom animation. | |||
| const DRAW_CALLED_DURING_ANIMATION = 32; | |||
There was a problem hiding this comment.
@stephenfarrar Good work, I will add _VERSION to this const real quick if you don't mind.
|
@stephenfarrar Although I keep thinking if we want to keep simulating the I understand that removing this would affect our support for older versions, but at the same time I don't believe we should have different code for each version of Google Map (I know its not the case and its just for versions below 3.32, but my question is more related to which approach to take when). What do you think? 🤔 |
|
I wrote it this way because I think it's good for customers to be able to choose independently the version of the Maps API and google-map-react. But I agree that this version-detection is not very good. We can delete this section in late August 2018, because v3.31 is scheduled to be deleted by then. Does that sound acceptable? |
|
@stephenfarrar I think it does! Lets keep it as it is right now, I will delete it afterwards if needed. |
|
This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. |
I managed to reduce the surface of the change.
The basic demo seems to work fine, but I haven't tried it in any other demos.
Summary of change:
fromLatLngToContainerPixel()method, which accounts for fractional zoom during a zoom animation.Fixes #552