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

Phebonacci · 2021-08-23T21:19:26Z

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.

The issue is present in the latest release.
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:

Open the Autocomplete component by clicking on Labels.
Make sure there are options selected.
Make sure the Autocomplete input field is focused.
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

The text was updated successfully, but these errors were encountered:

mnajdova · 2021-08-24T12:01:35Z

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 · 2021-09-01T16:31:49Z

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 · 2021-09-07T13:34:27Z

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

anirudh1713 · 2021-09-11T09:46:37Z

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 · 2022-01-26T11:59:23Z

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 · 2022-02-22T05:06:02Z

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 · 2022-02-22T05:46:40Z

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 · 2022-03-03T11:25:50Z

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

snorkel875 · 2022-07-31T17:03:18Z

May I tackle this issue?

mnajdova · 2022-08-05T12:09:52Z

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

gauravsoti1 · 2022-08-09T13:14:47Z

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 · 2022-08-11T23:27:38Z

@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 · 2022-08-15T09:30:00Z

Should we close this issue?

mnajdova · 2022-08-25T07:04:31Z

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 :)

Phebonacci added the status: waiting for maintainer These issues haven't been looked at yet by a maintainer label Aug 23, 2021

mnajdova added bug 🐛 Something doesn't work component: autocomplete This is the name of the generic UI component, not the React module! and removed status: waiting for maintainer These issues haven't been looked at yet by a maintainer labels Aug 24, 2021

mnajdova added the good first issue Great for first contributions. Enable to learn the contribution process. label Aug 24, 2021

imsuvesh added a commit to imsuvesh/material-ui that referenced this issue Aug 24, 2021

Fixes mui#27933

a72a990

imsuvesh mentioned this issue Aug 24, 2021

[Autocomplete] Don't crash during keyboard navigation when renderTags returns nothing #27949

Closed

1 task

lpizzinidev mentioned this issue Oct 11, 2021

[Autocomplete] Don't crash during keyboard navigation when renderTags returns nothing #28985

Closed

1 task

gauravsoti1 mentioned this issue Aug 9, 2022

[Autocomplete] Fixed crash during keyboard navigation when renderTags function returns nothing #33872

Closed

1 task

mnajdova closed this as completed Aug 25, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

Phebonacci commented Aug 23, 2021 •

edited

Loading

mnajdova commented Aug 24, 2021

namangirdhar16 commented Sep 1, 2021

anirudh1713 commented Sep 7, 2021

anirudh1713 commented Sep 11, 2021

mnajdova commented Jan 26, 2022

Phebonacci commented Feb 22, 2022

Phebonacci commented Feb 22, 2022

mnajdova commented Mar 3, 2022

snorkel875 commented Jul 31, 2022

mnajdova commented Aug 5, 2022

gauravsoti1 commented Aug 9, 2022

gauravsoti1 commented Aug 11, 2022

gauravsoti1 commented Aug 15, 2022

mnajdova commented Aug 25, 2022

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

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

Comments

Phebonacci commented Aug 23, 2021 • edited Loading

Current Behavior 😯

Expected Behavior 🤔

Steps to Reproduce 🕹

Context 🔦

Your Environment 🌎

mnajdova commented Aug 24, 2021

namangirdhar16 commented Sep 1, 2021

anirudh1713 commented Sep 7, 2021

anirudh1713 commented Sep 11, 2021

mnajdova commented Jan 26, 2022

Phebonacci commented Feb 22, 2022

Phebonacci commented Feb 22, 2022

mnajdova commented Mar 3, 2022

snorkel875 commented Jul 31, 2022

mnajdova commented Aug 5, 2022

gauravsoti1 commented Aug 9, 2022

gauravsoti1 commented Aug 11, 2022

gauravsoti1 commented Aug 15, 2022

mnajdova commented Aug 25, 2022

Phebonacci commented Aug 23, 2021 •

edited

Loading