[Autocomplete] ArrowLeft key press throws an error when setting renderTags to return null.

[Phebonacci]

When the ArrowLeft key is pressed while there are selected options and renderTags is set to return null (so that no tags are rendered), the component breaks.

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

Current Behavior 😯

When the ArrowLeft key is pressed while there are selected options and renderTags is set to return null (so that no tags are rendered), the component breaks and returns the error describe below:

It appears that the useAutocomplete hook tries to put focus on the tag elements without checking whether there are tags rendered or not as per the source code:

  const focusTag = useEventCallback((tagToFocus) => {
    if (tagToFocus === -1) {
      inputRef.current.focus();
    } else {
      anchorEl.querySelector(`[data-tag-index="${tagToFocus}"]`).focus(); // <-- The line in question.
    }
  });

Expected Behavior 🤔

When the ArrowLeft key is pressed while there are selected options and renderTags is set to return null, it shouldn't try to put focus on tags.

Steps to Reproduce 🕹

Codesandbox link: https://codesandbox.io/s/nt24o Taken from: https://material-ui.com/components/autocomplete/#githubs-picker

Steps:

1. Open the Autocomplete component by clicking on Labels.
2. Make sure there are options selected.
3. Make sure the Autocomplete input field is focused.
4. Press the ArrowLeft key. Error should appear.

Context 🔦

We are trying to build a component similar to the Github Labels picker sample provided by the docs: https://material-ui.com/components/autocomplete/#githubs-picker because we need a filterable/searchable dropdown component, but we don't need to show tags/chips because we want the input field of the Autocomplete to just act like an input for filtering.

Your Environment 🌎

`npx @material-ui/envinfo`

Browser used: Chrome

  System:
    OS: macOS 10.15.7
  Binaries:
    Node: 14.17.0 - ~/.nvm/versions/node/v14.17.0/bin/node
    Yarn: 1.22.10 - /usr/local/bin/yarn
    npm: 6.14.13 - ~/.nvm/versions/node/v14.17.0/bin/npm
  Browsers:
    Chrome: 92.0.4515.131
    Edge: 92.0.902.78
    Firefox: 91.0
    Safari: 14.1.2

[mnajdova]

Thanks for the report. I can reproduce. We should add a check for it. This diff should do it:

index 62bc7df557..e81a9baaa4 100644
--- a/packages/material-ui-unstyled/src/AutocompleteUnstyled/useAutocomplete.js
+++ b/packages/material-ui-unstyled/src/AutocompleteUnstyled/useAutocomplete.js
@@ -252,7 +252,7 @@ export default function useAutocomplete(props) {
     if (tagToFocus === -1) {
       inputRef.current.focus();
     } else {
-      anchorEl.querySelector(`[data-tag-index="${tagToFocus}"]`).focus();
+      anchorEl.querySelector(`[data-tag-index="${tagToFocus}"]`)?.focus();
     }
   });

@Phebonacci would you like to create a PR? :)

[namangirdhar16]

Hi @Phebonacci, if this hasn't fixed yet, can i work on this issue, also can you guide me on how to get started since i am just starting out?

[anirudh1713]

This seems inactive, so I'm starting to work on this.

[anirudh1713]

Hey @mnajdova can you help me with the test? I've no experience with tests. sorry about that. I've done something like this. though this throws an error.

it('should not throw when render tags is null', () => {
    const Test = (props) => {
      const { options, renderTags, defaultValue } = props;
      const {
        groupedOptions,
        getRootProps,
        getInputLabelProps,
        getInputProps,
        getListboxProps,
        getOptionProps,
        setAnchorEl,
      } = useAutocomplete({
        options,
        open: true,
        // useAutocomplete doesn't expect renderTags prop
        renderTags,
        multiple: true,
        defaultValue,
      });

      return (
        <div>
          <div {...getRootProps()} ref={setAnchorEl}>
            <label {...getInputLabelProps()}>useAutocomplete</label>
            <input {...getInputProps()} type="text" />
          </div>
          {groupedOptions.length > 0 ? (
            <ul {...getListboxProps()}>
              {groupedOptions.map((option, index) => {
                return <li {...getOptionProps({ option, index })}>{option}</li>;
              })}
            </ul>
          ) : null}
        </div>
      );
    };

    render(<Test options={['foo', 'bar']} renderTags={() => null} defaultValue={['foo']} />);

    const combobox = screen.getByRole('combobox');
    expect(combobox).toBeVisible();

    fireEvent.keyDown(combobox, { key: 'ArrowLeft' });
  });

