-
-
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
Contour legend #2891
Contour legend #2891
Changes from 1 commit
44aa70e
597a845
7f0db7c
0f532f3
7843d83
e25b9ff
73f279b
74e7f84
1d05081
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,3 +1,5 @@ | ||
var d3 = require('d3'); | ||
|
||
var Plotly = require('@lib/index'); | ||
var Plots = require('@src/plots/plots'); | ||
var Lib = require('@src/lib'); | ||
|
@@ -414,12 +416,12 @@ describe('contour plotting and editing', function() { | |
|
||
function _assert(exp) { | ||
var msg = ' index in <g.contourlayer> (call #' + cnt + ')'; | ||
var contourLayer = gd.querySelector('.xy > .plot > .contourlayer'); | ||
var contourPlot = gd.querySelector('.xy > .plot > .contourlayer > .contour'); | ||
var hmIndex = -1; | ||
var contoursIndex = -1; | ||
|
||
for(var i in contourLayer.children) { | ||
var child = contourLayer.children[i]; | ||
for(var i in contourPlot.children) { | ||
var child = contourPlot.children[i]; | ||
if(child.querySelector) { | ||
if(child.querySelector('.hm')) hmIndex = +i; | ||
else if(child.querySelector('.contourlevel')) contoursIndex = +i; | ||
|
@@ -439,21 +441,21 @@ describe('contour plotting and editing', function() { | |
.then(function() { | ||
_assert({ | ||
hmIndex: 0, | ||
contoursIndex: 1 | ||
contoursIndex: 3 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Oh ok, that's tricky. Now that Maybe we could instead assert: Array.prototype.slice.call(contourPlot.children).map(n => n.classList[0])
// to check ordering
Array.prototype.slice.call(contourPlot.children).map(n => n.children.length)
// to check the number of children There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Since this is really about layer ordering, which is a consistently testable feature in SVG and likely to be useful elsewhere in our tests, I added a generic custom assertion for this. See what you think e25b9ff There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
💯 |
||
}); | ||
return Plotly.restyle(gd, 'contours.coloring', 'lines'); | ||
}) | ||
.then(function() { | ||
_assert({ | ||
hmIndex: -1, | ||
contoursIndex: 1 | ||
contoursIndex: 3 | ||
}); | ||
return Plotly.restyle(gd, 'contours.coloring', 'heatmap'); | ||
}) | ||
.then(function() { | ||
_assert({ | ||
hmIndex: 0, | ||
contoursIndex: 1 | ||
contoursIndex: 3 | ||
}); | ||
}) | ||
.catch(fail) | ||
|
@@ -501,4 +503,34 @@ describe('contour plotting and editing', function() { | |
.catch(fail) | ||
.then(done); | ||
}); | ||
|
||
it('keeps the correct ordering after hide and show', function(done) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nice test. Thanks! |
||
function getIndices() { | ||
var out = []; | ||
d3.selectAll('.contour').each(function(d) { out.push(d.trace.index); }); | ||
return out; | ||
} | ||
|
||
Plotly.newPlot(gd, [{ | ||
type: 'contour', | ||
z: [[1, 2], [3, 4]] | ||
}, { | ||
type: 'contour', | ||
z: [[2, 1], [4, 3]], | ||
contours: {coloring: 'lines'} | ||
}]) | ||
.then(function() { | ||
expect(getIndices()).toEqual([0, 1]); | ||
return Plotly.restyle(gd, 'visible', false, [0]); | ||
}) | ||
.then(function() { | ||
expect(getIndices()).toEqual([1]); | ||
return Plotly.restyle(gd, 'visible', true, [0]); | ||
}) | ||
.then(function() { | ||
expect(getIndices()).toEqual([0, 1]); | ||
}) | ||
.catch(fail) | ||
.then(done); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Oh would you mind There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. failTest in contour & carpet -> 7843d83 |
||
}); | ||
}); |
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.
Ah yeah. Way better.
By the looks of it,
heatmap
andcarpet
probably suffer from similar bugs:From which we could 🔪
getUidsFromCalcdata
entirely:No need to do this in this PR of course, but opening a new issue about it would be nice.
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.
Good call -> #2907