Skip to content

Use paper_bgcolor for parcats text-shadow instead of hardcoding to white #3191

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

Merged
merged 1 commit into from
Oct 31, 2018

Conversation

jonmmease
Copy link
Contributor

Fix to make parcats look good with a dark theme. Previously the SVG text shadow behind the category labels was hardcoded to white. But this makes the text unbearably blurry when the font is white and the background is dark:

newplot 70

This PR makes the text-shadow take on the color of the paper_bgcolor. This doesn't change the appearance with a white background, but significantly improves the situation for dark background:

newplot 72

This image is from a new parcats_dark mock to 🔒 it in.

What about Sankey?
The text shadow logic was largely borrowed from the sankey trace type, so I took a look at this trace on a black background as well.

newplot 73

It looks like the sankey trace doesn't pick up it's default font color from layout.font.color, and it draws all node labels on top of paths, so it's default behavior still works reasonable well on a dark background. So I didn't make any changes to it.

This keeps category labels from becomming blurred when text is white
on a black background
@alexcjohnson
Copy link
Collaborator

Definitely an improvement! I'm just wondering if we want to go one step further and make a new attribute (tickshadow?) that defaults to paper_bgcolor but people could set it to whatever they want (including transparent).

@alexcjohnson
Copy link
Collaborator

I guess though if we want this to be categorized as a bugfix, so it can go in a patch, it should stand as is...

@etpinard
Copy link
Contributor

Nice fix 💃

@etpinard
Copy link
Contributor

Looks like @jonmmease isn't online at the moment. I'll merge this in to make it part of 1.42.1

@etpinard etpinard merged commit f0f9a86 into master Oct 31, 2018
@etpinard etpinard deleted the parcats_dark branch October 31, 2018 13:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug something broken
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants