Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork 2k
Contour legend#2891
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
Uh oh!
There was an error while loading. Please reload this page.
Contour legend #2891
Changes from all commits
44aa70e597a8457f0db7c0f532f37843d83e25b9ff73f279b74e7f841d05081File filter
Filter by extension
Conversations
Uh oh!
There was an error while loading. Please reload this page.
Jump to
Uh oh!
There was an error while loading. Please reload this page.
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' : | ||
Contributor 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! Contributor 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); | ||
Contributor 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 CollaboratorAuthor 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(' ') | ||
Contributor 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; | ||
CollaboratorAuthor 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 🙄 CollaboratorAuthor 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 Contributor 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', | ||
Uh oh!
There was an error while loading. Please reload this page.

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
getLegendDataand 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