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

Update save indicator contrast to pass AA #15514

Merged
merged 1 commit into from May 9, 2019

Conversation

@kjellr
Copy link
Contributor

commented May 8, 2019

Fixes one portion of the issues noted in #15280.

This PR changes the "✔️ Saved" toolbar message from #a2aab2 to #555d66 in order to pass WCAG AA contrast ratios. It looks like there was a note in the code that this doesn't need to pass:

// Doesn't need to meet AA because button is disabled and it's supporting text.

This change appears to come from #10552, but I don't see any discussion there around it. Personally, I don't particularly mind the darker shade, but I'm happy to keep it light if there's good reason to.


Before:

Screen Shot 2019-05-08 at 3 07 35 PM

After:

Screen Shot 2019-05-08 at 3 07 18 PM

@@ -3,7 +3,7 @@
align-items: center;
width: $icon-button-size - 8px;
padding: #{ $grid-size-small * 3 } $grid-size-small;
color: $light-gray-900; // Doesn't need to meet AA because button is disabled and it's supporting text.
color: $dark-gray-500;

This comment has been minimized.

Copy link
@jasmussen

jasmussen May 9, 2019

Contributor

You could use $dark-gray-300 if you like. That also meets AA contrast.

This comment has been minimized.

Copy link
@kjellr

kjellr May 9, 2019

Author Contributor

Thanks — I just tried this out, but I think we should stick with $dark-gray-500 for now. $dark-gray-300 adds another gray shade to the toolbar, and I think the hierarchy is more straightforward if we keep the color palette limited there.

@jasmussen jasmussen self-requested a review May 9, 2019

@jasmussen
Copy link
Contributor

left a comment

Left a comment with an additional option for the color. But looks good 👍 👍

@afercia

This comment has been minimized.

Copy link
Contributor

commented May 9, 2019

It looks like there was a note in the code that this doesn't need to pass:

This is because, if I remember correctly, "Saved" used to be a <button>. Disabled buttons don't need to pass color contrast, see https://www.w3.org/TR/WCAG21/#contrast-minimum

Text or images of text that are part of an inactive user interface component ... have no contrast requirement.

This is now a <span>, thus its content needs to pass contrast for text.

However, dynamically changing the <button> to a <span> creates other problems. Not sure why it was changed to a <span>.

  • When using the keyboard and pressing "Save Draft": there's a focus loss. The previously focused element gets removed from the DOM so focus is lost.
  • Nothing is announced: when using a screen reader and pressing "Save Draft", there's no feedback at all.

Overall, the "Save Draft" / "Saved" combo needs some rethinking. Of course, I'm not opposed to improve color contrast 🙂 but the "Save Draft" button shouldn't disappear to start with. Will split in a new issue.

@afercia

afercia approved these changes May 9, 2019

@kjellr

This comment has been minimized.

Copy link
Contributor Author

commented May 9, 2019

Thanks, everyone!

@kjellr kjellr merged commit d17e867 into master May 9, 2019

1 check passed

Travis CI - Pull Request Build Passed
Details

@kjellr kjellr deleted the update/save-indicator-contrast branch May 9, 2019

@youknowriad youknowriad added this to the 5.7 (Gutenberg) milestone May 10, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.