Combobox items get wrong keyboard navigation order after being reordered

[avinashbot]

🐛 Bug report

Current Behavior

When combobox list items are reordered, their keyboard navigation order doesn't seem to change. So if you use arrow keys to traverse through the list after a reorder, it jumps around the list instead of to the next item.

Expected behavior

When I press down, only the next immediate option should be selected.

Reproducible example

Example of a list of countries that are re-ordered as you type:
https://codesandbox.io/s/beautiful-panka-8ow2p?file=/src/App.js

1. Type in any character. The matches are reordered so that countries that start with the letter are presented first.
2. Now press down to traverse the list. As you traverse, you occasionally start jumping to random elements in the list.

Suggested solution(s)

I'm not sure why it's happening, but I'm guessing it has something to do with @reach/descendants. It seems like it's the same problem as in #827, but the fix in the CR doesn't help in my case.

Additional context

n/a

Your environment

Software	Name(s)	Version
Reach Package	@reach/combobox	^0.16.1
React		^17.0.2
Browser	Firefox, Chrome	92.0, 93.0.4577.82
Assistive tech	None
Node		16.6.2
npm/yarn	npm	7.20.3
Operating System	Ubuntu	21.04

[phacks]

Hey! Jumping in because I’ve had a similar issue, and found an odd fix to it. The following change:

<ComboboxList>
  {matches.map((match) => (
+    <div>
        <ComboboxOption key={match} value={match} />
+    </div>
  ))}
</ComboboxList>

appears to fix the keyboard navigation issues you see in the sandbox. I have no idea why that is, or whether the extra div is having a negative impact on accessibility, or if something other than a div would also fix this—but anyway, thought I’d share this here in the hopes it helps find a proper fix.

[avinashbot]

Hi! I also found an alternative fix to this last night and forgot to mention it here. Thanks for the reminder! Mine looks like this:

<ComboboxList>
  {options.map((option, i) => (
    <ComboboxOption key={`${i}-${option.value}`} value={option.value} />
  ))}
</ComboboxList>

The trick is the use of the array index in the key. I think both of our hacks work by forcing the library to treat the child as a new element each time. I combine the value with an index to make sure every reordering is treated by React as a new element, and you wrap your option with a key-less div to do the same.

In my case, eslint-plugin-react doesn't let you get away with not putting a key in an iterated value, which is why I went with a new key each time.

[lukasvice]

Hi, I got the same problem. I solved it by explicitly setting the internal index:

<ComboboxList>
  {options.map((option, i) => (
    <ComboboxOption index={i} key={option.value} value={option.value} />
  ))}
</ComboboxList>

[jordan-paz]

Hi, I got the same problem. I solved it by explicitly setting the internal index:

<ComboboxList>
  {options.map((option, i) => (
    <ComboboxOption index={i} key={option.value} value={option.value} />
  ))}
</ComboboxList>

This worked for me, thank you!

[tometo-dev]

Hi, I got the same problem. I solved it by explicitly setting the internal index:

<ComboboxList>
  {options.map((option, i) => (
    <ComboboxOption index={i} key={option.value} value={option.value} />
  ))}
</ComboboxList>

Thank you @lukasvice. This worked for me.