backport-2.0: ui: Fix Zoom Bug on Cluster Map #24183
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Fixes a bug where zoom behavior could become broken after zooming all the way out on the cluster map. This happened because the react-land state (ZoomTransformer) got out of sync with the d3 zoom state whenever we applied an adjustment in the ZoomTransformer - we were not informing the d3 zoom behavior of the change, and thus the zoom behavior thought we were at a different zoom level than was actually displayed, causing the zoom to behave unpredictably. This is fixed by synchronizing the zoom state back to d3 in the OnZoom event. However, this exposes another bug where animations are broken; with the synchronization fix applied, freely panning and zooming would cause the next animation to have the wrong start position - the animation would begin from wherever the last programmatic animation stopped. After reading some source code, it turns out that this is a mutation curiosity in d3. Zoom.event maintains an object with the current translation and scale which it shares with the zoom behavior. Oddly, free pan-and-zoom events directly mutate this object, while calling `.scale()` or `.translate()` cause a copy-on-write situation. This means that setting the zoom or translation programmatically without going through zoom.event will cause this animation bug. Fixes cockroachdb#24160 Release note (Admin UI): Fixed a bug where zooming on the node map could break after zooming out to the maximum extent.
|
This change is |
bdarnell
approved these changes
Mar 23, 2018
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Backport 1/1 commits from #24164.
/cc @cockroachdb/release
Fixes a bug where zoom behavior could become broken after zooming all
the way out on the cluster map.
This happened because the react-land state (ZoomTransformer) got out of
sync with the d3 zoom state whenever we applied an adjustment in the
ZoomTransformer - we were not informing the d3 zoom behavior of the
change, and thus the zoom behavior thought we were at a different zoom
level than was actually displayed, causing the zoom to behave
unpredictably. This is fixed by synchronizing the zoom state back to d3
in the OnZoom event.
However, this exposes another bug where animations are broken; with the
synchronization fix applied, freely panning and zooming would cause the
next animation to have the wrong start position - the animation would
begin from wherever the last programmatic animation stopped. After
reading some source code, it turns out that this is a mutation curiosity
in d3. Zoom.event maintains an object with the current translation and
scale which it shares with the zoom behavior. Oddly, free pan-and-zoom
events directly mutate this object, while calling
.scale()or.translate()cause a copy-on-write situation. This means that settingthe zoom or translation programmatically without going through
zoom.event will cause this animation bug.
Fixes #24160
Release note (Admin UI): Fixed a bug where zooming on the node map could
break after zooming out to the maximum extent.