Fix low color contrast issues across several widgets #1282
Conversation
Myranae
changed the title
|
Size Change: -166 B (-0.02%) Total Size: 843 kB
ℹ️ View Unchanged
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1282 +/- ##
==========================================
+ Coverage 69.59% 71.22% +1.62%
==========================================
Files 485 489 +4
Lines 102927 102997 +70
Branches 7376 11132 +3756
==========================================
+ Hits 71633 73357 +1724
+ Misses 31178 29640 -1538
+ Partials 116 0 -116
... and 121 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
|
Myranae
changed the title
| INTERACTING: color.green, | ||
| INTERACTIVE: color.green, |
There was a problem hiding this comment.
Could also just combine these into one since they are both the same color. Thoughts?
There was a problem hiding this comment.
I'd be down for that! How many changes would that add to this PR? Or does it have wider consequences?
There was a problem hiding this comment.
Hmm, there are 45 instances of INTERACTIVE and 26 instances of INTERACTING all across 13 files. I would probably make INTERACTING just be INTERACTIVE, so it would only add 5 files with changes.
I don't think it would have wider consequences since both are using the same color, though I don't know if the change in meaning would be troublesome.
Perhaps this should be part of a wider change of updating KhanColors at some point and saved for another time since it has so many instances. I think I assumed it would be a smaller change. 😅
There was a problem hiding this comment.
Yeah in that case I would hold off and just submit a backlog task to revisit in the future 😉
| // Don't actually use _BACKGROUND! Make things transparent instead. The | ||
| // background color used in exercises is subject to change at the whim | ||
| // of any redesigns. | ||
| _BACKGROUND: "#FDFDFD", // TODO(eater): Get rid of this altogether. |
There was a problem hiding this comment.
Got rid of this part since both comments seem to think it is not needed. Replaced where they are used with just "transparent".
GeraldRequired Reviewers
Don't want to be involved in this pull request? Comment |
|
Do you have screen shots of the UX changes with the new colors? :) |
|
Were you able to run an accessibility scan and confirm the color doesn't have any more issues? |
|
Not sure how it works here just yet, but were you able to confirm or check with a designer on these new colors? |
Ooo, thank you @catandthemachines! Will get these added :D |
@catandthemachines Hmm, I'm not sure about an accessibility scan. How would I do that and what other issues are possible? I did triple check the ratio was better. I noticed the ratio between the lines and the dots might not be ideal, but not sure if that is fixable since the dot color has to have good contrast with the background (and so does the line). |
npm Snapshot: PublishedGood news!! We've packaged up the latest commit from this PR (6d5cd30) and published it to npm. You Example: yarn add @khanacademy/perseus@PR1282If you are working in Khan Academy's webapp, you can run: ./dev/tools/bump_perseus_version.sh -t PR1282 |
@catandthemachines I added a link to the PR summary that leads to the conversation I had with the team and a designer about what colors to use. Planning to make a ZND and check with our QE person on the change as well. (Before merging but after things slow down a bit). |
There are a lot of great accessibility scan tools for Chrome browsers (I use Accessibility Insights when I want to confirm is my site has any issues or not https://chromewebstore.google.com/detail/accessibility-insights-fo/pbjjkligggfmakdaogkfomddhfmpjeni ). For the question on the dot vs. the line, that might be a great question for Level Access to give us suggestions. I can't remember off the top of my head but I believe distinguishing content in a visual diagram has different color contrast requirements than text vs. background. |
| INTERACTING: color.green, | ||
| INTERACTIVE: color.green, |
There was a problem hiding this comment.
I'd be down for that! How many changes would that add to this PR? Or does it have wider consequences?
Summary:
Worked on as part of GAAD.
The issue here was specific to the low contrast found for the dot and line in number-line. This fix changes the KhanColors object in Perseus which is used across a number of widgets. The changes here specifically affect grapher, number-line, interaction, and interactive-graph (based on the updated snapshots).
Discussion about what colors to use. Includes input from a designer
Before:
After:
Here's also some manual testing of how it looks in webapp (test everything course). Checked the other three types of graphs also affected by this as well.
Before:
After:
Issue: LEMS-252
Test plan: