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

[Cub] - Color contrast ratio of some elements fail WCAG AA or AAA requirements #39

Closed
moody2times opened this issue Oct 19, 2020 · 14 comments · Fixed by #44
Closed

[Cub] - Color contrast ratio of some elements fail WCAG AA or AAA requirements #39

moody2times opened this issue Oct 19, 2020 · 14 comments · Fixed by #44
Assignees
Labels
bug Something isn't working good first issue Good for newcomers

Comments

@moody2times
Copy link
Contributor

moody2times commented Oct 19, 2020

Describe the bug

The color contrast ratio between the background and foreground elements on the page do not meet the WCAG AA or AAA requirements. As such users with color blindness or some other similar conditions will have trouble differentiating between the background and foreground colors, which is bad user-experience.

To Reproduce

On a firefox browser, right click the page (Cub) and click on inspect elements. In the devtools, expand the tab and click accessibility and from the accessibility window, click on check for issues.

Expected behavior

The texts on the page should be legible to all users. To ensure legibility, the color contrast of the elements in the photo below should be adjusted to meet at least WCAG AA requirements. Read more about the guideline here https://developer.mozilla.org/en-US/docs/Web/Accessibility/Understanding_WCAG/Perceivable/Color_contrast?utm_source=devtools&utm_medium=a11y-panel-checks-color-contrast

Browser information

Firefox v.8.0.1

Photo Description

contrast-a11y

@moody2times moody2times added bug Something isn't working good first issue Good for newcomers hacktoberfest labels Oct 19, 2020
@moody2times
Copy link
Contributor Author

moody2times commented Oct 19, 2020

I have experimented with different colors and the following are my recommendations -

For the large text, namely - remove-red-eye and preview-text elements - #949594 or #2E97FF has a 3:1 ratio which is the exact requirement for WCAG AA. Here is a before and after sample - before #C7E3FF / after #949594 or #2E97FF in order to maintain the theme. But I would recommend #949594 because it meets the intention of the preview text, which is to have a faint text.

For the remaining elements which are body text (small text), #0576DF has a 4.5:1 ratio which is the exact requirement for WCAG AA in respect of small or body text. Here is a before and after sample - before #037DE1 / after #0576DF.

@mmaismma
Copy link
Member

mmaismma commented Oct 19, 2020

Cub's Contraption has both dark and light modes. Different themes have different colors for almost all elements.
For example, Choose Background has #037de1 color in light theme but #eeeeee in dark theme.
As per me, the dark theme contrasts meet the guidelines perfectly. So only the colors in the light theme need changing.

The Preview text color should be changed directly where it is mentioned but for the body text, change the CSS variable --themeColor mentioned under html. Changing the variable will change the color of everything wanted.

@SaravgiYash
Copy link

@mmaismma I would like to work on this issue. Can you please guide me on what all changes do I need to make and in which file

@mmaismma
Copy link
Member

@Yashs911 Sure I would. Although this issue is already assigned to @moody2times, I think they are kind enough for you to work on this issue instead. He must have already done his research, so I would leave @moody2times as your mentor for this issue. He is a great person and I am sure you will enjoy his guidance.

@moody2times
Copy link
Contributor Author

@mmaismma I would like to work on this issue. Can you please guide me on what all changes do I need to make and in which file

You are welcome to give it a go @Yashs911. I have already indicated the colors above and it seems @mmaismma approves. Just make sure to read the contributing guideline, it will show you the correct file to edit and how to do your commit. If you have any questions after that you can ask. Also, take a look at my research above and the hints @mmaismma gave and you should be ready to tackle it. All the best 😁

@SaravgiYash
Copy link

@moody2times I have made the changes in large text but for small text I didn't get the themeColor part can you please guide me
a

@moody2times
Copy link
Contributor Author

moody2times commented Oct 19, 2020

Ok, good job. For the small text, we are working with css variables. The color for the small text are held in a variable named --themeColor. If you look at the css file, you would see that the elements that we are targeting have this property color or background set to var(--themeColor).

Just as you changed the large text by targeting the #preview-text selector in the css file, there is a html selector in the css file too that has the variable --themeColor. So, go and change the value of that variable from #037de1 to the #0576DF. That is all there is to the task, 😀 that is the magic of css variables, it makes refactoring easy.

Although, I would like to hear how you solved the problem for the remove_red_eye element, since it has no selector in the css file. Perhaps, you should ask @mmaismma whether he would prefer inline styling for remove_red_eye element since it is just one property.

OK, I hope this was helpful? Let me know if there are any further questions. Looking forward to your solution. All the best.

@SaravgiYash
Copy link

SaravgiYash commented Oct 19, 2020

@moody2times For large text I changed the color in #preview-text and it changed both red eye and preview color

You are right. It was my mistake. Both elements can be targeted with the same #preview-text selector. Good job.

@srishtij2000
Copy link

Can I work on this?

@mmaismma
Copy link
Member

@srishtij2000 Why not? Everybody is welcome. Check our other issues too.

@srishtij2000
Copy link

So are you going to assign this issue or should si just submit the PR?

@SaravgiYash
Copy link

@srishtij2000 I am working on this issue.

@mmaismma
Copy link
Member

@srishtij2000 Since Yash is already working on this issue, I would recommend you to work on another issue, preferably #23 or #42.

@srishtij2000
Copy link

Okay okay

@SaravgiYash SaravgiYash mentioned this issue Oct 20, 2020
mmaismma pushed a commit that referenced this issue Oct 20, 2020
* 🆕Added info for submitting code in plain JS only

* Changed Color to increase accessibility (fixes #39)
@mmaismma mmaismma linked a pull request Oct 20, 2020 that will close this issue
moody2times pushed a commit to moody2times/Thumbnail-Maker that referenced this issue Oct 20, 2020
* 🆕Added info for submitting code in plain JS only

* Changed Color to increase accessibility (fixes Hermit-Tools#39)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working good first issue Good for newcomers
Projects
None yet
4 participants