Don't reuse Map#{moving,zooming} state #6183
Conversation
|
There's at least one place where we're reading the Lines 1529 to 1530 in 38c7250 As a way to sweep for those and also since |
|
@anandthakker Ready for second-pass review here. |
| test('Map#isMoving returns true when drag panning and scroll zooming interleave', (t) => { | ||
| const map = createMap(); | ||
|
|
||
| map.on('dragstart', () => { |
There was a problem hiding this comment.
Add a quick comment here that we're expecting the events to come in the order they're listed here?
| */ | ||
| isZooming(): boolean { | ||
| return this._zooming || | ||
| this.scrollZoom.isActive(); |
There was a problem hiding this comment.
should this.doubleClickZoom.isActive() also be called here?
There was a problem hiding this comment.
DoubleClickZoomHandler implements the zoom using zoomTo, which will set _zooming, so it isn't strictly necessary. (In fact I think DoubleClickZoomHandler should probably not have an isActive() method at all. Unlike drag panning/rotating, where there's an interval bookended by down/up events, double-clicking is an instantaneous action.)
There was a problem hiding this comment.
DoubleClickZoomHandler #isActive() turns out to be used internally, so I'll abandon that idea for now.
Instead, implement isMoving and isZooming using the independent state that the relevant handlers already track.
Instead, implement isMoving and isZooming using the independent state that the relevant handlers already track.
Fixes #6173.