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

fix: accessibility - color contrast and keyboard navigation #6757

Merged
merged 14 commits into from
May 16, 2023

Conversation

kewitham
Copy link
Contributor

@kewitham kewitham commented May 1, 2023

Summary

Our team at ada.gov uses Netlify CMS. Recently Terri Youngblood, the accessibility consultant for the DOJ Civil Rights Section, did an audit of Netlify to verify its accessibility and identified some issues. This PR addresses those concerns - specifically color contrast, and keyboard navigation updates.

Screenshot 2023-05-01 at 11 26 45 AM

Screenshot 2023-05-01 at 11 27 00 AM

Screenshot 2023-05-01 at 11 27 22 AM

Screenshot 2023-05-01 at 11 27 33 AM

Screenshot 2023-05-01 at 11 27 39 AM

Test plan

  1. Verify that the color contrast for the elements now fulfill the contrast ratio requirements:

i.e.
Before
image
After
image

Note: I used this tool to darken the colors in question until they met the minimum contrast requirements.

  1. Verify that you are able to navigate out of the editor panel by using the esc key.
  2. Verify that you are able to tab through the media asset modal.

Checklist

Please add a x inside each checkbox:

A picture of a cute animal (not mandatory but encouraged)
image

color contrast, keyboard navigation, and label updates
@netlify
Copy link

netlify bot commented May 1, 2023

Deploy Preview for decap-www ready!

Name Link
🔨 Latest commit 88f8e24
🔍 Latest deploy log https://app.netlify.com/sites/decap-www/deploys/645569e6d3e54e00089a7545
😎 Deploy Preview https://deploy-preview-6757--decap-www.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 settings.

@martinjagodic
Copy link
Member

Hi @kewitham thanks for this contribution.

This product supports many languages, so aria labels should be translated also. I also don't understand why the Russian translation was modified, perhaps it's just a merge issue?

@kewitham
Copy link
Contributor Author

kewitham commented May 3, 2023

Hi @kewitham thanks for this contribution.

This product supports many languages, so aria labels should be translated also. I also don't understand why the Russian translation was modified, perhaps it's just a merge issue?

@martinjagodic The Russian translation updates are just formatting updates. Do you have an internal resource for translating the aria labels? What are the next steps there?

@samo4
Copy link
Collaborator

samo4 commented May 4, 2023

It would be nice if this is split into multiple PRs which encompass logical areas of the changes. I don't have a firm opinion on how to structure this, but first I would recommend moving the formatting of russian translations to separate PR. Also separate PR: aria labels should be split from minor accessibility improvements. That's because currently they need to be translated.

@kewitham
Copy link
Contributor Author

kewitham commented May 5, 2023

It would be nice if this is split into multiple PRs which encompass logical areas of the changes. I don't have a firm opinion on how to structure this, but first I would recommend moving the formatting of russian translations to separate PR. Also separate PR: aria labels should be split from minor accessibility improvements. That's because currently they need to be translated.

@samo4 @martinjagodic I have removed the aria label updates and formatting updates from this PR. I will submit another one with just the aria updates

@kewitham kewitham changed the title fix: accessibility - color contrast, keyboard navigation and aria-labels fix: accessibility - color contrast and keyboard navigation May 5, 2023
@kewitham
Copy link
Contributor Author

@samo4 @martinjagodic checking back in on this PR - is it good to merge now?

@martinjagodic
Copy link
Member

@demshy since you're working on updating the markdown editor, is this PR conflicting with your efforts?

@martinjagodic martinjagodic requested a review from demshy May 16, 2023 07:52
@demshy
Copy link
Member

demshy commented May 16, 2023

Some of it for sure, because we had to basically rewrite all keyboard events. I don't mind if this is merged though.

I will make sure the escape key is handled properly in the new handlers.

@martinjagodic martinjagodic merged commit 194d1ce into decaporg:master May 16, 2023
martinjagodic pushed a commit that referenced this pull request Oct 16, 2023
* fix: accessibility

color contrast, keyboard navigation, and label updates

* Update ViewStyleControl.js

* Update EditorControl.js

* Update MediaLibraryHeader.js

* Update SettingsDropdown.js

* Update ObjectWidgetTopBar.js

* Update Toolbar.js

* Update ListControl.spec.js.snap

* Update DateTimeControl.js

* Update DateTimeControl.js

* Update DateTimeControl.js

* Update MediaLibraryCard.spec.js.snap

* Update index.js
martinjagodic pushed a commit that referenced this pull request Oct 17, 2023
* fix: accessibility

color contrast, keyboard navigation, and label updates

* Update ViewStyleControl.js

* Update EditorControl.js

* Update MediaLibraryHeader.js

* Update SettingsDropdown.js

* Update ObjectWidgetTopBar.js

* Update Toolbar.js

* Update ListControl.spec.js.snap

* Update DateTimeControl.js

* Update DateTimeControl.js

* Update DateTimeControl.js

* Update MediaLibraryCard.spec.js.snap

* Update index.js
martinjagodic pushed a commit to geotrev/netlify-cms that referenced this pull request Oct 17, 2023
…#6757)

* fix: accessibility

color contrast, keyboard navigation, and label updates

* Update ViewStyleControl.js

* Update EditorControl.js

* Update MediaLibraryHeader.js

* Update SettingsDropdown.js

* Update ObjectWidgetTopBar.js

* Update Toolbar.js

* Update ListControl.spec.js.snap

* Update DateTimeControl.js

* Update DateTimeControl.js

* Update DateTimeControl.js

* Update MediaLibraryCard.spec.js.snap

* Update index.js
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.

4 participants