Inputs do not conform to accessibility standards

[brettinternet]

Describe the bug 🐛

The default inputs in components such as SearchBox and SortBy do not allow an aria-label property. Default values or this prop interface should be available by default without having to use connectSearchBox.

Not only does this hurt a site's Lighthouse score, but most importantly it degrades the experience of our friends who rely on accessibility attributes in HTML.

To Reproduce 🔍

See Algolia's CodeSandbox demo. Examine the inputs with your browser's dev tools.

I've also tried passing the aria-label as a prop to the component but it looks like it's ignored. No documentation indicates that this prop is featured. An id prop also doesn't pass through to the input to execute a different accessibility method.

Expected behavior 💭

It looks like an aria-label was added back in 2017 according to https://github.com/algolia/react-instantsearch/issues/43, but I haven't looked in the commit history or git blame.

Additional context

See w3.org web accessibility tutorial for more information about the aria-label approach.

While an answer to this bug report may just be to use connectSearchBox, I don't think it's the right answer. There are even Algolia sites that don't comply with basic input accessibility standards (1 2). Setting generic search defaults or making this prop more readily available on components such as SearchBox will help spread accessibility usage.

Let's make accessibility a priority on the web. Thank you for maintaining a great project!

[Haroenv]

Hey, this is a good point, and it was on my personal todo-list to revisit this. Originally we didn't go with an aria-label, because it duplicated the input from the placeholder. On revisiting that, you likely don't want the same aria-label as placeholder.

What we definitely want to avoid is having a label which harms users.

Research:

Google has aria-label="Search" Twitter has aria-label="Search query" Stack overflow has aria-label="Search"

Thanks a lot, let's get this not only done on React InstantSearch, but on all versions

[brettinternet]

@Haroenv Thanks for the reply! Let me know how I can help out. I'd be happy to perform a PR once the team has some direction on this.

[Haroenv]

I guess aria-label being set by default to "Search" and "Sort By", which could be overridden via translations in React InstantSearch is a good way to start. Are there other places you see that could be helped with this? I'd love to think about a11y in a more broad term, since it doesn't really seem like the InstantSearch pattern is covered in existing WAI patterns.

A PR would definitely be welcome!

[websocket98765]

@Haroenv Best way to see where labels are needed is to load a page with many react-instantsearch components and run Lighthouse browser extension because it will generate a list of all components missing a label.

Personally, I'd be happy if Algolia components allowed passing of an aria-label property, b/c then I can make it work in most cases. (There are other ways though, such as allowing an id or name property to be passed into the react-instantsearch element so that a label's for attribute can point to it. But passing aria-label is more flexible and is probably sufficient for most cases. It's a good start either way.)

I made a feature request to allow passing of aria-label for A11y in the Algolia Community in the past and received the standard "build a custom component for that" response :/ Clearly much more effective to use Github than the Community.

(Something similar while you're at it, please consider passing title HTML attribute as well to allow inputs--such as ToggleRefinement--that may be labeled with an acronym, to show the full length text via a title tag, when desired.)

[websocket98765]

Also, ideally it should work to pass aria-label to custom components built using connectors too.

[brettinternet]

I agree that the translations object is a great candidate for this sort of thing, and including an 'aria-label' field for components that don't at least have a label would be a great starting point.

@websocket98765 Could you describe the bit about ToggleRefinement? It looks like the toggle passes Lighthouse in the component example, because there's a label for the input.

[websocket98765]

@brettinternet My mention of ToggleRefinement isn't related to aria-label, but to HTML title attribute. Just figure if @Haroenv is looking at how to pass/set aria-label, it'd be a timesaver to also have in mind that passing the HTML title attribute is useful for some Algolia components like ToggleRefinement too (for non-A11y-related reasons, like my earlier example), since these would be achieved in a similar way.

But you bring up a good point. I think it's still useful to pass aria-label for ToggleRefinement because it allows overriding the element's label when read by screen readers. E.g. For sighted people, the label might be an acronym GPS and title global positioning system in case user needs a longer description. For screenreaders, aria-label could be set to global positioning system to override GPS as what is read. (GPS isn't a good example b/c it's a common acronym. But useful for more obtuse acronyms.)

TLDR: I believe passing aria-label is useful for most, maybe all, elements.

[websocket98765]

In addition to aria-label, something to keep in mind is it may be useful to pass id and aria-labelledby so that existing labels on the page, that may be outside the Algolia component, can be associated with it. If such a label exists, it's preferrable to associate it with the input/select/etc, rather than also defining aria-label and having two labels. https://developer.mozilla.org/en-US/docs/Web/Accessibility/ARIA/ARIA_Techniques/Using_the_aria-labelledby_attribute

But aria-label is a good start. This is a further improvement.

[Haroenv]

Also, ideally it should work to pass aria-label to custom components built using connectors too.

I’d love this, but don’t practically know how to implement it. Are you thinking of something like Downshift’s ...inputProps? @websocket98765

[websocket98765]

I'm not familiar with Downshift's ...inputProps, but based on the spread syntax, yeah that looks like it would work well if it's doing what I assume, which is passing all props through to the child. That would very flexible, if that's possible in this case.

[Haroenv]

Note that with the connectors, we don't render any dom, it's all done by users, so they can easily spread the props themselves if that's something required. We don't do that in the native components, since most of them have multiple dom elements inside, so it isn't obvious to where things should be spread.