Skip to content

Display parcoords deselected lines#3123

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.

Sign up for GitHub

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

Merged
alexcjohnson merged 3 commits into from
Oct 18, 2018
Merged

Display parcoords deselected lines #3123

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
e034ffe
fix for issue 3099
archmoj Oct 18, 2018
b369c0c
explicitly handle contextLayer callback - may run faster in some scen…
archmoj Oct 18, 2018
156a9db
test parcoords relayout fix
alexcjohnson Oct 18, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 7 additions & 1 deletion src/traces/parcoords/parcoords.js
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -456,12 +456,18 @@ 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);

d.viewModel[d.key] = d.lineLayer;
d.lineLayer.render(d.viewModel.panels, !d.context);

var setChanged = ((d.key) &&
(((d.key !== 'contextLayer') || (callbacks)) || // unless there is callback on this line layer
(!d.context))); // don't update background

d.lineLayer.render(d.viewModel.panels, setChanged);
}
});

Expand Down
59 changes: 38 additions & 21 deletions test/jasmine/tests/parcoords_test.js
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -12,6 +12,7 @@ var failTest = require('../assets/fail_test');
var mouseEvent = require('../assets/mouse_event');
var click = require('../assets/click');
var supplyAllDefaults = require('../assets/supply_defaults');
var readPixel = require('../assets/read_pixel');

// mock with two dimensions (one panel); special case, e.g. left and right panel is obv. the same
var mock2 = require('@mocks/gl2d_parcoords_2.json');
Expand DownExpand Up@@ -579,11 +580,12 @@ describe('parcoords edge cases', function() {
});

describe('parcoords Lifecycle methods', function() {
var gd;
beforeEach(function() { gd = createGraphDiv(); });
afterEach(purgeGraphDiv);

it('Plotly.deleteTraces with one trace removes the plot', function(done) {

var gd = createGraphDiv();
var mockCopy = Lib.extendDeep({}, mock);

mockCopy.data[0].line.showscale = false;
Expand All@@ -603,7 +605,6 @@ describe('parcoords Lifecycle methods', function() {

it('@gl Plotly.deleteTraces with two traces removes the deleted plot', function(done) {

var gd = createGraphDiv();
var mockCopy = Lib.extendDeep({}, mock);
var mockCopy2 = Lib.extendDeep({}, mock);
mockCopy2.data[0].dimensions.splice(3, 4);
Expand DownExpand Up@@ -635,11 +636,25 @@ describe('parcoords Lifecycle methods', function() {
.then(done);
});

function _assertVisibleData(visible, msg) {
return function() {
var canvases = d3.selectAll('.gl-canvas');
expect(canvases.size()).toBe(3, msg);
canvases.each(function() {
var imageArray = readPixel(this, 0, 0, this.width, this.height);
var foundPixel = false;
var i = 0;
do {
foundPixel = foundPixel || imageArray[i++] !== 0;
} while(!foundPixel && i < imageArray.length);
expect(foundPixel).toBe(visible, msg + ' - ' + this.className);
});
};
}

it('@gl Calling `Plotly.restyle` with zero panels left should erase lines', function(done) {

var mockCopy = Lib.extendDeep({}, mock2);
var gd = createGraphDiv();
Plotly.plot(gd, mockCopy.data, mockCopy.layout);

function restyleDimension(key, dimIndex, setterValue) {
var value = Array.isArray(setterValue) ? setterValue[0] : setterValue;
Expand All@@ -650,27 +665,31 @@ describe('parcoords Lifecycle methods', function() {
};
}

restyleDimension('values', 1, [[]])()
.then(function() {
d3.selectAll('.parcoords-lines').each(function(d) {
var imageArray = d.lineLayer.readPixels(0, 0, d.model.canvasWidth, d.model.canvasHeight);
var foundPixel = false;
var i = 0;
do {
foundPixel = foundPixel || imageArray[i++] !== 0;
} while(!foundPixel && i < imageArray.length);
expect(foundPixel).toEqual(false);
});
})
.catch(failTest)
.then(done);
Plotly.plot(gd, mockCopy)
.then(_assertVisibleData(true, 'initial'))
.then(restyleDimension('values', 1, [[]]))
.then(_assertVisibleData(false, 'no panels'))

@alexcjohnsonalexcjohnsonOct 18, 2018

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

previously this test didn't bother checking that it was actually doing anything! d3.selectAll('.parcoords-lines') was an empty set - this must have been refactored at some point - so the .each block never ran. Updated it to not only ensure that it's doing something (expect(canvases.size()).toBe(3, msg); up above) but also to test that something changed (_assertVisibleData(true, 'initial') before the change, _assertVisibleData(false, 'no panels') afterward).

.catch(failTest)
.then(done);
});

it('@gl displays focused and context data after relayout', function(done) {
var mockCopy = Lib.extendDeep({}, mock2);

Plotly.plot(gd, mockCopy)
.then(_assertVisibleData(true, 'initial'))
.then(function() {
return Plotly.relayout(gd, 'paper_bgcolor', '#eef');
})
.then(_assertVisibleData(true, 'after relayout'))

@alexcjohnsonalexcjohnsonOct 18, 2018

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

@archmoj before your patch this fails with:

Expected false to be true, 'after relayout - gl-canvas gl-canvas-context'.

.catch(failTest)
.then(done);
});

describe('Having two datasets', function() {

it('@gl Two subsequent calls to Plotly.plot should create two parcoords rows', function(done) {

var gd = createGraphDiv();
var mockCopy = Lib.extendDeep({}, mock);
var mockCopy2 = Lib.extendDeep({}, mock);
mockCopy.data[0].domain = {x: [0, 0.45]};
Expand DownExpand Up@@ -700,7 +719,6 @@ describe('parcoords Lifecycle methods', function() {

it('@gl Plotly.addTraces should add a new parcoords row', function(done) {

var gd = createGraphDiv();
var mockCopy = Lib.extendDeep({}, mock);
var mockCopy2 = Lib.extendDeep({}, mock);
mockCopy.data[0].domain = {y: [0, 0.35]};
Expand All@@ -727,7 +745,6 @@ describe('parcoords Lifecycle methods', function() {

it('@gl Plotly.restyle should update the existing parcoords row', function(done) {

var gd = createGraphDiv();
var mockCopy = Lib.extendDeep({}, mock);
var mockCopy2 = Lib.extendDeep({}, mock);

Expand Down