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

Add controls to hide combinations not meeting a contrast threshold #8

Open
wants to merge 12 commits into
base: main
Choose a base branch
from

Conversation

extra808
Copy link

I thought it would be helpful to add the ability to hide color combinations in the grid that fail to meet a threshold based on the WCAG color contrast calculation. In addition to having buttons to set the threshold at values specified in WCAG criteria (3:1, 4.5:1, 7:1), I included a "range" input so any contrast value can be used.

Here's a copy of this contrast grid branch, I tweaked the URLs so it wouldn't have to be at the root of a domain.

@netlify
Copy link

netlify bot commented Jun 17, 2022

Deploy Preview for dazzling-hermann-6e0d29 ready!

Name Link
🔨 Latest commit 9d3edcf
🔍 Latest deploy log https://app.netlify.com/sites/dazzling-hermann-6e0d29/deploys/66d473ed7f2790000845e46d
😎 Deploy Preview https://deploy-preview-8--dazzling-hermann-6e0d29.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@mherchel
Copy link
Owner

Ha! Somehow I missed this PR. Will set a reminder to give a review shortly :)

@mherchel
Copy link
Owner

@extra808 Sorry that this fell off my radar. I'm looking at your branch, but I don't see the controls. What am I missing?

image

@benjifisher
Copy link

@mherchel: What URL are you using?

This is what I see following the link from the Netlify bot (https://deploy-preview-8--dazzling-hermann-6e0d29.netlify.app/):

Screenshot 2024-08-28 at 1 32 29 PM

@mherchel
Copy link
Owner

Hmmm... I pulled it down locally and did not see that. I'm about to head into meeting and then will give it a closer look.

@mherchel
Copy link
Owner

It was a user error on why I didn't see it before 🤦

Anyway, this is looking amazing. I haven't done the code review yet, but I do have some requests to the UI:

  1. Can you remove the checkbox for "Hide Colors Below X"? Instead lets start the slider set to maximum, so it will show all colors by default.
  2. Change the label of the slider to "Showing colors below a X:1 contrast ratio".
  3. But if the maximum is selected on the slider, change the label to "Showing all colors"
  4. Lets have the grid update dynamically when the slider is changed.

Let me know if you strongly disagree with any of this. Thank you!

@extra808
Copy link
Author

  1. Good idea.
  2. I think so. Screen readers are funny about control names changing dynamically and it'll already be busy announcing the value change. Usually you want the name change to be announced ("Play," "Pause") but in this case you probably don't.
  3. Ditto.
  4. Because there's a form for submitting the values in the textareas, I was treating the contrast as just another form input. Maybe it doesn't need that. I'll need to see if there's any problem if the range input is used before any color values have been submitted.

@extra808
Copy link
Author

extra808 commented Sep 1, 2024

The range input now dynamically shows/hides the table cells based on whether they meet the contrast threshold or not.

By default, it's set to '1' so all cells are shown. I changed the input label to "Show colors with at least X:1 contrast ratio," updating the 'X' as the value changes (it's also an ARIA live region so it's announced). As the input is being used, NVDA repeats the input's label, presumably because its content has changed, VoiceOver doesn't.

I didn't make the label change to something like "Showing all colors" when the ratio is 1:1 because I don't think it conveys what using the input does as well.

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

Successfully merging this pull request may close these issues.

None yet

3 participants