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

Legibility / contrast warning is displayed in wrong situations #14687

Closed
draganescu opened this issue Mar 28, 2019 · 8 comments
Closed

Legibility / contrast warning is displayed in wrong situations #14687

draganescu opened this issue Mar 28, 2019 · 8 comments
Labels
[Focus] Accessibility (a11y) Changes that impact accessibility and need corresponding review (e.g. markup changes). Good First Issue An issue that's suitable for someone looking to contribute for the first time [Type] Bug An existing feature does not function as intended [Type] Regression Related to a regression in the latest release

Comments

@draganescu
Copy link
Contributor

Describe the bug
When a Paragraph block is added and from colour settings we pick a dark colour, we can see the contrast warning even if the contrast is fine. If we also pick a light text colour the warning disappears.

At the same time, when a Paragraph block is added and from colour settings we pick a light text colour the poor contrast warning doesn't show unless we also pick a light bg colour.

The problem in sum seems to be that for the contrast warning to work properly both the bg and the text colours must be picked.

To Reproduce
Steps to reproduce the behavior:

  1. Make a post
  2. Add a Paragraph block
  3. Paste some text inside
  4. Pick a dark blue Background color
  5. Notice the contrast warning.

Expected behavior
The contrast warning work as the eye does, when we see light text on light background, not depending on what properties the block has selected.

Screenshots
If applicable, add screenshots to help explain your problem.

Desktop (please complete the following information):

  • OS: OSX
  • Browser Chrome
  • Version latest

Additional context

  • This has been tested on the current Gutenberg master, and discovered by @afercia
@gziolo gziolo added [Type] Bug An existing feature does not function as intended [Type] Regression Related to a regression in the latest release [Focus] Accessibility (a11y) Changes that impact accessibility and need corresponding review (e.g. markup changes). User Experience (UX) Good First Issue An issue that's suitable for someone looking to contribute for the first time labels Mar 28, 2019
@gziolo
Copy link
Member

gziolo commented Mar 28, 2019

Discussed on WordPress.org Slack (requires registration) as well:
https://wordpress.slack.com/archives/C02QB2JS7/p1553781179021700

@Jackie6
Copy link
Contributor

Jackie6 commented Mar 29, 2019

Hi @draganescu I do not understand why we need the following lines of code

[ backgroundColor.class ]: backgroundColor.class,
[ textColor.class ]: textColor.class,
[ fontSize.class ]: fontSize.class,

Specifically, if we have these lines, when we change the background color of the block to dark blue (secondary color), the text color will be automatically changed to white, I do not understand why we should change the text color.
qkoUUNG7Tj

@draganescu
Copy link
Contributor Author

@Jackie6 It is a nice feature that helps with making sure we have default accessibility ON, that is it prevents mistakes of default black text on dark blue background.

@Jackie6
Copy link
Contributor

Jackie6 commented Mar 29, 2019

@draganescu I agree it is a nice feature. But I do not find any documentation about this feature. Do you know it? Or maybe we could write a doc for this feature. Because I think this design especially these lines of code is a little confusing at first glance.

@karmatosed
Copy link
Member

I am removing the 'User Experience (UX)' label as part of a label cleanup. It's not being used anymore consistently so let's try and keep to 'needs design' and 'needs design feedback'. If we find a need for another label we can consider it but having those 2 should cover this.

@mapk
Copy link
Contributor

mapk commented Oct 22, 2019

After reviewing this issue in the design slack triage today, I'm noticing a number of bugs. I'll add some screenshots to help visualize the problems.

  1. The contrast warning msg is incorrect. Making the text darker will not fix this issue.

Screen Shot 2019-10-22 at 9 46 12 AM

  1. There is no contrast warning when I have light text on a light background.

Screen Shot 2019-10-22 at 9 48 43 AM

@mapk
Copy link
Contributor

mapk commented Oct 22, 2019

Looks to me like the logic behind when to display a contrast warning needs some help. And some of the wording in the messages needs refining. Do we have a list of the messages anywhere so I can review them all, @draganescu ?

@mapk mapk removed the Needs Design Feedback Needs general design feedback. label Oct 22, 2019
@skorasaurus skorasaurus removed the [Status] In Progress Tracking issues with work in progress label Feb 9, 2021
@draganescu
Copy link
Contributor Author

Tested again and with the new color controls on paragraphs and other blocks (also tested the cover block) this does not seem to be a problem anymore.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Focus] Accessibility (a11y) Changes that impact accessibility and need corresponding review (e.g. markup changes). Good First Issue An issue that's suitable for someone looking to contribute for the first time [Type] Bug An existing feature does not function as intended [Type] Regression Related to a regression in the latest release
Projects
None yet
6 participants