Add options to display text over heatmaps & histogram2d #6028
Conversation
- add texttemplate and textfont
|
Looks awesome! Let's make sure we get auto-color-contrasting and a new attribute to optionally force the color. And let's get this in histogram2d if it's not already done ;) |
Done in b52adf2. |
|
Nice! For baseline 13, white-on-red seems very hard to read to me... is that normal? |
- corect label positions for histograd2dcontours
archmoj
changed the title
| @@ -42,6 +42,18 @@ module.exports = extendFlat({ | |||
| yhoverformat: axisHoverFormat('y'), | |||
| zhoverformat: axisHoverFormat('z', 1), | |||
| hovertemplate: heatmapAttrs.hovertemplate, | |||
| texttemplate: extendFlat({}, heatmapAttrs.texttemplate, { | |||
| description: [ | |||
| 'For this trace it only has an effect if `coloring` is set to *heatmap*.', | |||
There was a problem hiding this comment.
Why this restriction? Not a big deal, probably won't get a lot of use in contour maps anyway. But the only issue I can think of is what autocolor to give the text - for coloring='fill' it should work pretty well with the same logic as heatmap, and for other coloring values we'd just contrast with the plot_bgcolor. No?
There was a problem hiding this comment.
I agree we may try to add this option for other coloring values in another PR.
But that would require extra work as those use different plot paths.
There was a problem hiding this comment.
A contour plot with a coloring other than heatmap has different plotting path. So enabling this feature requires extra work. So it could potentially be added later on the road.
| @@ -332,6 +352,185 @@ module.exports = function(gd, plotinfo, cdheatmaps, heatmapLayer) { | |||
| y: top, | |||
| 'xlink:href': canvas.toDataURL('image/png') | |||
| }); | |||
|
|
|||
| removeLabels(plotGroup); | |||
There was a problem hiding this comment.
In principle we should be able to do this with the d3 add/remove/update idiom, rather than removing and re-adding everything. The performance seems fine with all the mocks you added text to here, but I worry about it bogging down during interactions in more intense cases, like if we update https://dash.plotly.com/dash-bio/alignmentchart with this feature (try dragging the rangeslider there right now, it's terrible)
There was a problem hiding this comment.
When trying
Plotly.relayout(gd, 'xaxis.rangeslider', {});
Plotly.restyle(gd, 'texttemplate', '%{z}');e.g. on earth_heatmap the performance seems good.
So I suggest we move forward with this feature and come back to further optimize it if we hit a performance problem.
There was a problem hiding this comment.
OK - it is a little fiddly to get the add/remove/update idiom working properly, and anyway perhaps the higher-value addition to this feature would be a way to disable showing labels at all beyond a certain scale as you zoom out, which we have already decided not to do in this PR.
|
|
||
| labels | ||
| .enter() | ||
| .append('g') |
There was a problem hiding this comment.
Do we need each text element wrapped in a separate group? Could we get away with one group with all the text elements inside it?
There was a problem hiding this comment.
This quite similar to the way we draw ticklabels:
plotly.js/src/plots/cartesian/axes.js
Line 2980 in 10d3930
There was a problem hiding this comment.
Right, I think this is important for MathJax purposes, so I guess if we think supporting that in heatmap text is something we want to do in the future we can leave it.
Resolves #1116 by adding
texttemplateandtextfontoptions.@plotly/plotly_js