Uh oh!
There was an error while loading. Please reload this page.
Parcoords: Use paper background color for text shadow for better dark mode - #5506
Conversation
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
jonmmease
commented
Feb 16, 2021
Thanks for the feedback @archmoj, it all makes good sense! I'll try to make the updates this evening. |
Use in parcats, parcoords, and sankey traces
jonmmease
commented
Feb 17, 2021
@archmoj, I believe all of your suggestions are implemented now. Thanks! |
archmoj
commented
Feb 17, 2021
Discussing this PR with @alexcjohnson, we thought we need a |
| paperColor + ' 1px 1px 2px, ' + | ||
| paperColor + ' 1px -1px 2px, ' + | ||
| paperColor + ' -1px -1px 2px') | ||
| .style('text-shadow', svgTextUtils.makeTextShadow(1, 1, 2, paperColor)) |
There was a problem hiding this comment.
Interesting to see that parcats uses different "2" for blur-radius instead of 1.
@alexcjohnson do you think we should declare and unify this in a new layout container?
There was a problem hiding this comment.
Good 👁️
To me the 1 vs 2 doesn't make much visible difference, I'd probably standardize on 1. Also given that the purpose here is really outlining rather than a 3D-like shadow I'd combine offsetX and offsetY into a single offset.
There was a problem hiding this comment.
I'm fine switching to 1 and regenerating the parcats baselines. And fine with combining the offsets.
| var x = offsetX + 'px '; | ||
| var y = offsetY + 'px '; | ||
| var b = blurRadius + 'px '; | ||
| var clr = color + ' '; |
alexcjohnson
commented
Feb 17, 2021
I was thinking a trace attribute actually, named to correspond with whatever trace attribute sets the color of the text we're outlining. I wonder actually whether we could put this attribute into that font container, or if that would cause problems given that font containers have a standardized structure? Anyway, the logic for the default outline color, as @archmoj and I discussed:
varfontContrast=Color.contrast(fontColor);varpaperContrast=Color.contrast(paperColor);vardfltOutlineColor=(fontContrast===paperContrast) ? fontContrast : paperColor; |
alexcjohnson
commented
Feb 17, 2021
For now we could probably just use that logic without adding a new outline color attribute. |
jonmmease
commented
Feb 17, 2021
A potential issue choosing the shadow automatically is that, for |
alexcjohnson
commented
Feb 17, 2021
The point is just to create contrast with the text color. If it looks like the paper color will do that we would use it because we know this will fit with the overall style of the plot, otherwise use black or white. There can be lots of path colors, and your labels can bleed off the paths and onto the background, so I don't think we should involve path color at all. |
jonmmease
commented
Feb 18, 2021
Ok, here's what the auto cases look like for parcats: These make it pretty obvious that we don't currently use a shadow for the dimension labels, so it's not really usable this way in current form unless we add dimension label shadows as well. My perspective on this is that currently, if a user changes to a dark paper color they already need to change the font color to something light for basically any of our traces to look good. By having the shadow always match the paper color, the internal parcats/parcoords/sankey labels have the same color contrast characteristics as any other text that ends up being drawn on the paper (titles, tick labels, etc.). It doesn't really feel helpful to me for these labels to be special cases in terms of their contrast behavior. Unless our long-term goals is to go all in here and add shadows to all text when we determine there is poor contrast. |
alexcjohnson
commented
Feb 18, 2021
Sold. Those examples with similar text & paper colors look hideous using my proposed logic.
Probably not automatically, but I am starting to think we should add shadow support to all fonts at some point, and then this can just be used as the default shadow for the relevant fonts. |
| }; | ||
| } | ||
| exports.makeTextShadow = function(offset, color) { |
There was a problem hiding this comment.
Now that offset is always 1, let's simplify this function:
exports.makeTextShadow=function(color){return('1px 1px 1px'+color+', '+'-1px -1px 1px'+color+', '+'1px -1px 1px'+color+', '+'-1px 1px 1px'+color);};But wait why do we need these four corners?
There was a problem hiding this comment.
See discussion in https://www.codesdope.com/blog/article/adding-outline-to-text-using-css/
archmoj
commented
Feb 18, 2021
Nicely done. |
jonmmease
commented
Feb 19, 2021
Thanks @archmoj ! |


Extracted from #5500.
Overview
Simple change to
parcoordsto use the figure'spaper_bgcoloras the text shadow color and the axis brush border color. Before, these colors were hard-coded to white. This makesparcoordsusable with a dark background.Before
After