Material-ui: [Docs] Autocomplete Multiple Values delete bug

[Studio384]

• [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 😯

Clicking the delete button for an option in the "Multiple Values" and "Fixed Options" examples removes whatever is first in the last.

Expected Behavior 🤔

Clicking the delete button for an option removes its own optoin.

Steps to Reproduce 🕹

Link:

1. https://material-ui.com/components/autocomplete/#multiple-values
2. https://material-ui.com/components/autocomplete/#fixed-options

Steps:

1. Add a new option
2. Try to delete the new option
3. The option already present will be deleted

Your Environment 🌎

MUI Documentation

[Studio384]

On the same page, but unrelated to the issue, the following is written:

The component exposes a factory to create a filter method that can provided to the filerOption prop. You can use it to change the default option filter behavior.

I guess that's supposed to say filterOption and not filerOption.

[oliviertassinari]

@Studio384 Thank you for the report. I can think of two possible solutions:

1. We continue with the attribute path, it's relatively simple, outside the IE 11 ponyfill:

diff --git a/packages/material-ui-lab/src/useAutocomplete/useAutocomplete.js b/packages/material-ui-lab/src/useAutocomplete/useAutocomplete.js
index bc02f10f1..f43a99e08 100644
--- a/packages/material-ui-lab/src/useAutocomplete/useAutocomplete.js
+++ b/packages/material-ui-lab/src/useAutocomplete/useAutocomplete.js
@@ -11,6 +11,26 @@ function stripDiacritics(string) {
     : string;
 }

+// Support IE 11
+// https://developer.mozilla.org/en-US/docs/Web/API/Element/closest
+function closest(element, string) {
+  let closestPonyfill = Element.prototype.closest;
+
+  if (!closestPonyfill) {
+    closestPonyfill = () => {
+      let el = element;
+
+      do {
+        if (el.matches(string)) return el;
+        el = el.parentNode;
+      } while (el !== null && el.nodeType === 1);
+      return null;
+    };
+  }
+
+  return closestPonyfill(string);
+}
+
 export function createFilterOptions(config = {}) {
   const {
     ignoreAccents = true,
@@ -584,7 +604,8 @@ export default function useAutocomplete(props) {
   };

   const handleTagDelete = event => {
-    const index = Number(event.currentTarget.getAttribute('data-tag-index'));
+    const tagRoot = closest(event.currentTarget, '[data-tag-index]');
+    const index = Number(tagRoot.getAttribute('data-tag-index'));
     const newValue = [...value];
     newValue.splice(index, 1);
     handleValue(event, newValue);

1. We require getTagProps() to collect the tag index. Something like this:

diff --git a/packages/material-ui-lab/src/Autocomplete/Autocomplete.js b/packages/material-ui-lab/src/Autocomplete/Autocomplete.js
index 4c6aa5260..eddf2c0cb 100644
--- a/packages/material-ui-lab/src/Autocomplete/Autocomplete.js
+++ b/packages/material-ui-lab/src/Autocomplete/Autocomplete.js
@@ -220,21 +220,20 @@ const Autocomplete = React.forwardRef(function Autocomplete(props, ref) {
   let startAdornment;

   if (multiple && value.length > 0) {
-    const tagProps = {
-      ...getTagProps(),
+    const getTagProps2 = params => ({
       className: classes.tag,
-    };
+      ...getTagProps(params),
+    });

     if (renderTags) {
-      startAdornment = renderTags(value, tagProps);
+      startAdornment = renderTags(value, getTagProps2);
     } else {
       startAdornment = value.map((option, index) => (
         <Chip
           key={index}
-          data-tag-index={index}
           tabIndex={-1}
           label={getOptionLabel(option)}
-          {...tagProps}
+          {...getTagProps2({ index })}
         />
       ));
     }
diff --git a/packages/material-ui-lab/src/useAutocomplete/useAutocomplete.js b/packages/material-ui-lab/src/useAutocomplete/useAutocomplete.js
index bc02f10f1..ea709e7f7 100644
--- a/packages/material-ui-lab/src/useAutocomplete/useAutocomplete.js
+++ b/packages/material-ui-lab/src/useAutocomplete/useAutocomplete.js
@@ -583,8 +583,7 @@ export default function useAutocomplete(props) {
     selectNewValue(event, filteredOptions[highlightedIndexRef.current]);
   };

-  const handleTagDelete = event => {
-    const index = Number(event.currentTarget.getAttribute('data-tag-index'));
+  const handleTagDelete = index => event => {
     const newValue = [...value];
     newValue.splice(index, 1);
     handleValue(event, newValue);
@@ -672,8 +671,9 @@ export default function useAutocomplete(props) {
         event.preventDefault();
       },
     }),
-    getTagProps: () => ({
-      onDelete: handleTagDelete,
+    getTagProps: ({ index }) => ({
+      'data-tag-index': index,
+      onDelete: handleTagDelete(index),
     }),
     getPopupProps: () => ({
       role: 'presentation',

I might prefer 2. 🤔

[joshwooding]

2 looks better to me as well.

[oliviertassinari]

@joshwooding Alright, then let's go for it. I like the mental model of it, it makes the getTagProps() similar to the getOptionProps().