Error: `keydown` events can only be targeted at the active element which is

[mnajdova]

Apologizes, I have missed this comment. @anirudh1713 you need to first focus the combobox before firing some keyboard events. Would be best if you can create a PR and we can iterate there.

[Phebonacci]

Thanks for the report. I can reproduce. We should add a check for it. This diff should do it:

index 62bc7df557..e81a9baaa4 100644
--- a/packages/material-ui-unstyled/src/AutocompleteUnstyled/useAutocomplete.js
+++ b/packages/material-ui-unstyled/src/AutocompleteUnstyled/useAutocomplete.js
@@ -252,7 +252,7 @@ export default function useAutocomplete(props) {
     if (tagToFocus === -1) {
       inputRef.current.focus();
     } else {
-      anchorEl.querySelector(`[data-tag-index="${tagToFocus}"]`).focus();
+      anchorEl.querySelector(`[data-tag-index="${tagToFocus}"]`)?.focus();
     }
   });

@Phebonacci would you like to create a PR? :)

Hi @mnajdova , apologies. I have been busy and only got back to this. Has anyone started working on a PR? I can also create one if no one has started working on this yet.

[Phebonacci]

Hey @mnajdova can you help me with the test? I've no experience with tests. sorry about that. I've done something like this. though this throws an error.

it('should not throw when render tags is null', () => {
    const Test = (props) => {
      const { options, renderTags, defaultValue } = props;
      const {
        groupedOptions,
        getRootProps,
        getInputLabelProps,
        getInputProps,
        getListboxProps,
        getOptionProps,
        setAnchorEl,
      } = useAutocomplete({
        options,
        open: true,
        // useAutocomplete doesn't expect renderTags prop
        renderTags,
        multiple: true,
        defaultValue,
      });

      return (
        <div>
          <div {...getRootProps()} ref={setAnchorEl}>
            <label {...getInputLabelProps()}>useAutocomplete</label>
            <input {...getInputProps()} type="text" />
          </div>
          {groupedOptions.length > 0 ? (
            <ul {...getListboxProps()}>
              {groupedOptions.map((option, index) => {
                return <li {...getOptionProps({ option, index })}>{option}</li>;
              })}
            </ul>
          ) : null}
        </div>
      );
    };

    render(<Test options={['foo', 'bar']} renderTags={() => null} defaultValue={['foo']} />);

    const combobox = screen.getByRole('combobox');
    expect(combobox).toBeVisible();

    fireEvent.keyDown(combobox, { key: 'ArrowLeft' });
  });

Error: `keydown` events can only be targeted at the active element which is

Hi @anirudh1713 , if you already have a PR, i can help you with the tests if you want.

[mnajdova]

@Phebonacci feel free to create a new PR, looks like no one else is actively working on the issue

[snorkel875]

May I tackle this issue?

[mnajdova]

There is no open linked PR, so whoever wants can open one :)

[gauravsoti1]

Hi, since there was no activity on this issue for 9 days. I have created a PR and have made sure to add a test.

[gauravsoti1]

@mnajdova In the latest version of the library, we don't have this problem, as we can observe here https://codesandbox.io/s/p7njj6?file=/demo.tsx (It's the same example just with latest library) But this issue is using version 4 of the library.

[gauravsoti1]

Should we close this issue?

[mnajdova]

Ah I haven't realized that the bug used v4. We don't accept fixes for v4, and it indeed seems like is already working in v5. I am closing this. Thanks @gauravsoti1 for the info :)