Skip to content
This repository has been archived by the owner on Jun 20, 2022. It is now read-only.

Accessibility issues #149

Open
darekkay opened this issue Aug 28, 2019 · 9 comments
Open

Accessibility issues #149

darekkay opened this issue Aug 28, 2019 · 9 comments

Comments

@darekkay
Copy link

🐛 Bug Report

The framework advertises "Accessibility" and WAI-ARIA compliance. While in general it looks quite good, I have found multiple a11y issues.

  • While the focus ring looks great for input components, it's completely missing for buttons. Keyboard users won't know which element is currently focused.
  • Some Button colors do not pass the contrast-ratio as per WCAG (e.g. the green Success button)
  • Form inputs shouldn't use both aria-labelledby and aria-describedby if the content is equal. It will be announced twice via the screen reader.
@gregberge
Copy link
Member

Hello @darekkay, thanks for reporting it! PR are welcome to fix this issues, else we will take care of fixing them as soon as possible!

@gregberge
Copy link
Member

  • Some Button colors do not pass the contrast-ratio as per WCAG (e.g. the green Success button)

Do you have an idea to fix it?

@gregberge gregberge mentioned this issue Aug 30, 2019
@darekkay
Copy link
Author

Do you have an idea to fix it?

  1. Find all contrast ratio issues using any tool ( e.g. Chrome DevTools Audit or pa11y)
  2. Choose better values, either with https://contrast-ratio.com/ or again using the Chrome Dev Tools

@darekkay
Copy link
Author

Here's a result from web.dev (which uses Google Lighthouse)

@gregberge
Copy link
Member

Thanks, but the color generated is automatic. So I would need a function that uses the best approaching color to meet the contrast requirement.

@diegohaz
Copy link
Contributor

Not sure if it helps, but we're experimenting with automatic generation of contrast colors in Reakit.

Some usage examples can be seen here and here.

There's a lot to improve though.

@gregberge
Copy link
Member

Hello @diegohaz, thanks it is very interesting! Also the current approach need to be rethink with CSS Custom Properties. We will have to create a transformation and to pre-generate it, not at runtime.

@diegohaz
Copy link
Contributor

Babel macros may help.

@linus-amg
Copy link

linus-amg commented Oct 21, 2019

I would like to add 2 more bugs to this issue:

  • when using SSR (for example with Next.js) the smooth-ui's useRandomId method is getting in the way of proper react component rehydration as soon as the developer starts implementing a form, since label and input ids get generated randomly, they differ on the serverside rendered and client side rendered component, so every component using a form needs to get re-rendered on the client, which is not beneficial when using SSR. (this is happening because unlinke babel-plugin-styled-components, smooth-ui does not generate those ids in some incremental fashion). To fix this issue it would be nice to let the developer customize the useRandomId method, or create a plugin which does what babel-plugin-styled-components does in the ssr: true regard.

  • accessibility is broken because of former mentioned bug, the aria-labelledby value contains a different id (probably the serverside one? no idea.. actually) than the id of the label itself from the client-side, so there is no real connection made by aria-labelledby.

To be clear, those issues only happen when trying to use smooth-ui serverside and clientside at the same time, like when implementing something with Next.js, styled-components had the same issue and fixed it with the ssr: true option using the babel-plugin-styled-components.

relevant lines from babel-plugin-styled-components to fix that issue:

relevant lines from smooth-ui:

EDIT: I found that giving the <Form> element a custom id solves the problem, because then, no random id needs to get generated. 👍

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

No branches or pull requests

4 participants