-
-
Notifications
You must be signed in to change notification settings - Fork 1.9k
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.
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
Display parcoords deselected lines #3123
Conversation
Looks great @archmoj ! Can you add a test to 🔒 this fix? I was going to suggest modeling it after this test: plotly.js/test/jasmine/tests/parcoords_test.js Lines 655 to 663 in 3f95f04
(with the inverse expectation of course, Don't spend any time fixing that test now unless it's trivial, but please do come up with a similar test showing that all the layers have some data in them after an edit. No need to simulate clicking the modebar button, some other edit like the one I mentioned in #3099 |
also fix parcoords restyle test while we're at it
.then(function() { | ||
return Plotly.relayout(gd, 'paper_bgcolor', '#eef'); | ||
}) | ||
.then(_assertVisibleData(true, 'after relayout')) |
There was a problem hiding this comment.
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'.
Plotly.plot(gd, mockCopy) | ||
.then(_assertVisibleData(true, 'initial')) | ||
.then(restyleDimension('values', 1, [[]])) | ||
.then(_assertVisibleData(false, 'no panels')) |
There was a problem hiding this comment.
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).
Thank you @alexcjohnson |
This PR could fix the issue #3099 i.e. before the implementation of hovermode for parcoords.
@alexcjohnson