Skip to content
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

Make it possible to set color of text labels on BarChartNode #49

Closed
jessegreenberg opened this issue Apr 30, 2020 · 7 comments
Closed

Comments

@jessegreenberg
Copy link
Contributor

They currently match the color of the bar only.

@jessegreenberg
Copy link
Contributor Author

This is done. There is now an entry to the "bar" object called labelColor which can specify the color of the label. The default is now black, NOT the bar color. I went with this because in phetsims/energy-skate-park#240 @arouinfar said

I would be fine with doing this universally across all energy graphs. I suspect that the colors do not have sufficient contrast against light gray or white backgrounds to pass at the WCAG AAA level. https://webaim.org/resources/contrastchecker/

This would impact pendulum-lab and masses-and-springs. I kept the labels with their colors for now. @arouinfar can you confirm that the labels in these sims should be black?

@arouinfar
Copy link

@jessegreenberg yes, I think we should change the labels in pendulum-lab and masses-and-springs to black. In the graph, the text is right next to the bar so I don't think an extra level of color-cueing is necessary. These sims do have an energy legend that I think we'll need to make some design tweaks to, so I'll open an issue in the sim repos.

Color Contrast Ratio Against White Comments
KINETIC_ENERGY rgb( 30, 200, 45 ) 2.24:1 Fail
GRAVITATIONAL_POTENTIAL_ENERGY rgb( 55, 130, 215 ) 3.49:1 Fails for normal text. Passes at AA level for large text.
ELASTIC_POTENTIAL_ENERGY rgb( 0, 204, 255 ) 1.89:1 Fail
HEAT_THERMAL_ENERGY rgb( 255, 85, 0 ) 3.2:1 Fails for normal text. Passes at AA level for large text.
TOTAL_ENERGY rgb(180, 180, 0) 2.21:1 Fail

@jessegreenberg
Copy link
Contributor Author

OK thanks @arouinfar - since you have made issues for the above two sims I think this issue can be closed.

@arouinfar
Copy link

@jessegreenberg I opened issues to fix the Energy Legend dialog which use the same colors as the bar graph labels, but I'd assumed the graph labels would be changed to black in this issue. Can you make this change, or should I open issues in the sim repos for that as well?

@arouinfar arouinfar reopened this May 1, 2020
@jessegreenberg
Copy link
Contributor Author

Ah sorry, I thought those issues included the change to the graphs as well. I can definitely do that here.

jessegreenberg added a commit to phetsims/masses-and-springs that referenced this issue May 5, 2020
jessegreenberg added a commit to phetsims/pendulum-lab that referenced this issue May 5, 2020
@jessegreenberg
Copy link
Contributor Author

OK, labels have been made black. Anything else to do @arouinfar?

@arouinfar
Copy link

Looks good in master, thanks @jessegreenberg!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants