Skip to content

Sort transform #1609

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
merged 8 commits into from
May 10, 2017
Merged

Sort transform #1609

Changes from 1 commit
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
0be666f
first cut sort transform
etpinard Apr 19, 2017
36f7acd
Merge branch 'master' into sort-transform
etpinard Apr 26, 2017
1a21ad9
mv getTargetArray to lib/
etpinard Apr 26, 2017
823375c
mv getDataToCoordFunc in axes.js
etpinard Apr 26, 2017
2dd1333
make Lib.getTargetArray API more robust
etpinard May 9, 2017
9c70ee9
do not slice array just to read its values in sort loop
etpinard May 9, 2017
4f91957
:lock: behavior target arrays are shorter/longer than other arrays
etpinard May 9, 2017
4e8efe4
add test for sort transform side effects on eventData pointNumber
etpinard May 10, 2017
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
99 changes: 99 additions & 0 deletions test/jasmine/tests/transform_sort_test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ var d3 = require('d3');
var createGraphDiv = require('../assets/create_graph_div');
var destroyGraphDiv = require('../assets/destroy_graph_div');
var fail = require('../assets/fail_test');
var mouseEvent = require('../assets/mouse_event');

describe('Test sort transform defaults:', function() {
function _supply(trace, layout) {
Expand Down Expand Up @@ -241,4 +242,102 @@ describe('Test sort transform interactions:', function() {
.catch(fail)
.then(done);
});

it('does not preserve hover/click `pointNumber` value', function(done) {
Copy link
Contributor Author

@etpinard etpinard May 10, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

@n-riesco @alexcjohnson IMPORTANT: event data pointNumber does not correspond to the array index in gd.data after a sort transform. The same is true for filter transform with preservegaps: false. This is an unfortunate consequence of the current transform pipeline. Using ids become crucial here. That is, as presented in this test case:

var pt = eventData.points[0]

pt.fullData.ids[pt.pointNumber] 
// => is preserved after a sort transform

We could eventually add a originalPointNumber field to the event data to solve this problem. Moreover, we should really add an id field to the hover & click event data and make the ids data array more ubiquitous (it is only available for scatter traces at the moment).

var gd = createGraphDiv();

function getPxPos(gd, id) {
var trace = gd.data[0];
var fullLayout = gd._fullLayout;
var index = trace.ids.indexOf(id);

return [
fullLayout.xaxis.d2p(trace.x[index]),
fullLayout.yaxis.d2p(trace.y[index])
];
}

function hover(gd, id) {
return new Promise(function(resolve) {
gd.once('plotly_hover', function(eventData) {
resolve(eventData);
});

var pos = getPxPos(gd, id);
mouseEvent('mousemove', pos[0], pos[1]);
});
}

function click(gd, id) {
return new Promise(function(resolve) {
gd.once('plotly_click', function(eventData) {
resolve(eventData);
});

var pos = getPxPos(gd, id);
mouseEvent('mousemove', pos[0], pos[1]);
mouseEvent('mousedown', pos[0], pos[1]);
mouseEvent('mouseup', pos[0], pos[1]);
});
}

function wait() {
return new Promise(function(resolve) {
setTimeout(resolve, 60);
});
}

function assertPt(eventData, x, y, pointNumber, id) {
var pt = eventData.points[0];

expect(pt.x).toEqual(x, 'x');
expect(pt.y).toEqual(y, 'y');
expect(pt.pointNumber).toEqual(pointNumber, 'pointNumber');
expect(pt.fullData.ids[pt.pointNumber]).toEqual(id, 'id');
}

Plotly.plot(gd, [{
mode: 'markers',
x: [-2, -1, -2, 0, 1, 3, 1],
y: [1, 2, 3, 1, 2, 3, 1],
ids: ['A', 'B', 'C', 'D', 'E', 'F', 'G'],
marker: {
size: [10, 20, 5, 1, 6, 0, 10]
},
transforms: [{
enabled: false,
type: 'sort',
target: 'marker.size',
}]
}], {
width: 500,
height: 500,
margin: {l: 0, t: 0, r: 0, b: 0},
hovermode: 'closest'
})
.then(function() { return hover(gd, 'D'); })
.then(function(eventData) {
assertPt(eventData, 0, 1, 3, 'D');
})
.then(wait)
.then(function() { return click(gd, 'G'); })
.then(function(eventData) {
assertPt(eventData, 1, 1, 6, 'G');
})
.then(wait)
.then(function() {
return Plotly.restyle(gd, 'transforms[0].enabled', true);
})
.then(function() { return hover(gd, 'D'); })
.then(function(eventData) {
assertPt(eventData, 0, 1, 1, 'D');
})
.then(wait)
.then(function() { return click(gd, 'G'); })
.then(function(eventData) {
assertPt(eventData, 1, 1, 5, 'G');
})
.catch(fail)
.then(done);
});
});