[Autocomplete] for Voiceover on Mac is reading incorrect labels on any browser

[inform880]

• The issue is present in the latest release.
• I have searched the issues of this repository and believe that this is not a duplicate.

Current Behavior 😯

Voiceover for Mac is reading the values incorrectly on the component, after typing characters and triggering the filter. It seems to always read off the same values for after you type in more than one character. After typing in two ore more characters, it sticks with the read labels from typing in one character.

Expected Behavior 🤔

The values should be read correctly, even after typing in multiple characters.

Steps to Reproduce 🕹

Steps:

1. Use the following example codesandbox: https://codesandbox.io/s/material-ui-issue-with-voiceover-autocomplete-91se7
2. Enable voiceover, and type in the following: "the dark".
3. Use the arrow keys to move the cursor down onto the suggestions.
4. Observe that voiceover is reading it as different values than what is selected.

Context 🔦

I am trying to accomplish a completely a11y website for a client.

Your Environment 🌎

`npx @material-ui/envinfo`

    System:
    OS: macOS 10.15.7
  Binaries:
    Node: 11.10.1 - ~/.nvm/versions/node/v11.10.1/bin/node
    Yarn: Not Found
    npm: 6.14.8 - ~/.nvm/versions/node/v11.10.1/bin/npm
  Browsers:
    Chrome: 87.0.4280.88
    Edge: Not Found
    Firefox: 81.0
    Safari: 14.0
  npmPackages:
    @emotion/styled:  10.0.27
    @material-ui/core: ^4.11.0 => 4.11.0
    @material-ui/icons: ^4.9.1 => 4.9.1
    @material-ui/lab: 4.0.0-alpha.56 => 4.0.0-alpha.56
    @material-ui/styles:  4.10.0
    @material-ui/system:  4.9.14
    @material-ui/types:  5.1.0
    @material-ui/utils:  4.10.2
    @types/react: ^16.9.43 => 16.9.56
    react: ^16.13.1 => 16.14.0
    react-dom: ^16.13.1 => 16.14.0
    styled-components: ^5.2.1 => 5.2.1
    typescript: ^3.9.6 => 3.9.7

[oliviertassinari]

@inform880 I can reproduce it too, since the first version. Wow, I can't believe it wasn't reported before. How is the component even usable with this behavior? 🙈 From what I understand, the bug has been here for 1 year and unnoticed…

It looks like VoiceOver caches the results per DOM node. What do you think about the following fix?

diff --git a/packages/material-ui/src/useAutocomplete/useAutocomplete.js b/packages/material-ui/src/useAutocomplete/useAutocomplete.js
index a47295e97d..4510f826a4 100644
--- a/packages/material-ui/src/useAutocomplete/useAutocomplete.js
+++ b/packages/material-ui/src/useAutocomplete/useAutocomplete.js
@@ -1005,7 +1005,7 @@ export default function useAutocomplete(props) {
       const disabled = getOptionDisabled ? getOptionDisabled(option) : false;

       return {
-        key: index,
+        key: `${getOptionLabel(option)}-${index}`,
         tabIndex: -1,
         role: 'option',
         id: `${id}-option-${index}`,

Do you want to work on a pull request? :)

[eps1lon]

Don't we want to remove the index entirely from the key in case these are re-ordered?

[oliviertassinari]

@eps1lon From what I understand, having the index in the key means that we are recreating options potentially more often than we might need to. In exchange, we hedge against two options that have the same label but are visually rendered differently. So in the case you are considering, it seems that the worse case is about wasted DOM nodes, but the a11y feature remains intact.

[eps1lon]

In exchange, we hedge against two options that have the same label but are visually rendered differently.

Why would we do that? It's a mistake.

[oliviertassinari]

Why would we do that?

@eps1lon It allows the component to be more versatile. From what I understand it doesn't matter if DOM nodes aren't shared as much as they could have been, at least, we can benchmark the difference once somebody takes on this effort. My assumption is that it's negligible, not worse taking into account.

I think that the best-case scenario would be for us to have a value we can leverage, in which case I think that we can assume uniqueness, maybe it will come with #23708. It's how Downshift solves the problem.

Alternatively, we can try to be greedy. We can try to create a new constrain by asking for the label to be unique. Then we can wait and see if it "fly". I have no idea if it will.

diff --git a/packages/material-ui/src/useAutocomplete/useAutocomplete.js b/packages/material-ui/src/useAutocomplete/useAutocomplete.js
index a47295e97d..4510f826a4 100644
--- a/packages/material-ui/src/useAutocomplete/useAutocomplete.js
+++ b/packages/material-ui/src/useAutocomplete/useAutocomplete.js
@@ -1005,7 +1005,7 @@ export default function useAutocomplete(props) {
       const disabled = getOptionDisabled ? getOptionDisabled(option) : false;

       return {
-        key: index,
+        key: getOptionLabel(option),
         tabIndex: -1,
         role: 'option',
         id: `${id}-option-${index}`,

[inform880]

I have a PR up with the most recent suggested fix, and it seems to work. This is my first time contributing to such a big project, any feedback is appreciated.

#24213

[eps1lon]

@eps1lon It allows the component to be more versatile.

What does this mean?

We can try to create a new constrain by asking for the label to be unique.

Why would you render option with the same label? How would end-users be able to decide which option to choose from?

[oliviertassinari]

What does this mean?

@eps1lon It means that, right now, having unique labels isn't a constraint the Autocomplete enforces. The component works (technically, I'm not judging the UX) with duplicates. It's possible that some applications have options with identical labels.

How would end-users be able to decide which option to choose from?

Developers can customize how each option is rendered. The notion of "label" was introduced to render a value inside the textbox but it can be bypassed.

How would end-users be able to decide which option to choose from?

I agree, adding this new constraint has the potential to yield a better UX. It's why I called this approach "greedy", which doesn't have a negative connotation, on the contrary, we can try it out, learn, see if we get pushback from developers. Hopefully, it will fly.

[eps1lon]

[...]I'm not judging the UX [...]

Why not?

It's possible that some applications have options with identical labels.

There are infitenitely many possible applications so anything is possible. That isn't an argument for anything.

Developers can customize how each option is rendered. The notion of "label" was introduced to render a value inside the textbox but it can be bypassed.

Should it be bypassed?

I agree, adding this new constraint has the potential to yield a better UX. It's why I called this approach "greedy"

A "greedy" approach is one that yields a better UX? Why do you add this qualifier to your statement when that is literally the whole point of a component library. Why would you use a component library that does not yield a better UX?

[oliviertassinari]

Why not?

I think that these two concerns (DX and UX) can be discussed on their own even if interdependent. Improving the two can sometimes be at odds, tradeoffs need to be made.

Should it be bypassed?

In the nominal use case, it seems that we both agree that the answer is no.
In the advanced use cases, I personally don't know the full scope of the valid use cases. Considering one of the biggest pain developers have with the library is customizability (which isn't only about the CSS), I think being able to do such isn't without merit.

Why do you add this qualifier to your statement?

The meaning for the adjective I had in mind is "always wanting more". I think that it's healthy for an API to try to impose as many constraints as it can possibly get away with. The more useful constraints an API has, the more valuable properties it can yield in exchange.

Why would you use a component library that does not yield a better UX?

The primary motivation for using a component library is a. saving developers time b. filling the gap of missing design skills (UI & UX) among developers. At least, it's what I think we can conclude from our experience so far. I think that having a pit-of-success for the API to yield a better UX is inside value proposition b.
I also think that at the end of the day, it's all about the developers, they decide what solution to use.