react-spectrum icon indicating copy to clipboard operation
react-spectrum copied to clipboard

useSelect can't use click to open select anymore and weird behaviour

Open pontageorge opened this issue 2 years ago • 5 comments

🐛 Bug Report

I have implemented the useSelect in my dropdown/select component.

The issue I was experiencing which I solved in a hacky way was when I would open the select by pressing Enter then closing it with Escape when I would try to open again the select using the mouse click it would not work. The isOpen state is not updating anymore.

To solve this I had to remove from the buttonProps the onKeyDown function and pass the restOfProps to the button:

const { onKeyDown, ...restOfProps } = buttonProps;

Which is weird as that is the function that should also listen to the Enter key event. Started removing functions from the buttonProps object 1 by 1 to see which one is actually listening to the Enter and the Enter key event worked fine after removing all functions from the buttonProps until I removed onClick:

const { onKeyDown, onBlur, onKeyUp, onFocus, onPointerDown, onPointerUp, onClick, ...restOfProps } = buttonProps;

After this the dropdown wouldn't open at all with either Enter or click

Now if I only remove onClick and pass the restOfProps to the button the Enter key event works but I still get the bug described above:

const { onClick, ...restOfProps } = buttonProps;

EDIT:

I have realised how to avoid the bug without having to remove onKeyDown from the buttonProps. The reason this bug is occuring is apparently because for the FocusScope of the Select body I am passing autoFocus and as seen bellow I am setting autoFocus: false for the useListBox as our component can sometimes have a Search input at the top of the body and we want that to have focus on open rather than the options as we are using this as more of a dropdown than a Select so we are passing more than the options. Is there any work around around this without removing the onKeyDown function from the props?

💻 Code Sample

Can't provide a whole code sample but I will provide which hooks I am using from react-aria and react-stately.


    const state = useSelectState({
        children: renderItems(),
        name,
        defaultOpen: open,
        isOpen: open,
        onOpenChange: setOpen,
        isRequired: required,
    });

    const {
        triggerProps,
        menuProps,
    } = useSelect({}, state, buttonNode);

    const { buttonProps } = useButton({ ...triggerProps, id: `${id}-button`, "aria-labelledby": `${id}-name` }, buttonNode);

    const { overlayProps } = useOverlay({
        isOpen: open,
        onClose: () => setOpen(false),
        shouldCloseOnBlur: true,
        isDismissable: true,
    }, ref);

    const { listBoxProps } = useListBox(
        { ...menuProps, autoFocus: false, id },
        state,
        infiniteScrollContainer,
    );

🌍 Your Environment

I am using version 3.12.0 for react-aria

pontageorge avatar Feb 17 '22 14:02 pontageorge

Its a bit hard for me to understand how your select component is implemented here without a codesandbox since I can't tell what the structure of your select is and what props are getting spread where. Is there any chance you can make a simplified example in a codesandbox?

As an aside, if you have a search input field in your ListBox, perhaps a combobox would be more appropriate for your use case?

LFDanLu avatar Feb 17 '22 18:02 LFDanLu

Hi, here is a very stripped down version of the Dropdown we use:

https://codesandbox.io/s/agitated-wildflower-mrbus2?file=/src/Dropdown/Search.jsx

To test the bug:

  1. Focus the dropdown with TAB.
  2. Press Enter to open.
  3. Close with Escape.
  4. Try to open with Click.

It won't work anymore. To be able to open again need to use Enter. Click works fine if you don't do the Enter -> Escape combination. Even Enter and then Click outside doesn't do the bug.

The reason why we can't use ComboBox is because it doesn't align with the design we implement and can't make the change.

pontageorge avatar Feb 18 '22 12:02 pontageorge

Did some digging, it looks like the reason for this lies within usePress, specifically https://github.com/adobe/react-spectrum/blob/1aa66e6e75e2da014f44177686c3332cd625fb9f/packages/%40react-aria/interactions/src/usePress.ts#L287-L293. What happens here is the following:

  1. User begins a press via Enter on the button. This sets state.isPressed to true within usePress for the button. At the same time the dropdown is opened and focus is set on the search field.
  2. The user then releases the Enter key. This triggers usePress's onKeyUp but since focus is now on the searchfield, the isValidKeyboardEvent returns false and the state.isPressed for the button isn't properly set to false. This is the reason why subsequent clicks on the trigger button: https://github.com/adobe/react-spectrum/blob/1aa66e6e75e2da014f44177686c3332cd625fb9f/packages/%40react-aria/interactions/src/usePress.ts#L331

Will have to discuss with the team about a possible fix (reset isPressed state regardless of isValidKeyboardEvent if e.target from onKeyUp is on a different element than the one that triggered onKeyDown), a workaround in the meantime would be to only open the dropdown when the user releases the Enter/Space/etc key.

LFDanLu avatar Feb 18 '22 19:02 LFDanLu

Sounds good thank you. I already found a workround removing the onKeyDown hook. I will have to see if it introduces other bugs but at the moment looks alright.

The usePress hooks are very inconsistent we are also implementing useButton and it default calls e.preventDefault and e.stopPropagation in some places. For example the onClick hook coming from buttonProps doesn't do e.preventDefault and it doesn't expose the Event object and we need it to do that for some buttons so we had to strip onClick and pass it ourselves.

Can you also see if it is possible to reintroduce the Event object so we can decide when to use these functions as we had to strip down the buttonProps heavily for our Button component to make it work.

pontageorge avatar Feb 21 '22 10:02 pontageorge

Hello, it seems like I'm getting this error as well on react-aria 3.17.0, using the default demo code for useSelect (https://react-spectrum.adobe.com/react-aria/useSelect.html). It is possible to open the dropdown with space and enter, but regular clicking does not work.

anonmily avatar Jul 08 '22 02:07 anonmily