Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Reset updates object on every zoomDone #3028

Merged
merged 1 commit into from
Sep 19, 2018

Conversation

etpinard
Copy link
Contributor

@etpinard etpinard commented Sep 19, 2018

... so that 'old' keys from two-sided zoomboxes don't trickle into single-side zoombox updates.

Fixes #3026, which was made apparent staring from v1.40.0 (and PR #2823), as we no longer run a calc edit when autoranging cartesian axes (e.g on double-click).

This one is fairly bad bug, so I'll make a release as soon as this thing is merged.

cc @alexcjohnson @antoinerg

- so that 'old' keys from two-sided zoomboxes don't trickle into
  single-side zoombox updates
@etpinard etpinard added bug something broken status: reviewable labels Sep 19, 2018
@@ -386,6 +385,8 @@ function makeDragBox(gd, plotinfo, x, y, w, h, ns, ew) {
}

function zoomDone() {
updates = {};
Copy link
Contributor Author

@etpinard etpinard Sep 19, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note that the same updates object is also "reset" in

// Draw ticks and annotations (and other components) when ranges change.
// Also records the ranges that have changed for use by update at the end.
function ticksAndAnnotations(ns, ew) {
var activeAxIds = [],
i;
function pushActiveAxIds(axList) {
for(i = 0; i < axList.length; i++) {
if(!axList[i].fixedrange) activeAxIds.push(axList[i]._id);
}
}
if(editX) {
pushActiveAxIds(xaxes);
pushActiveAxIds(links.xaxes);
}
if(editY) {
pushActiveAxIds(yaxes);
pushActiveAxIds(links.yaxes);
}
updates = {};
for(i = 0; i < activeAxIds.length; i++) {
var axId = activeAxIds[i];
doTicksSingle(gd, axId, true);
var ax = getFromId(gd, axId);
updates[ax._name + '.range[0]'] = ax.range[0];
updates[ax._name + '.range[1]'] = ax.range[1];
}
function redrawObjs(objArray, method, shortCircuit) {
for(i = 0; i < objArray.length; i++) {
var obji = objArray[i];
if((ew && activeAxIds.indexOf(obji.xref) !== -1) ||
(ns && activeAxIds.indexOf(obji.yref) !== -1)) {
method(gd, i);
// once is enough for images (which doesn't use the `i` arg anyway)
if(shortCircuit) return;
}
}
}
// annotations and shapes 'draw' method is slow,
// use the finer-grained 'drawOne' method instead
redrawObjs(gd._fullLayout.annotations || [], Registry.getComponentMethod('annotations', 'drawOne'));
redrawObjs(gd._fullLayout.shapes || [], Registry.getComponentMethod('shapes', 'drawOne'));
redrawObjs(gd._fullLayout.images || [], Registry.getComponentMethod('images', 'draw'), true);
}

which is called on pan and scroll.

@alexcjohnson
Copy link
Collaborator

Thanks for the quick work! 💃
Could probably simplify and prevent this kind of issue by only constructing updates when required and passing it in to dragTail... but that's a bigger project, lets get this out!

@etpinard etpinard merged commit 9dbac47 into master Sep 19, 2018
@etpinard etpinard deleted the xy-drag-dblclick-x-or-y-only-drag-fix branch September 19, 2018 17:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug something broken
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants