-
-
Notifications
You must be signed in to change notification settings - Fork 2.7k
replace rgb codes with CSS colors including plotly recommended colors #5175
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
base: doc-prod
Are you sure you want to change the base?
Conversation
Before the Plotly team takes too much of a look at the details of this, I'd welcome a conversation about whether to expand the scope of this PR to address other non compliance in this page (e.g. hardcoded data) and whether this is worth pursuing at this time. I could also close out this PR and replace it with a PR targeting just hex code colors in 6 files in the docs. Hex codes are rarer than rgb colors in the docs. |
thanks @rl-utility-man - I'll merge this in as part of 6.2. |
@@ -200,15 +198,15 @@ for yd, xd in zip(y_data, x_data): | |||
x=space + (xd[i]/2), y=yd, | |||
text=str(xd[i]) + '%', | |||
font=dict(family='Arial', size=14, | |||
color='rgb(248, 248, 255)'), | |||
color=f"{'white'*(i<2)}{'black'*(i>=2)}"), |
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.
Why this f-string for the color? Could it not just be a named color that is close to rgb(248, 248, 255)
?
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 is color of the text that goes on the bars, which is currently CSS GhostWhite and is already has less than ideal contrast with lightest bar. When I went to CSS colors, I ended up with bigger differences between the lightest and darkest bars and struggled to find one color that had enough contrast for all of them. I make the text on the first two bars white and the text on the last three bars black to provide adequate contrast.
Here is the proposed bar graph with all white text -- the middle bar in particular is nearly illegible. Yellow and gray perform similarly.
Here's a potential alternative fix --swapping Cyan for DarkSlateBlue and then using all white text
I welcome thoughts. I am happy to comment this to explain, perhaps: "make the text on the first two bars white and the text on the last three bars black to provide adequate contrast." I use this style of conditional F-string a fair bit in my production code and this is a fairly rich, polished example where it might be helpful to show that technique.
doc/python/horizontal-bar-charts.md
Outdated
'rgba(190, 192, 213, 1)'] | ||
|
||
colors = ['DarkBlue', 'MediumBlue', 'cyan', 'mediumpurple', 'thistle'] | ||
#, 'silver' |
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.
Can this comment go?
this PR is entirely trying to apply the following expectation. I often chose CSS colors closer to those in the original graphics even if they were not on the plotly list
....
...