From 6057fdf181fc2d836576fc7978c2d2a5dda16c10 Mon Sep 17 00:00:00 2001 From: alexcjohnson Date: Mon, 29 Nov 2021 10:42:39 -0500 Subject: [PATCH 1/2] fix uirevision autorange bug --- src/plot_api/plot_api.js | 45 ++++++++++--- test/jasmine/tests/plot_api_react_test.js | 77 +++++++++++++++++------ 2 files changed, 94 insertions(+), 28 deletions(-) diff --git a/src/plot_api/plot_api.js b/src/plot_api/plot_api.js index 1a9386a3915..a0092714d91 100644 --- a/src/plot_api/plot_api.js +++ b/src/plot_api/plot_api.js @@ -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}; } } } @@ -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 { @@ -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. diff --git a/test/jasmine/tests/plot_api_react_test.js b/test/jasmine/tests/plot_api_react_test.js index fc0b3a4846f..c2b41f79147 100644 --- a/test/jasmine/tests/plot_api_react_test.js +++ b/test/jasmine/tests/plot_api_react_test.js @@ -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 { @@ -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); }); From 84a257433b4f38964c0069e400784343e64f9d6b Mon Sep 17 00:00:00 2001 From: alexcjohnson Date: Mon, 29 Nov 2021 11:38:06 -0500 Subject: [PATCH 2/2] log for uirevision fix 6046 --- draftlogs/6046_fix.md | 1 + 1 file changed, 1 insertion(+) create mode 100644 draftlogs/6046_fix.md diff --git a/draftlogs/6046_fix.md b/draftlogs/6046_fix.md new file mode 100644 index 00000000000..5ab255f8433 --- /dev/null +++ b/draftlogs/6046_fix.md @@ -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)]