wix-incubator/wix-ui-tpa

<Counter/> - a11y issues

Closed this issue · 7 comments

We think the plus/minus buttons shouldn't be accessible via sr/tabs because the input should be enough.
Also, it doesn’t make sense that the minus button would be the first focus.
In our current product page, we have a similar counter and we decided to add aria-hidden={true} to the plus/minus buttons. In our case the buttons are visible only on hover so we didn't have to add tabIndex=-1 but i think we should in this case.
https://github.com/wix-private/ecom/blob/master/client/wixstores-client/wixstores-client-product-page/src/components/ProductOptions/NumberInputSpinner/NumberInputSpinner.tsx#L104-L125

@NeilWix
should we make the buttons non-navigational?

I think sighted keyboard users might find it awkward if the increment & decrement were not focusable, which brings me to the aria-hidden remark. Setting aria-hidden as true on interactive elements such as buttons is a big nono! Doing so would break UX for screen reader users as they would focus a link/button/etc and would only hear: link or button which is meaningless without a label (should be "add to car, button" or "sign in, link".
So, whenever you wish to hide something from screen reader users you should also hide it from the tab-order with tabindex="-1". This obviously come at the expanse of sighted keyboard users... so there is a tradeoff here.

As to the order of the elements, indeed it doesn't make sense, from a blind user point of view, to start with the decrementation option. This consideration should not be resolved at the expanse of sighted users who may find the current design instructive/intuitive. So to serve both audiences we can keep the current presentation order but implement it in a different DOM order via flex or grid.

@ronnyrin @NeilWix @alexkronenberg
We decided on going with the following solution:
Changing the DOM order, so that the incrementation will be first, followed by the input, and then decrementation.
In addition, we'll add a role="region" to the wrapping div, and allow the TPA to pass a region aria-label.
What do you think Neil?

@jonathanadler
That's great. Would it also be possible to allow TPA's to also set (alternatively to aria-label) an aria-labelledby property? The rational is the case where there is already a labeling element entered for sighted users...

@jonathanadler
That's great. Would it also be possible to allow TPA's to also set (alternatively to aria-label) an aria-labelledby property? The rational is the case where there is already a labeling element entered for sighted users...

@NeilWix yeah sure 👍

fixed by #262