useListNavigation moves focus into list unexpectedly when an item is dynamically removed from the list

[mihkeleidast]

Describe the bug

I'm building a non-floating Listbox component with useListNavigation and useTypeahead hooks. One possibility there is that items can be added/removed dynamically. However, if that is done, useListNavigation sometimes moves focus unexpectedly.

To Reproduce

https://codesandbox.io/p/sandbox/floating-ui-react-listbox-with-composable-children-g524xl?file=%2Fsrc%2FApp.tsx%3A89%2C40

Since the focus issue is most problematic with keyboard navigation, it is recommended to use keyboard nav and selection for the steps.

1. Move into listbox, select third option ("Watermelon")
2. Move out of listbox, to "toggle item" button
3. Toggle the item on, then off
4. See that focus is moved to "Watermelon" option unexpectedly.

Expected behavior

Focus should stay on the button.

Context:

• OS: Windows
• Browser: Edge
• Version: 124

Additional context

I think useListNavigation currenly attempts to focus the selectedIndex item if it changes, but it does not check whether the focus is inside the list.

There is also another issue visible there, that when dynamically adding an item to the selected index position. I would expect that the selected item remains the same, whereas right now it changes to the dynamic item. But this may be a misconfiguration issue and can be worked around, as it does not happen in my own codebase.

[atomiks]

I can't seem to reproduce from the steps provided - focus stays on the button.

[mihkeleidast]

Hmm, here's a video, definitely happens for me:

screen-capture.webm

[mihkeleidast]

From tracing the focus, it seems to come from here: https://github.com/floating-ui/floating-ui/blob/master/packages/react/src/hooks/useListNavigation.ts#L457

Which makes sense, as that effect also runs on selectedIndex change, which happens if the toggled item is before the selected item.

[mihkeleidast]

How about this for a repro, it's a bit less realistic but should show the same issue: https://codesandbox.io/p/sandbox/floating-ui-react-listbox-with-composable-children-forked-p32x5d

Prerequisite is that activeIndex is not null, this essentially happens whenever you've focused into the listbox at least once, but for demo purposes I set the initial value as 1.

1. Focus button with keyboard
2. Press button
3. Focus moves to "Blueberry" option, but it should not - this is essentially "controlled" selectedIndex behavior, should select option without moving focus.

[atomiks]

That one does reproduce by moving focus into the listbox, not sure why the original demo doesn't

[atomiks]

Okay so I manged to reproduce the original demo now.

From what I can see, this is user error because you're reading a ref in render which is forbidden by React. If you refactor selectedLabel to selectedIndex so that a ref isn't being read in render, the problem doesn't occur.

https://codesandbox.io/p/sandbox/floating-ui-react-listbox-with-composable-children-forked-5wtms5

[mihkeleidast]

@atomiks fair point about my original repro, but my second repro with controlling the selectedIndex value (i.e. https://codesandbox.io/p/sandbox/floating-ui-react-listbox-with-composable-children-forked-p32x5d) does not do that, unless I'm missing something? Do you think it's reasonable to move focus into the listbox when selectedIndex changes by some means (in a real world usecase this may just be data updating in a polling fetch, or something like that).

[atomiks]

Oh good point, didn't verify the second one too. From what I can see, simply making selectedIndex non-reactive fixes the issue. I'll make a PR in a sec