smooth-ui icon indicating copy to clipboard operation
smooth-ui copied to clipboard

Published 20 hours ago • verified publisher icon smooth-code

Accessibility issues

Open darekkay opened this issue 5 years ago • 9 comments

🐛 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.

darekkay avatar Aug 28 '19 10:08 darekkay

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 avatar Aug 28 '19 10:08 gregberge

  • 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 avatar Aug 30 '19 07:08 gregberge

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 avatar Aug 30 '19 09:08 darekkay

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

darekkay avatar Aug 30 '19 09:08 darekkay

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

gregberge avatar Aug 30 '19 14:08 gregberge

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.

diegohaz avatar Sep 23 '19 22:09 diegohaz

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.

gregberge avatar Sep 24 '19 07:09 gregberge

Babel macros may help.

diegohaz avatar Sep 24 '19 16:09 diegohaz

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:

  • https://github.com/styled-components/babel-plugin-styled-components/blob/f308f5c9db323712c502e21eaae579f76f3962cf/src/visitors/displayNameAndId.js#L160
  • https://github.com/styled-components/babel-plugin-styled-components/blob/f308f5c9db323712c502e21eaae579f76f3962cf/src/visitors/displayNameAndId.js#L133
  • https://github.com/styled-components/babel-plugin-styled-components/blob/f308f5c9db323712c502e21eaae579f76f3962cf/src/visitors/displayNameAndId.js#L125
  • https://www.styled-components.com/docs/tooling#serverside-rendering

relevant lines from smooth-ui:

  • https://github.com/smooth-code/smooth-ui/blob/2a9be0f54e23bf08c0787e5ddebc195de3904ac6/packages/shared/core/Form.js#L25
  • https://github.com/smooth-code/smooth-ui/blob/48a23d3eeb5e2e890de1f1efc103a2eb0b57a5f3/packages/shared/core/util/hooks.js#L3

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

linus-amg avatar Oct 21 '19 10:10 linus-amg