-
-
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 all commits
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 |
---|---|---|
|
@@ -61,25 +61,78 @@ module.exports = function style(s, gd) { | |
var showFill = trace.visible && trace.fill && trace.fill !== 'none'; | ||
var showLine = subTypes.hasLines(trace); | ||
var contours = trace.contours; | ||
var showGradientLine = false; | ||
var showGradientFill = false; | ||
|
||
if(contours && contours.type === 'constraint') { | ||
showLine = contours.showlines; | ||
showFill = contours._operation !== '='; | ||
if(contours) { | ||
var coloring = contours.coloring; | ||
|
||
if(coloring === 'lines') { | ||
showGradientLine = true; | ||
} | ||
else { | ||
showLine = coloring === 'none' || coloring === 'heatmap' || | ||
contours.showlines; | ||
} | ||
|
||
if(contours.type === 'constraint') { | ||
showFill = contours._operation !== '='; | ||
} | ||
else if(coloring === 'fill' || coloring === 'heatmap') { | ||
showGradientFill = true; | ||
} | ||
} | ||
|
||
var fill = d3.select(this).select('.legendfill').selectAll('path') | ||
.data(showFill ? [d] : []); | ||
// with fill and no markers or text, move the line and fill up a bit | ||
// so it's more centered | ||
var markersOrText = subTypes.hasMarkers(trace) || subTypes.hasText(trace); | ||
var anyFill = showFill || showGradientFill; | ||
var anyLine = showLine || showGradientLine; | ||
var pathStart = (markersOrText || !anyFill) ? 'M5,0' : | ||
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 touch! 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. |
||
// with a line leave it slightly below center, to leave room for the | ||
// line thickness and because the line is usually more prominent | ||
anyLine ? 'M5,-2' : 'M5,-3'; | ||
|
||
var this3 = d3.select(this); | ||
|
||
var fill = this3.select('.legendfill').selectAll('path') | ||
.data(showFill || showGradientFill ? [d] : []); | ||
fill.enter().append('path').classed('js-fill', true); | ||
fill.exit().remove(); | ||
fill.attr('d', 'M5,0h30v6h-30z') | ||
.call(Drawing.fillGroupStyle); | ||
fill.attr('d', pathStart + 'h30v6h-30z') | ||
.call(showFill ? Drawing.fillGroupStyle : fillGradient); | ||
|
||
var line = d3.select(this).select('.legendlines').selectAll('path') | ||
.data(showLine ? [d] : []); | ||
line.enter().append('path').classed('js-line', true) | ||
.attr('d', 'M5,0h30'); | ||
var line = this3.select('.legendlines').selectAll('path') | ||
.data(showLine || showGradientLine ? [d] : []); | ||
line.enter().append('path').classed('js-line', true); | ||
line.exit().remove(); | ||
line.call(Drawing.lineGroupStyle); | ||
|
||
// this is ugly... but you can't apply a gradient to a perfectly | ||
// horizontal or vertical line. Presumably because then | ||
// the system doesn't know how to scale vertical variation, even | ||
// though there *is* no vertical variation in this case. | ||
// so add an invisibly small angle to the line | ||
// This issue (and workaround) exist across (Mac) Chrome, FF, and Safari | ||
line.attr('d', pathStart + (showGradientLine ? 'l30,0.0001' : 'h30')) | ||
.call(showLine ? Drawing.lineGroupStyle : lineGradient); | ||
|
||
function fillGradient(s) { | ||
if(s.size()) { | ||
var gradientID = 'legendfill-' + trace.uid; | ||
Drawing.gradient(s, gd, gradientID, 'horizontalreversed', | ||
trace.colorscale, 'fill'); | ||
} | ||
} | ||
|
||
function lineGradient(s) { | ||
if(s.size()) { | ||
var gradientID = 'legendline-' + trace.uid; | ||
Drawing.lineGroupStyle(s); | ||
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. We should double-check that this works ok with 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. Good call, added to a mock in 73f279b |
||
Drawing.gradient(s, gd, gradientID, 'horizontalreversed', | ||
trace.colorscale, 'stroke'); | ||
} | ||
} | ||
|
||
} | ||
|
||
function stylePoints(d) { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -173,7 +173,13 @@ module.exports = { | |
valType: 'boolean', | ||
role: 'info', | ||
editType: 'legend', | ||
description: 'Determines whether or not a legend is drawn.' | ||
description: [ | ||
'Determines whether or not a legend is drawn.', | ||
'Default is `true` if there is a trace to show and any of these:', | ||
'a) Two or more traces would by default be shown in the legend.', | ||
'b) One pie trace is shown in the legend.', | ||
'c) One trace is explicitly given with `showlegend: true`.' | ||
].join(' ') | ||
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. thanks for 📚 |
||
}, | ||
colorway: { | ||
valType: 'colorlist', | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -1144,9 +1144,13 @@ plots.supplyTraceDefaults = function(traceIn, traceOut, colorIndex, layout, trac | |
coerce('ids'); | ||
|
||
if(Registry.traceIs(traceOut, 'showLegend')) { | ||
traceOut._dfltShowLegend = true; | ||
coerce('showlegend'); | ||
coerce('legendgroup'); | ||
} | ||
else { | ||
traceOut._dfltShowLegend = false; | ||
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. #749 🙄 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. For the record: I added 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. yeah, that's what I expected. I don't think there's a nicer way to do this than what you coded in this commit 👌 |
||
} | ||
|
||
Registry.getComponentMethod( | ||
'fx', | ||
|
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.
This was used in
getLegendData
and inlegendDefaults
, but even before one of them overrode half the logic, and now that logic has diverged further, so it didn't make sense to pull it out.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.
nice 🔪 job here