Skip to content

Fix stacked area layering and deletion bugs #3005

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 4 commits into from
Sep 14, 2018
Merged

Fix stacked area layering and deletion bugs #3005

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
37ee992
fix #3003 - fill and layering for multiple stackgroups and unstacked …
alexcjohnson Sep 12, 2018
f6c5d56
fix #3002 - allow deleting stack groups
alexcjohnson Sep 13, 2018
fe0c988
describe fill reordering in scatter attributes
alexcjohnson Sep 13, 2018
72c61c7
clean cruft out of stacked_area_groups mock
alexcjohnson Sep 14, 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/traces/scatter/plot.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ module.exports = function plot(gd, plotinfo, cdscatter, scatterLayer, transition
});
});
} else {
scatterLayer.selectAll('g.trace').each(function(d, i) {
join.each(function(d, i) {
Copy link
Collaborator Author

@alexcjohnson alexcjohnson Sep 13, 2018

Choose a reason for hiding this comment

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

@etpinard it doesn't seem like animations can remove traces, is that correct? That's what it looks like, since I don't see a join.exit() associated with them (since isFullReplot = !transitionOpts)... yet the comment above implies that you can add traces:

// Must run the selection again since otherwise enters/updates get grouped together
// and these get executed out of order. Except we need them in order!

So... do I need to worry about animations deleting traces or not?

Copy link
Contributor

@etpinard etpinard Sep 13, 2018

Choose a reason for hiding this comment

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

do I need to worry about animations deleting traces or not?

Right, removing traces with 'frame.redraw': false (i.e. without a replot at the end of the animation), doesn't work currently:

peek 2018-09-13 16-17

https://codepen.io/etpinard/pen/ZMRReB

so I don't think you broke anything here.

Copy link
Contributor

@etpinard etpinard Sep 14, 2018

Choose a reason for hiding this comment

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

Ah Ricky wrote an issue about this #932

plotOne(gd, i, plotinfo, d, cdscatterSorted, this, transitionOpts);
});
}
Expand Down
33 changes: 33 additions & 0 deletions test/jasmine/tests/scatter_test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1184,6 +1184,39 @@ describe('stacked area', function() {
.then(done);
});

it('can add/delete stack groups', function(done) {
var data01 = [
{mode: 'markers', y: [1, 2, -1, 2, 1], stackgroup: 'a'},
{mode: 'markers', y: [2, 3, 2, 3, 2], stackgroup: 'b'}
];
var data0 = [Lib.extendDeep({}, data01[0])];
var data1 = [Lib.extendDeep({}, data01[1])];

function _assert(yRange, nTraces) {
expect(gd._fullLayout.yaxis.range).toBeCloseToArray(yRange, 2);
expect(gd.querySelectorAll('g.trace.scatter').length).toBe(nTraces);
}

Plotly.newPlot(gd, data01)
.then(function() {
_assert([-1.293, 3.293], 2);
return Plotly.react(gd, data0);
})
.then(function() {
_assert([-1.220, 2.220], 1);
return Plotly.react(gd, data01);
})
.then(function() {
_assert([-1.293, 3.293], 2);
return Plotly.react(gd, data1);
})
.then(function() {
_assert([0, 3.205], 1);
})
.catch(failTest)
.then(done);
});

it('does not stack on date axes', function(done) {
Plotly.newPlot(gd, [
{y: ['2016-01-01', '2017-01-01'], stackgroup: 'a'},
Expand Down