Skip to content

Commit

Permalink
Merge pull request #6046 from plotly/uirevision-autorange-fix
Browse files Browse the repository at this point in the history
fix uirevision autorange bug
  • Loading branch information
alexcjohnson committed Dec 2, 2021
2 parents 5144ab6 + 84a2574 commit 7d3b93e
Show file tree
Hide file tree
Showing 3 changed files with 95 additions and 28 deletions.
1 change: 1 addition & 0 deletions draftlogs/6046_fix.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
- Fix interaction between `uirevision` and `autorange`. Because we push `autorange` and `range` back into `layout`, there can be times it looks like we're applying GUI-driven changes on top of explicit autorange and other times it's an implicit autorange, even though the user's intent was always implicit. This fix treats them as equivalent. [[#6046](https://github.com/plotly/plotly.js/pull/6046)]
45 changes: 37 additions & 8 deletions src/plot_api/plot_api.js
Original file line number Diff line number Diff line change
Expand Up @@ -2396,7 +2396,8 @@ function findUIPattern(key, patternSpecs) {
var spec = patternSpecs[i];
var match = key.match(spec.pattern);
if(match) {
return {head: match[1], attr: spec.attr};
var head = match[1] || '';
return {head: head, tail: key.substr(head.length + 1), attr: spec.attr};
}
}
}
Expand Down Expand Up @@ -2448,26 +2449,54 @@ function valsMatch(v1, v2) {

function applyUIRevisions(data, layout, oldFullData, oldFullLayout) {
var layoutPreGUI = oldFullLayout._preGUI;
var key, revAttr, oldRev, newRev, match, preGUIVal, newNP, newVal;
var key, revAttr, oldRev, newRev, match, preGUIVal, newNP, newVal, head, tail;
var bothInheritAutorange = [];
var newAutorangeIn = {};
var newRangeAccepted = {};
for(key in layoutPreGUI) {
match = findUIPattern(key, layoutUIControlPatterns);
if(match) {
revAttr = match.attr || (match.head + '.uirevision');
head = match.head;
tail = match.tail;
revAttr = match.attr || (head + '.uirevision');
oldRev = nestedProperty(oldFullLayout, revAttr).get();
newRev = oldRev && getNewRev(revAttr, layout);

if(newRev && (newRev === oldRev)) {
preGUIVal = layoutPreGUI[key];
if(preGUIVal === null) preGUIVal = undefined;
newNP = nestedProperty(layout, key);
newVal = newNP.get();

if(valsMatch(newVal, preGUIVal)) {
if(newVal === undefined && key.substr(key.length - 9) === 'autorange') {
bothInheritAutorange.push(key.substr(0, key.length - 10));
if(newVal === undefined && tail === 'autorange') {
bothInheritAutorange.push(head);
}
newNP.set(undefinedToNull(nestedProperty(oldFullLayout, key).get()));
continue;
} else if(tail === 'autorange' || tail.substr(0, 6) === 'range[') {
// Special case for (auto)range since we push it back into the layout
// so all null should be treated equivalently to autorange: true with any range
var pre0 = layoutPreGUI[head + '.range[0]'];
var pre1 = layoutPreGUI[head + '.range[1]'];
var preAuto = layoutPreGUI[head + '.autorange'];
if(preAuto || (preAuto === null && pre0 === null && pre1 === null)) {
// Only read the input layout once and stash the result,
// so we get it before we start modifying it
if(!(head in newAutorangeIn)) {
var newContainer = nestedProperty(layout, head).get();
newAutorangeIn[head] = newContainer && (
newContainer.autorange ||
(newContainer.autorange !== false && (
!newContainer.range || newContainer.range.length !== 2)
)
);
}
if(newAutorangeIn[head]) {
newNP.set(undefinedToNull(nestedProperty(oldFullLayout, key).get()));
continue;
}
}
}
}
} else {
Expand All @@ -2478,12 +2507,12 @@ function applyUIRevisions(data, layout, oldFullData, oldFullLayout) {
// so remove it from _preGUI for next time.
delete layoutPreGUI[key];

if(key.substr(key.length - 8, 6) === 'range[') {
newRangeAccepted[key.substr(0, key.length - 9)] = 1;
if(match && match.tail.substr(0, 6) === 'range[') {
newRangeAccepted[match.head] = 1;
}
}

// Special logic for `autorange`, since it interacts with `range`:
// More special logic for `autorange`, since it interacts with `range`:
// If the new figure's matching `range` was kept, and `autorange`
// wasn't supplied explicitly in either the original or the new figure,
// we shouldn't alter that - but we may just have done that, so fix it.
Expand Down
77 changes: 57 additions & 20 deletions test/jasmine/tests/plot_api_react_test.js
Original file line number Diff line number Diff line change
Expand Up @@ -1366,6 +1366,59 @@ describe('Plotly.react and uirevision attributes', function() {
.then(done, done.fail);
});

function setCartesianRanges(xRange, yRange) {
return function() {
return Registry.call('_guiRelayout', gd, {
'xaxis.range': xRange,
'yaxis.range': yRange
});
};
}

function checkCartesianRanges(xRange, yRange, msg) {
return checkState([], {
'xaxis.range': [xRange],
'yaxis.range': [yRange]
}, msg);
}

it('treats explicit and implicit cartesian autorange the same', function(done) {
function fig(explicit, uirevision) {
return {
data: [{z: [[1, 2], [3, 4]], type: 'heatmap', x: [0, 1, 2], y: [3, 4, 5]}],
layout: {
xaxis: explicit ? {autorange: true, range: [0, 2]} : {},
yaxis: explicit ? {autorange: true, range: [3, 5]} : {},
uirevision: uirevision
}
};
}

// First go from implicit to explicit and back after zooming in
Plotly.newPlot(gd, fig(false, 'a'))
.then(checkCartesianRanges([0, 2], [3, 5], 'initial implicit'))
.then(setCartesianRanges([2, 4], [5, 7]))
.then(checkCartesianRanges([2, 4], [5, 7], 'zoomed from implicit'))
.then(_react(fig(true, 'a')))
.then(checkCartesianRanges([2, 4], [5, 7], 'react to explicit'))
.then(_react(fig(true, 'a')))
.then(checkCartesianRanges([2, 4], [5, 7], 'react to STAY explicit'))
.then(_react(fig(false, 'a')))
.then(checkCartesianRanges([2, 4], [5, 7], 'back to implicit'))
// then go from explicit to implicit and back after zooming in
.then(_react(fig(true, 'b')))
.then(checkCartesianRanges([0, 2], [3, 5], 'new uirevision explicit'))
.then(setCartesianRanges([4, 6], [7, 9]))
.then(checkCartesianRanges([4, 6], [7, 9], 'zoomed from explicit'))
.then(_react(fig(false, 'b')))
.then(checkCartesianRanges([4, 6], [7, 9], 'react to implicit'))
.then(_react(fig(false, 'b')))
.then(checkCartesianRanges([4, 6], [7, 9], 'react to STAY implicit'))
.then(_react(fig(true, 'b')))
.then(checkCartesianRanges([4, 6], [7, 9], 'back to explicit'))
.then(done, done.fail);
});

it('respects reverting an explicit cartesian axis range to auto', function(done) {
function fig(xRange, yRange) {
return {
Expand All @@ -1378,28 +1431,12 @@ describe('Plotly.react and uirevision attributes', function() {
};
}

function setRanges(xRange, yRange) {
return function() {
return Registry.call('_guiRelayout', gd, {
'xaxis.range': xRange,
'yaxis.range': yRange
});
};
}

function checkRanges(xRange, yRange) {
return checkState([], {
'xaxis.range': [xRange],
'yaxis.range': [yRange]
});
}

Plotly.newPlot(gd, fig([1, 3], [4, 6]))
.then(checkRanges([1, 3], [4, 6]))
.then(setRanges([2, 4], [5, 7]))
.then(checkRanges([2, 4], [5, 7]))
.then(checkCartesianRanges([1, 3], [4, 6], 'initial explicit ranges'))
.then(setCartesianRanges([2, 4], [5, 7]))
.then(checkCartesianRanges([2, 4], [5, 7], 'zoomed to different explicit'))
.then(_react(fig(undefined, undefined)))
.then(checkRanges([0, 2], [3, 5]))
.then(checkCartesianRanges([0, 2], [3, 5], 'react to autorange'))
.then(done, done.fail);
});

Expand Down

0 comments on commit 7d3b93e

Please sign in to comment.