Skip to content

Faster trace visibility toggling #2837

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

Closed
wants to merge 9 commits into from
Closed

Faster trace visibility toggling #2837

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
e85725a
cleanup in scattergl sceneUpdate & plot
etpinard Jul 23, 2018
1675047
swap visible editType 'calc' -> 'plot'
etpinard Jul 19, 2018
3790734
push trace module into fullLayout._modules even if visible:false
etpinard Jul 23, 2018
cf65abd
use trace.uid-based visible batch array
etpinard Jul 23, 2018
b816a8e
fill in list of visible:true module in fullLayout._visibleModules
etpinard Jul 23, 2018
25ed567
fix and :lock: splom trace visible toggling
etpinard Jul 23, 2018
1f61ff0
adapt scatterpolargl for scene.visibleBatch
etpinard Jul 23, 2018
f1f06f4
add a few comments
etpinard Jul 23, 2018
4af6528
adapt and improve scattergl visible toggle tests
etpinard Jul 23, 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
2 changes: 1 addition & 1 deletion src/plots/cartesian/type_defaults.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ function getFirstNonEmptyTrace(data, id, axLetter) {

if(trace.type === 'splom' &&
trace._length > 0 &&
trace['_' + axLetter + 'axes'][id]
(trace['_' + axLetter + 'axes'] || {})[id]
) {
return trace;
}
Expand Down
35 changes: 35 additions & 0 deletions test/jasmine/tests/splom_test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -579,6 +579,41 @@ describe('@gl Test splom interactions:', function() {
.catch(failTest)
.then(done);
});

it('should toggle trace correctly', function(done) {
var fig = Lib.extendDeep({}, require('@mocks/splom_iris.json'));

function _assert(msg, exp) {
for(var i = 0; i < 3; i++) {
var draw = gd.calcdata[i][0].t._scene.draw;
expect(draw).toHaveBeenCalledTimes(exp[i], msg + ' - trace ' + i);
draw.calls.reset();
}
}

Plotly.plot(gd, fig).then(function() {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is broken on master, consider this a bug fix.

spyOn(gd.calcdata[0][0].t._scene, 'draw');
spyOn(gd.calcdata[1][0].t._scene, 'draw');
spyOn(gd.calcdata[2][0].t._scene, 'draw');

return Plotly.restyle(gd, 'visible', 'legendonly', [0, 2]);
})
.then(function() {
_assert('0-2 legendonly', [0, 1, 0]);

return Plotly.restyle(gd, 'visible', false);
})
.then(function() {
_assert('all gone', [0, 0, 0]);

return Plotly.restyle(gd, 'visible', true);
})
.then(function() {
_assert('all back', [1, 1, 1]);
})
.catch(failTest)
.then(done);
});
});

describe('@gl Test splom hover:', function() {
Expand Down