From 538710c5f2b90695da4bc7917ebe033e1c4e06ca Mon Sep 17 00:00:00 2001 From: archmoj Date: Thu, 18 Oct 2018 13:02:40 -0400 Subject: [PATCH 1/6] fix to enable callbacks e.g. restyle to redraw --- src/traces/parcoords/parcoords.js | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/src/traces/parcoords/parcoords.js b/src/traces/parcoords/parcoords.js index 3b2ed55a218..0a60befdf5c 100644 --- a/src/traces/parcoords/parcoords.js +++ b/src/traces/parcoords/parcoords.js @@ -456,18 +456,20 @@ module.exports = function(root, svg, parcoordsLineLayers, styledData, layout, ca parcoordsLineLayer .each(function(d) { - if(d.viewModel) { - if(d.lineLayer) d.lineLayer.update(d); - else d.lineLayer = lineLayerMaker(this, d); + if((!d.lineLayer) || + (callbacks)) { // recreate in case of having callbacks e.g. restyle, Should we explicitly test for callback to be a restyle? + d.lineLayer = lineLayerMaker(this, d); + } else d.lineLayer.update(d); - d.viewModel[d.key] = d.lineLayer; + if (d.key) { + d.viewModel[d.key] = d.lineLayer; - var setChanged = ((d.key) && - (((d.key !== 'contextLayer') || (callbacks)) || // unless there is callback on this line layer - (!d.context))); // don't update background + var setChanged = ((!d.context) || // don't update background + ((d.key !== 'contextLayer') || (callbacks))); // unless there is a callback on this line layer - d.lineLayer.render(d.viewModel.panels, setChanged); + d.lineLayer.render(d.viewModel.panels, setChanged); + } } }); From 54f93f44e03e91990187744a82c39a1cdbeccff2 Mon Sep 17 00:00:00 2001 From: archmoj Date: Thu, 18 Oct 2018 13:15:55 -0400 Subject: [PATCH 2/6] updated syntax and comments --- src/traces/parcoords/parcoords.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/traces/parcoords/parcoords.js b/src/traces/parcoords/parcoords.js index 0a60befdf5c..2d3bbed4131 100644 --- a/src/traces/parcoords/parcoords.js +++ b/src/traces/parcoords/parcoords.js @@ -458,15 +458,15 @@ module.exports = function(root, svg, parcoordsLineLayers, styledData, layout, ca .each(function(d) { if(d.viewModel) { if((!d.lineLayer) || - (callbacks)) { // recreate in case of having callbacks e.g. restyle, Should we explicitly test for callback to be a restyle? + (callbacks)) { // recreate in case of having callbacks e.g. restyle. Should we test for callback to be a restyle? d.lineLayer = lineLayerMaker(this, d); } else d.lineLayer.update(d); - if (d.key) { + if(d.key) { d.viewModel[d.key] = d.lineLayer; var setChanged = ((!d.context) || // don't update background - ((d.key !== 'contextLayer') || (callbacks))); // unless there is a callback on this line layer + ((d.key !== 'contextLayer') || (callbacks))); // unless there is a callback on the context layer. Should we test the callback? d.lineLayer.render(d.viewModel.panels, setChanged); } From 0dd337e0340343e533598dad4e6c1b1bb19491de Mon Sep 17 00:00:00 2001 From: archmoj Date: Thu, 18 Oct 2018 14:16:13 -0400 Subject: [PATCH 3/6] increased delay in config_test --- test/jasmine/tests/config_test.js | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/test/jasmine/tests/config_test.js b/test/jasmine/tests/config_test.js index 395b8f36d4e..dd6ce559ca0 100644 --- a/test/jasmine/tests/config_test.js +++ b/test/jasmine/tests/config_test.js @@ -7,6 +7,7 @@ var click = require('../assets/click'); var mouseEvent = require('../assets/mouse_event'); var failTest = require('../assets/fail_test'); var delay = require('../assets/delay'); +var RESIZE_DELAY = 300; describe('config argument', function() { @@ -585,7 +586,7 @@ describe('config argument', function() { viewport.set(width / 2, height / 2); return Promise.resolve() - .then(delay(200)) + .then(delay(RESIZE_DELAY)) .then(function() { checkLayoutSize(elWidth / 2, elHeight / 2); }) @@ -639,7 +640,7 @@ describe('config argument', function() { Plotly.plot(gd, data, {}, {responsive: true}) .then(function() {return Plotly.restyle(gd, 'y[0]', data[0].y[0] + 2);}) .then(function() {viewport.set(width / 2, width / 2);}) - .then(delay(200)) + .then(delay(RESIZE_DELAY)) // .then(function() {viewport.set(newWidth, 2 * newHeight);}).then(delay(200)) .then(function() { expect(cntWindowResize).toBe(1); @@ -667,7 +668,7 @@ describe('config argument', function() { // Resize viewport .then(function() {viewport.set(width / 2, height / 2);}) // Wait for resize to happen (Plotly.resize has an internal timeout) - .then(delay(200)) + .then(delay(RESIZE_DELAY)) // Check that final figure's size hasn't changed .then(function() {checkLayoutSize(width, height);}) .catch(failTest) From 6b76026df88a64cb79cfebfa55cd225d4d44d83f Mon Sep 17 00:00:00 2001 From: archmoj Date: Thu, 18 Oct 2018 14:40:31 -0400 Subject: [PATCH 4/6] put render call outside key check --- src/traces/parcoords/parcoords.js | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/src/traces/parcoords/parcoords.js b/src/traces/parcoords/parcoords.js index 2d3bbed4131..3d2e0229efc 100644 --- a/src/traces/parcoords/parcoords.js +++ b/src/traces/parcoords/parcoords.js @@ -462,14 +462,12 @@ module.exports = function(root, svg, parcoordsLineLayers, styledData, layout, ca d.lineLayer = lineLayerMaker(this, d); } else d.lineLayer.update(d); - if(d.key) { - d.viewModel[d.key] = d.lineLayer; + if(d.key) d.viewModel[d.key] = d.lineLayer; - var setChanged = ((!d.context) || // don't update background - ((d.key !== 'contextLayer') || (callbacks))); // unless there is a callback on the context layer. Should we test the callback? + var setChanged = ((!d.context) || // don't update background + (callbacks)); // unless there is a callback on the context layer. Should we test the callback? - d.lineLayer.render(d.viewModel.panels, setChanged); - } + d.lineLayer.render(d.viewModel.panels, setChanged); } }); From dbf4e29749be6a9ab59b3b4faaf16672014365ca Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20T=C3=A9treault-Pinard?= Date: Mon, 29 Oct 2018 16:06:23 -0400 Subject: [PATCH 5/6] add parcoords line.color restyle test --- test/jasmine/tests/parcoords_test.js | 45 ++++++++++++++++++++++++++++ 1 file changed, 45 insertions(+) diff --git a/test/jasmine/tests/parcoords_test.js b/test/jasmine/tests/parcoords_test.js index 0e1a74406bc..f54fd053a37 100644 --- a/test/jasmine/tests/parcoords_test.js +++ b/test/jasmine/tests/parcoords_test.js @@ -790,6 +790,51 @@ describe('parcoords Lifecycle methods', function() { .then(done); }); }); + + it('@gl line.color `Plotly.restyle` should work', function(done) { + function getAvgPixelByChannel() { + var canvas = d3.select('.gl-canvas-focus').node(); + var imgData = readPixel(canvas, 0, 0, canvas.width, canvas.height); + var n = imgData.length / 4; + var r = 0; + var g = 0; + var b = 0; + + for(var i = 0; i < imgData.length; i++) { + r += imgData[i++]; + g += imgData[i++]; + b += imgData[i++]; + } + return [r / n, g / n, b / n]; + } + + Plotly.plot(gd, [{ + type: 'parcoords', + dimensions: [{ + values: [1, 2] + }, { + values: [2, 4] + }], + line: {color: 'blue'} + }], { + width: 300, + height: 200 + }) + .then(function() { + var rbg = getAvgPixelByChannel(); + expect(rbg[0]).toBe(0, 'no red'); + expect(rbg[2]).not.toBe(0, 'all blue'); + + return Plotly.restyle(gd, 'line.color', 'red'); + }) + .then(function() { + var rbg = getAvgPixelByChannel(); + expect(rbg[0]).not.toBe(0, 'all red'); + expect(rbg[2]).toBe(0, 'no blue'); + }) + .catch(failTest) + .then(done); + }); }); describe('parcoords basic use', function() { From a8fd24f7b852d649fddfba658c55a1a5242b394d Mon Sep 17 00:00:00 2001 From: archmoj Date: Mon, 29 Oct 2018 10:19:00 -0400 Subject: [PATCH 6/6] fixups --- src/traces/parcoords/parcoords.js | 9 ++++----- test/jasmine/tests/parcoords_test.js | 2 +- 2 files changed, 5 insertions(+), 6 deletions(-) diff --git a/src/traces/parcoords/parcoords.js b/src/traces/parcoords/parcoords.js index 3d2e0229efc..14874abaccf 100644 --- a/src/traces/parcoords/parcoords.js +++ b/src/traces/parcoords/parcoords.js @@ -457,15 +457,14 @@ module.exports = function(root, svg, parcoordsLineLayers, styledData, layout, ca parcoordsLineLayer .each(function(d) { if(d.viewModel) { - if((!d.lineLayer) || - (callbacks)) { // recreate in case of having callbacks e.g. restyle. Should we test for callback to be a restyle? + if(!d.lineLayer || callbacks) { // recreate in case of having callbacks e.g. restyle. Should we test for callback to be a restyle? d.lineLayer = lineLayerMaker(this, d); } else d.lineLayer.update(d); - if(d.key) d.viewModel[d.key] = d.lineLayer; + if(d.key || d.key === 0) d.viewModel[d.key] = d.lineLayer; - var setChanged = ((!d.context) || // don't update background - (callbacks)); // unless there is a callback on the context layer. Should we test the callback? + var setChanged = (!d.context || // don't update background + callbacks); // unless there is a callback on the context layer. Should we test the callback? d.lineLayer.render(d.viewModel.panels, setChanged); } diff --git a/test/jasmine/tests/parcoords_test.js b/test/jasmine/tests/parcoords_test.js index f54fd053a37..467536be946 100644 --- a/test/jasmine/tests/parcoords_test.js +++ b/test/jasmine/tests/parcoords_test.js @@ -913,7 +913,7 @@ describe('parcoords basic use', function() { }); - it('@gl Calling `Plotly.restyle` with a string path should amend the preexisting parcoords', function(done) { + it('@gl Calling `Plotly.restyle` with a string path to colorscale should amend the preexisting parcoords', function(done) { expect(gd.data.length).toEqual(1);