Unpredictable ListBox behavior when used in Dialog

[mattrothenberg]

📝 Feedback

Original Twitter thread with @devongovett – https://twitter.com/mattrothenberg/status/1681749782740496384?s=20

I'd like to build a custom Select menu that supports multiple selection. To this end, I've sketched out some code that looks like so. https://codesandbox.io/s/rac-multiselect-yptnvy?file=/src/App.js:324-1192

export default function App() {
  let [selectedKeys, setSelectedKeys] = useState(new Set());
  let selectedItems = [...selectedKeys].map((k) =>
    items.find((item) => item.id === k)
  );

  return (
    <div className="App">
      <DialogTrigger>
        <Button>
          {selectedItems.length
            ? selectedItems.map((i) => i.name).join(", ")
            : "No selected items"}
        </Button>
        <Popover>
          <Dialog>
            <ListBox
              aria-label="Favorite animal"
              selectionMode="multiple"
              autoFocus="first"
              items={items}
              selectedKeys={selectedKeys}
              onSelectionChange={setSelectedKeys}
            >
              {(item) => <Item>{item.name}</Item>}
            </ListBox>
          </Dialog>
        </Popover>
      </DialogTrigger>
    </div>
  );
}

This almost works perfectly. There are two UX issues, however, that prevent me from being to able to add this functionality to my component library.

1. When the dialog is open and the list of options is visible, hitting Esc clears the selected value. While this is expected behavior for a standalone ListBox, this is unexpected behavior in this context. I would expect hitting Esc would close the dialog and maintain my selection, rather than clobber it.
2. The autofocus prop can be used to ensure that, the first time you open the dialog, a ListBox item is selected. However, if you open the dialog, select an item, click outside of it to close the dialog, and then re-open, you'll note that autofocus is totally ignored. This feels like a bug, but I'm reticent to make that determination myself.

🌍 Your Environment

Software	Version(s)
[react-aria-components]	1.0.0-alpha.5
Chrome

[LFDanLu]

When the dialog is open and the list of options is visible, hitting Esc clears the selected value. While this is expected behavior for a standalone ListBox, this is unexpected behavior in this context. I would expect hitting Esc would close the dialog and maintain my selection, rather than clobber it.

That makes sense, we've had prior discussions about this here and have implemented that behavior for Menu. For your use case, would a Menu make more sense instead of a ListBox? If not, we would need some way to distinguish when to prevent Esc from clearing the ListBox's selection and close the Dialog since I don't believe we'd want that to happen all the time.

The autofocus prop can be used to ensure that, the first time you open the dialog, a ListBox item is selected. However, if you open the dialog, select an item, click outside of it to close the dialog, and then re-open, you'll note that autofocus is totally ignored. This feels like a bug, but I'm reticent to make that determination myself.

Certainly feels like a bug, digging right now. Doesn't seem to happen for a Menu though, so if you can use a Menu instead it should just work.

EDIT: Looks like ListBox could use the same kind of logic as Menu does here. At the moment the ListBox is rendered, its collection hasn't been formed (needs a second render) and thus useSelectableCollection's call here doesn't actually set a focused key since the collection is empty.

[mattrothenberg]

@LFDanLu Thanks so much for the quick response! I'll try using a Menu and see what it feels like.

[LFDanLu]

For whoever picks up this ticket, we'll consider adding a prop to ListBox to control Esc behavior and fix the second point using the code outlined in my previous comment

[DoReVo]

Trying to do the same thing, using Menu works for me.

[lzhou-aops]

Same here - was having a bunch of different issues, saw this thread, swapped out ListBox for Menu with no other changes and everything cleared up. 😅 For any future readers, you can probably also get rid of any Dialogs if you do this

[joshuajaco]

We are experiencing a similar issue. We have a GridList inside a Popover. We can't use the Menu component as we need a GridList.

[snowystinger]

Should be able to change the Esc behavior with a capturing keydown listener, something like so https://codesandbox.io/p/sandbox/rac-multiselect-forked-w8gt9s

Can probably either put it on the Dialog or on the ListBox.

@LFDanLu we've had some changes in how the Collection is constructed now, so I'm updating that link with a reference https://github.com/adobe/react-spectrum/blob/14f324fe890fcedc6e34889d9b04d5d6bfeb8380/packages/react-aria-components/src/Menu.tsx#L159C8-L159C45

Though won't this potentially cause an issue with empty states? Maybe we can solve it a different way now. Extra side note, autofocus doesn't appear to work with selected values regardless of if it's in a Dialog or not.

[LFDanLu]

@snowystinger Hm in that case we might need to render a copy of the collection even when the Dialog is closed.

[snowystinger]

I don't think that will work given that autoFocus="first" isn't working outside of Dialogs from what I can tell.