instantsearch icon indicating copy to clipboard operation
instantsearch copied to clipboard

Published 20 hours ago • verified publisher icon algolia

Render of refinementList creates accessibility challenges

Open bobprokop opened this issue 4 years ago • 1 comments

🐛 When refinementList widgets are rendered, the innerHTML of some elements are updated. These DOM changes cause loss of focus from the currently active element. Natural tabbing order is lost, creating accessibility issues for keyboard users.

🔍 Bug reproduction

Steps to reproduce the behavior:

  1. Go to https://community.algolia.com/instantsearch.js/v2/widgets/refinementList.html#example
  2. Click on Any checkbox option -- try "Apple" -- which has 442 results associated with it in the example, making it the 5th item in the list.
  3. The "Apple" checkbox is now checked. It should have received (and retained) the focus event.
  4. Using your keyboard, try tabbing to the next option. Since the refinementList widget re-sorts the list (moving your chosen option to the top), tab should move you to the next checkbox -- in this case "Insignia".
  5. When pressing tab, focus event now moves to the option that originally followed your chosen option in the list. In this case, that is "GE", with 394 results.
  6. Whenever you act on an option (checkbox) in the refinementList widget, the focus event moves to the <body> tag due to how the DOM is re-shuffled upon each render. Try opening your console and then checking an option in the refinementList widget. Now check document.activeElement in your console -- you will see that the <body> tag now has received focus due to innerHTML changes.

Live reproduction:

https://codesandbox.io/s/github/algolia/create-instantsearch-app/tree/templates/instantsearch.js

💭 Expected behavior

Natural tabbing order should be respected and preserved whenever refinementList widget is rendered.

🖥 Screenshots

Environment

  • OS: All
  • Browser: All
  • Version: n/a

Additional context

There is a discussion of the issue here: https://discourse.algolia.com/t/prevent-sort-of-options-in-refinementlist/10267. Retaining native tab order by diffing DOM elements using a method such as Preact will work (see example here: https://codesandbox.io/s/reverent-shaw-7tyve?file=/src/app.js) -- but since natural tab order is such an important core accessibility issue (see: https://www.w3.org/TR/2008/REC-WCAG20-20081211/#navigation-mechanisms-focus-order) this behavior should be available OOB without the need to implement a DOM diffing solution such as Preact, React, Vue, etc.

bobprokop avatar Aug 31 '20 14:08 bobprokop

Thank you for opening this issue — this is a problem that I'd really like to solve but that is not trivial because of our internal templating system.

I explained why is happens in a Discourse thread but let me expand and give more context here for all contributors.

InstantSearch rendering system

InstantSearch provides multiple layers to allow users to build search UIs in a basic way, or in a advanced way.

  • Connectors: the search primitives that exposes the Algolia API values (hits, query, etc.)
  • Widgets: the implementation of a connector, they render components.
  • Components: Preact UI components that render what widgets forward them. In the InstantSearch codebase, we don't manipulate the DOM directly but leverage Preact to ease DX and to render whenever the state changes.
  • Templates: features that allow users to inject their own markup in our components.

Things go wrong at this last layer: templates. Let's take the refinementList widget as an example.

Why does this happen?

When we render the RefinementList component, we retrieve the item template that the user provided. This template is a string that is injected in the page using 'Preact's dangerouslySetInnerHTML.

dangerouslySetInnerHTML acts as a hard DOM reset and browsers are not able to keep track of which DOM elements has become which one, which results in focus lost, and therefore accessibility issues. This is why frameworks like React, Vue, etc. do some DOM diff computation to reduce DOM changes and have the concept of keys. They don't reset and recompute the whole DOM, they tell browser what are the minimal changes required to re-render the UI, which allow browsers to keep track of DOM elements changes.

User-land fix

Right now, the only way to prevent this accessibility issue user-land is to use connectRefinementList and a UI library that handles UI diffing, like this Preact example.

Internal fix

Fixing this behavior internally is tricky because we need to rework our entire templating system.

We still have the possibility to use Preact components as default templates, and to switch to string user-defined templates when provided. This means that the default implementation of refinementList will be accessible, but as soon as users modify the rendering, they will get back to the current behavior, unable to keep focus between renders.

francoischalifour avatar Sep 02 '20 08:09 francoischalifour

This should be working as expected starting from [email protected] which deprecated string-based templates and introduced tagged templates with htm.

dhayab avatar Dec 19 '22 17:12 dhayab