From 63798bf8df33458f36c26dd244948cb1e3380a96 Mon Sep 17 00:00:00 2001 From: Mojtaba Samimi Date: Mon, 14 Jul 2025 22:44:01 -0400 Subject: [PATCH 1/7] react to config changes --- src/plot_api/plot_api.js | 212 ++++++++++++++++++++------------------- 1 file changed, 108 insertions(+), 104 deletions(-) diff --git a/src/plot_api/plot_api.js b/src/plot_api/plot_api.js index 730428f675f..e981064934c 100644 --- a/src/plot_api/plot_api.js +++ b/src/plot_api/plot_api.js @@ -2615,122 +2615,126 @@ function react(gd, data, layout, config) { configChanged = diffConfig(oldConfig, gd._context); } - gd.data = data || []; - helpers.cleanData(gd.data); - gd.layout = layout || {}; - helpers.cleanLayout(gd.layout); - - applyUIRevisions(gd.data, gd.layout, oldFullData, oldFullLayout); - - // "true" skips updating calcdata and remapping arrays from calcTransforms, - // which supplyDefaults usually does at the end, but we may need to NOT do - // if the diff (which we haven't determined yet) says we'll recalc - Plots.supplyDefaults(gd, {skipUpdateCalc: true}); - - var newFullData = gd._fullData; - var newFullLayout = gd._fullLayout; - var immutable = newFullLayout.datarevision === undefined; - var transition = newFullLayout.transition; - - var relayoutFlags = diffLayout(gd, oldFullLayout, newFullLayout, immutable, transition); - var newDataRevision = relayoutFlags.newDataRevision; - var restyleFlags = diffData(gd, oldFullData, newFullData, immutable, transition, newDataRevision); - - // TODO: how to translate this part of relayout to Plotly.react? - // // Setting width or height to null must reset the graph's width / height - // // back to its initial value as computed during the first pass in Plots.plotAutoSize. - // // - // // To do so, we must manually set them back here using the _initialAutoSize cache. - // if(['width', 'height'].indexOf(ai) !== -1 && vi === null) { - // fullLayout[ai] = gd._initialAutoSize[ai]; - // } - - if(updateAutosize(gd)) relayoutFlags.layoutReplot = true; - - // clear calcdata and empty categories if required - if(restyleFlags.calc || relayoutFlags.calc) { - gd.calcdata = undefined; - var allNames = Object.getOwnPropertyNames(newFullLayout); - for(var q = 0; q < allNames.length; q++) { - var name = allNames[q]; - var start = name.substring(0, 5); - if(start === 'xaxis' || start === 'yaxis') { - var emptyCategories = newFullLayout[name]._emptyCategories; - if(emptyCategories) emptyCategories(); + if(configChanged) { + plotDone = exports.newPlot(gd, data, layout, config); + } else { + gd.data = data || []; + helpers.cleanData(gd.data); + gd.layout = layout || {}; + helpers.cleanLayout(gd.layout); + + applyUIRevisions(gd.data, gd.layout, oldFullData, oldFullLayout); + + // "true" skips updating calcdata and remapping arrays from calcTransforms, + // which supplyDefaults usually does at the end, but we may need to NOT do + // if the diff (which we haven't determined yet) says we'll recalc + Plots.supplyDefaults(gd, {skipUpdateCalc: true}); + + var newFullData = gd._fullData; + var newFullLayout = gd._fullLayout; + var immutable = newFullLayout.datarevision === undefined; + var transition = newFullLayout.transition; + + var relayoutFlags = diffLayout(gd, oldFullLayout, newFullLayout, immutable, transition); + var newDataRevision = relayoutFlags.newDataRevision; + var restyleFlags = diffData(gd, oldFullData, newFullData, immutable, transition, newDataRevision); + + // TODO: how to translate this part of relayout to Plotly.react? + // // Setting width or height to null must reset the graph's width / height + // // back to its initial value as computed during the first pass in Plots.plotAutoSize. + // // + // // To do so, we must manually set them back here using the _initialAutoSize cache. + // if(['width', 'height'].indexOf(ai) !== -1 && vi === null) { + // fullLayout[ai] = gd._initialAutoSize[ai]; + // } + + if(updateAutosize(gd)) relayoutFlags.layoutReplot = true; + + // clear calcdata and empty categories if required + if(restyleFlags.calc || relayoutFlags.calc) { + gd.calcdata = undefined; + var allNames = Object.getOwnPropertyNames(newFullLayout); + for(var q = 0; q < allNames.length; q++) { + var name = allNames[q]; + var start = name.substring(0, 5); + if(start === 'xaxis' || start === 'yaxis') { + var emptyCategories = newFullLayout[name]._emptyCategories; + if(emptyCategories) emptyCategories(); + } } + // otherwise do the calcdata updates and calcTransform array remaps that we skipped earlier + } else { + Plots.supplyDefaultsUpdateCalc(gd.calcdata, newFullData); } - // otherwise do the calcdata updates and calcTransform array remaps that we skipped earlier - } else { - Plots.supplyDefaultsUpdateCalc(gd.calcdata, newFullData); - } - // Note: what restyle/relayout use impliedEdits and clearAxisTypes for - // must be handled by the user when using Plotly.react. + // Note: what restyle/relayout use impliedEdits and clearAxisTypes for + // must be handled by the user when using Plotly.react. - // fill in redraw sequence - var seq = []; + // fill in redraw sequence + var seq = []; - if(frames) { - gd._transitionData = {}; - Plots.createTransitionData(gd); - seq.push(addFrames); - } + if(frames) { + gd._transitionData = {}; + Plots.createTransitionData(gd); + seq.push(addFrames); + } - // Transition pathway, - // only used when 'transition' is set by user and - // when at least one animatable attribute has changed, - // N.B. config changed aren't animatable - if(newFullLayout.transition && !configChanged && (restyleFlags.anim || relayoutFlags.anim)) { - if(relayoutFlags.ticks) seq.push(subroutines.doTicksRelayout); + // Transition pathway, + // only used when 'transition' is set by user and + // when at least one animatable attribute has changed, + // N.B. config changed aren't animatable + if(newFullLayout.transition && !configChanged && (restyleFlags.anim || relayoutFlags.anim)) { + if(relayoutFlags.ticks) seq.push(subroutines.doTicksRelayout); - Plots.doCalcdata(gd); - subroutines.doAutoRangeAndConstraints(gd); + Plots.doCalcdata(gd); + subroutines.doAutoRangeAndConstraints(gd); - seq.push(function() { - return Plots.transitionFromReact(gd, restyleFlags, relayoutFlags, oldFullLayout); - }); - } else if(restyleFlags.fullReplot || relayoutFlags.layoutReplot || configChanged) { - gd._fullLayout._skipDefaults = true; - seq.push(exports._doPlot); - } else { - for(var componentType in relayoutFlags.arrays) { - var indices = relayoutFlags.arrays[componentType]; - if(indices.length) { - var drawOne = Registry.getComponentMethod(componentType, 'drawOne'); - if(drawOne !== Lib.noop) { - for(var i = 0; i < indices.length; i++) { - drawOne(gd, indices[i]); - } - } else { - var draw = Registry.getComponentMethod(componentType, 'draw'); - if(draw === Lib.noop) { - throw new Error('cannot draw components: ' + componentType); + seq.push(function() { + return Plots.transitionFromReact(gd, restyleFlags, relayoutFlags, oldFullLayout); + }); + } else if(restyleFlags.fullReplot || relayoutFlags.layoutReplot || configChanged) { + gd._fullLayout._skipDefaults = true; + seq.push(exports._doPlot); + } else { + for(var componentType in relayoutFlags.arrays) { + var indices = relayoutFlags.arrays[componentType]; + if(indices.length) { + var drawOne = Registry.getComponentMethod(componentType, 'drawOne'); + if(drawOne !== Lib.noop) { + for(var i = 0; i < indices.length; i++) { + drawOne(gd, indices[i]); + } + } else { + var draw = Registry.getComponentMethod(componentType, 'draw'); + if(draw === Lib.noop) { + throw new Error('cannot draw components: ' + componentType); + } + draw(gd); } - draw(gd); } } - } - seq.push(Plots.previousPromises); - if(restyleFlags.style) seq.push(subroutines.doTraceStyle); - if(restyleFlags.colorbars || relayoutFlags.colorbars) seq.push(subroutines.doColorBars); - if(relayoutFlags.legend) seq.push(subroutines.doLegend); - if(relayoutFlags.layoutstyle) seq.push(subroutines.layoutStyles); - if(relayoutFlags.axrange) addAxRangeSequence(seq); - if(relayoutFlags.ticks) seq.push(subroutines.doTicksRelayout); - if(relayoutFlags.modebar) seq.push(subroutines.doModeBar); - if(relayoutFlags.camera) seq.push(subroutines.doCamera); - seq.push(emitAfterPlot); - } + seq.push(Plots.previousPromises); + if(restyleFlags.style) seq.push(subroutines.doTraceStyle); + if(restyleFlags.colorbars || relayoutFlags.colorbars) seq.push(subroutines.doColorBars); + if(relayoutFlags.legend) seq.push(subroutines.doLegend); + if(relayoutFlags.layoutstyle) seq.push(subroutines.layoutStyles); + if(relayoutFlags.axrange) addAxRangeSequence(seq); + if(relayoutFlags.ticks) seq.push(subroutines.doTicksRelayout); + if(relayoutFlags.modebar) seq.push(subroutines.doModeBar); + if(relayoutFlags.camera) seq.push(subroutines.doCamera); + seq.push(emitAfterPlot); + } - seq.push( - Plots.rehover, - Plots.redrag, - Plots.reselect - ); + seq.push( + Plots.rehover, + Plots.redrag, + Plots.reselect + ); - plotDone = Lib.syncOrAsync(seq, gd); - if(!plotDone || !plotDone.then) plotDone = Promise.resolve(gd); + plotDone = Lib.syncOrAsync(seq, gd); + if(!plotDone || !plotDone.then) plotDone = Promise.resolve(gd); + } } return plotDone.then(function() { @@ -3675,11 +3679,11 @@ function makePlotFramework(gd) { // The plot container should always take the full with the height of its // parent (the graph div). This ensures that for responsive plots // without a height or width set, the paper div will take up the full - // height & width of the graph div. + // height & width of the graph div. // So, for responsive plots without a height or width set, if the plot // container's height is left to 'auto', its height will be dictated by // its childrens' height. (The plot container's only child is the paper - // div.) + // div.) // In this scenario, the paper div's height will be set to 100%, // which will be 100% of the plot container's auto height. That is // meaninglesss, so the browser will use the paper div's children to set From d9e7b2daad8b850dcd2945fe7895a3e2fcc8d37c Mon Sep 17 00:00:00 2001 From: Mojtaba Samimi Date: Wed, 16 Jul 2025 14:41:41 -0400 Subject: [PATCH 2/7] adjust test --- test/jasmine/tests/plot_api_react_test.js | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/test/jasmine/tests/plot_api_react_test.js b/test/jasmine/tests/plot_api_react_test.js index 28128cc1075..ed359451a9f 100644 --- a/test/jasmine/tests/plot_api_react_test.js +++ b/test/jasmine/tests/plot_api_react_test.js @@ -418,6 +418,8 @@ describe('@noCIdep Plotly.react', function() { expect(d3SelectAll('.drag').size()).toBe(11); expect(d3SelectAll('.gtitle').text()).toBe('Click to enter Plot title'); expect(d3SelectAll('.gtitle-subtitle').text()).toBe('Click to enter Plot subtitle'); + + afterPlotCnt++; // since it uses newPlot pathway as a result of config change countCalls({plot: 1}); return Plotly.react(gd, data, layout, {staticPlot: true}); @@ -425,6 +427,9 @@ describe('@noCIdep Plotly.react', function() { .then(function() { expect(d3SelectAll('.drag').size()).toBe(0); expect(d3SelectAll('.gtitle').size()).toBe(0); + expect(d3SelectAll('.gtitle-subtitle').size()).toBe(0); + + afterPlotCnt++; // since it uses newPlot pathway as a result of config change countCalls({plot: 1}); return Plotly.react(gd, data, layout, {}); @@ -432,6 +437,9 @@ describe('@noCIdep Plotly.react', function() { .then(function() { expect(d3SelectAll('.drag').size()).toBe(11); expect(d3SelectAll('.gtitle').size()).toBe(0); + expect(d3SelectAll('.gtitle-subtitle').size()).toBe(0); + + afterPlotCnt++; // since it uses newPlot pathway as a result of config change countCalls({plot: 1}); }) .then(done, done.fail); From 1ff3b259df5fbf73a7047ec1a68ae119d6151ce4 Mon Sep 17 00:00:00 2001 From: Mojtaba Samimi Date: Thu, 17 Jul 2025 17:07:12 -0400 Subject: [PATCH 3/7] draftlog --- draftlogs/7475.md | 1 + 1 file changed, 1 insertion(+) create mode 100644 draftlogs/7475.md diff --git a/draftlogs/7475.md b/draftlogs/7475.md new file mode 100644 index 00000000000..e0a1f8ece7b --- /dev/null +++ b/draftlogs/7475.md @@ -0,0 +1 @@ + - Fix react case of a config change [7475](https://github.com/plotly/plotly.js/pull/7475) From 29ccfaf99f6eb2fb788180363634e1d59c106eea Mon Sep 17 00:00:00 2001 From: Cameron DeCoster Date: Wed, 3 Sep 2025 17:04:03 -0600 Subject: [PATCH 4/7] Save event listeners when calling newPlot Call react again to ensure transitions are triggered Remove obsolete references to configChanged --- src/plot_api/plot_api.js | 23 +++++++++++++++-------- 1 file changed, 15 insertions(+), 8 deletions(-) diff --git a/src/plot_api/plot_api.js b/src/plot_api/plot_api.js index e981064934c..5702c5a5a2b 100644 --- a/src/plot_api/plot_api.js +++ b/src/plot_api/plot_api.js @@ -2616,7 +2616,17 @@ function react(gd, data, layout, config) { } if(configChanged) { - plotDone = exports.newPlot(gd, data, layout, config); + // Save event listeners as newPlot will remove them + const eventListeners = gd._ev.eventNames().map(name => [name, gd._ev.listeners(name)]); + plotDone = exports.newPlot(gd, data, layout, config) + .then(() => { + for (const [name, callbacks] of eventListeners) { + callbacks.forEach((cb) => gd.on(name, cb)); + } + + // Call react in case transition should have occurred along with config change + return exports.react(gd, data, layout, config) + }); } else { gd.data = data || []; helpers.cleanData(gd.data); @@ -2683,7 +2693,7 @@ function react(gd, data, layout, config) { // only used when 'transition' is set by user and // when at least one animatable attribute has changed, // N.B. config changed aren't animatable - if(newFullLayout.transition && !configChanged && (restyleFlags.anim || relayoutFlags.anim)) { + if(newFullLayout.transition && (restyleFlags.anim || relayoutFlags.anim)) { if(relayoutFlags.ticks) seq.push(subroutines.doTicksRelayout); Plots.doCalcdata(gd); @@ -2692,7 +2702,7 @@ function react(gd, data, layout, config) { seq.push(function() { return Plots.transitionFromReact(gd, restyleFlags, relayoutFlags, oldFullLayout); }); - } else if(restyleFlags.fullReplot || relayoutFlags.layoutReplot || configChanged) { + } else if(restyleFlags.fullReplot || relayoutFlags.layoutReplot) { gd._fullLayout._skipDefaults = true; seq.push(exports._doPlot); } else { @@ -2737,11 +2747,8 @@ function react(gd, data, layout, config) { } } - return plotDone.then(function() { - gd.emit('plotly_react', { - data: data, - layout: layout - }); + return plotDone.then(() => { + if (!configChanged) gd.emit('plotly_react', { config, data, layout }); return gd; }); From 5965bdf3606d1934ae719b1b6002bcf7aa0de7d7 Mon Sep 17 00:00:00 2001 From: Cameron DeCoster Date: Thu, 4 Sep 2025 15:38:47 -0600 Subject: [PATCH 5/7] Don't update after plot count in tests --- test/jasmine/tests/plot_api_react_test.js | 6 ------ 1 file changed, 6 deletions(-) diff --git a/test/jasmine/tests/plot_api_react_test.js b/test/jasmine/tests/plot_api_react_test.js index ed359451a9f..40c8e0a3f53 100644 --- a/test/jasmine/tests/plot_api_react_test.js +++ b/test/jasmine/tests/plot_api_react_test.js @@ -418,8 +418,6 @@ describe('@noCIdep Plotly.react', function() { expect(d3SelectAll('.drag').size()).toBe(11); expect(d3SelectAll('.gtitle').text()).toBe('Click to enter Plot title'); expect(d3SelectAll('.gtitle-subtitle').text()).toBe('Click to enter Plot subtitle'); - - afterPlotCnt++; // since it uses newPlot pathway as a result of config change countCalls({plot: 1}); return Plotly.react(gd, data, layout, {staticPlot: true}); @@ -428,8 +426,6 @@ describe('@noCIdep Plotly.react', function() { expect(d3SelectAll('.drag').size()).toBe(0); expect(d3SelectAll('.gtitle').size()).toBe(0); expect(d3SelectAll('.gtitle-subtitle').size()).toBe(0); - - afterPlotCnt++; // since it uses newPlot pathway as a result of config change countCalls({plot: 1}); return Plotly.react(gd, data, layout, {}); @@ -438,8 +434,6 @@ describe('@noCIdep Plotly.react', function() { expect(d3SelectAll('.drag').size()).toBe(11); expect(d3SelectAll('.gtitle').size()).toBe(0); expect(d3SelectAll('.gtitle-subtitle').size()).toBe(0); - - afterPlotCnt++; // since it uses newPlot pathway as a result of config change countCalls({plot: 1}); }) .then(done, done.fail); From d7be55a1bdc2666f173c8e48676ea09979e07130 Mon Sep 17 00:00:00 2001 From: Cameron DeCoster Date: Thu, 4 Sep 2025 16:04:37 -0600 Subject: [PATCH 6/7] Update draftlog file name --- draftlogs/{7475.md => 7475_fix.md} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename draftlogs/{7475.md => 7475_fix.md} (100%) diff --git a/draftlogs/7475.md b/draftlogs/7475_fix.md similarity index 100% rename from draftlogs/7475.md rename to draftlogs/7475_fix.md From 61332683009ee1c6b00259403d2269ab44292632 Mon Sep 17 00:00:00 2001 From: Cameron DeCoster Date: Thu, 4 Sep 2025 16:05:15 -0600 Subject: [PATCH 7/7] Update draftlog description --- draftlogs/7475_fix.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/draftlogs/7475_fix.md b/draftlogs/7475_fix.md index e0a1f8ece7b..dc46720b572 100644 --- a/draftlogs/7475_fix.md +++ b/draftlogs/7475_fix.md @@ -1 +1 @@ - - Fix react case of a config change [7475](https://github.com/plotly/plotly.js/pull/7475) + - Update plot with all config changes during call to `Plotly.react` [#7475](https://github.com/plotly/plotly.js/pull/7475)