Material-ui: [Autocomplete] Simplify tooltip usage in options

Created on 15 Jan 2020  路  6Comments  路  Source: mui-org/material-ui

I found an issue with the implementation of autocomplete. If you look at the PR you generate the props from the useAutocomplete hook and pass that directly to the li. This is an issue because #11601 you are passing disabled directly from that which doesn't allow for the Tooltip workaround on the disabled elements here

I've already implemented this in #19235. I feel like this is a valid solution since you are currently able to change the 'ListBoxComponent = ul` to anything. You should have more control over the list item.

Also if you change the ListboxComponent to a div look at the HTML rendering. It renders invalid HTML and you can't change the li to anything you need:

Screen Shot 2020-01-15 at 10 35 00 AM

Heres a demo for the above screenshot:

Edit Material demo

  • [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 馃槸

The Tooltip isn't opening for disabled items.

Expected Behavior 馃

I expect Tooltip to render for disabled items. Using the workaround here the disabled items should render.

Steps to Reproduce 馃暪

We need to be able to use the Tooltip to explain to our users why an item is disabled. See example below:

Edit Material demo

Steps:

  1. Setup Autocomplete like above ^
  2. Try to render a tooltip
  3. The disabled items will not open the tooltip however the ones, not disabled will.

Context 馃敠

We need to be able to use the Tooltip to explain to our users why an item is disabled.

Your Environment 馃寧

Check the above code sandbox for more info.

| Tech | Version |
| ----------- | ------- |
| Material-UI | v4.8.3 |
| Material-UI-Lab | v4.0.0-alpha.39|
| React | 16.12.0 |
| Browser | Chrom |
| TypeScript | 3.7.4 |

breaking change Autocomplete enhancement good first issue

Most helpful comment

Would it make sense to make this change? Userland:

diff --git a/docs/src/pages/components/autocomplete/CheckboxesTags.tsx b/docs/src/pages/components/autocomplete/CheckboxesTags.tsx
index 97e99d4574..dbc8bcf274 100644
--- a/docs/src/pages/components/autocomplete/CheckboxesTags.tsx
+++ b/docs/src/pages/components/autocomplete/CheckboxesTags.tsx
@@ -18,8 +18,8 @@ export default function CheckboxesTags() {
       options={top100Films}
       disableCloseOnSelect
       getOptionLabel={(option) => option.title}
-      renderOption={(option, { selected }) => (
-        <React.Fragment>
+      renderOption={(props, option, { selected }) => (
+        <li聽{...props}>
           <Checkbox
             icon={icon}
             checkedIcon={checkedIcon}
@@ -27,7 +27,7 @@ export default function CheckboxesTags() {
             checked={selected}
           />
           {option.title}
-        </React.Fragment>
+        </li>
       )}
       style={{ width: 500 }}
       renderInput={(params) => (

From the implementation side of things, it would mean something close to this to have it work:

diff --git a/packages/material-ui-lab/src/Autocomplete/Autocomplete.d.ts b/packages/material-ui-lab/src/Autocomplete/Autocomplete.d.ts
index 03a025e822..097cf8fda8 100644
--- a/packages/material-ui-lab/src/Autocomplete/Autocomplete.d.ts
+++ b/packages/material-ui-lab/src/Autocomplete/Autocomplete.d.ts
@@ -165,11 +165,16 @@ export interface AutocompleteProps<
   /**
    * Render the option, use `getOptionLabel` by default.
    *
+   * @param {object} props The props to apply on the li element.
    * @param {T} option The option to render.
    * @param {object} state The state of the component.
    * @returns {ReactNode}
    */
-  renderOption?: (option: T, state: AutocompleteRenderOptionState) => React.ReactNode;
+  renderOption?: (
+    props: React.HTMLAttributes<HTMLLIElement>,
+    option: T,
+    state: AutocompleteRenderOptionState
+  ) => React.ReactNode;
   /**
    * Render the selected value.
    *
diff --git a/packages/material-ui-lab/src/Autocomplete/Autocomplete.js b/packages/material-ui-lab/src/Autocomplete/Autocomplete.js
index 88e1644283..076b4a9c0c 100644
--- a/packages/material-ui-lab/src/Autocomplete/Autocomplete.js
+++ b/packages/material-ui-lab/src/Autocomplete/Autocomplete.js
@@ -375,21 +375,20 @@ const Autocomplete = React.forwardRef(function Autocomplete(props, ref) {
       <ul className={classes.groupUl}>{params.children}</ul>
     </li>
   );
-
   const renderGroup = renderGroupProp || defaultRenderGroup;
-  const renderOption = renderOptionProp || getOptionLabel;
+
+  const defaultRenderOption = (props2, params) => (
+    <li {...props2}>{getOptionLabel(params.option)}</li>
+  );
+  const renderOption = renderOptionProp || defaultRenderOption;

   const renderListOption = (option, index) => {
     const optionProps = getOptionProps({ option, index });

-    return (
-      <li {...optionProps} className={classes.option}>
-        {renderOption(option, {
-          selected: optionProps['aria-selected'],
-          inputValue,
-        })}
-      </li>
-    );
+    return renderOption({ ...optionProps, className: classes.option }, option, {
+      selected: optionProps['aria-selected'],
+      inputValue,
+    });
   };

   const hasClearIcon = !disableClearable && !disabled;

Does anyone want to work on it? :)

All 6 comments

I have added the waiting for users upvotes tag. I'm closing the issue as we are not sure people are looking for such abstraction. So please upvote this issue if you are. We will prioritize our effort based on the number of upvotes.

A note for the core team members, in the future, that will come back to this issue ~10 of the upvotes comes from a single team and can be subtracted from the number of upvotes.


for instance:

  • sherodtaylor - Sherod Taylor - Bloomberg LP.
  • lisahoong - Lisa Hoong - Bloomberg LP.
  • lozzd - Laurie Denness - Bloomberg LP.
  • scottopell - Scott Opell - Bloomberg LP.
  • zohrenweiss - Zohren Weiss - Bloomberg LP.
  • jones77 - James Jones - Bloomberg LP.
  • dskoda1 - David Skoda - Bloomberg LP.

Would it make sense to make this change? Userland:

diff --git a/docs/src/pages/components/autocomplete/CheckboxesTags.tsx b/docs/src/pages/components/autocomplete/CheckboxesTags.tsx
index 97e99d4574..dbc8bcf274 100644
--- a/docs/src/pages/components/autocomplete/CheckboxesTags.tsx
+++ b/docs/src/pages/components/autocomplete/CheckboxesTags.tsx
@@ -18,8 +18,8 @@ export default function CheckboxesTags() {
       options={top100Films}
       disableCloseOnSelect
       getOptionLabel={(option) => option.title}
-      renderOption={(option, { selected }) => (
-        <React.Fragment>
+      renderOption={(props, option, { selected }) => (
+        <li聽{...props}>
           <Checkbox
             icon={icon}
             checkedIcon={checkedIcon}
@@ -27,7 +27,7 @@ export default function CheckboxesTags() {
             checked={selected}
           />
           {option.title}
-        </React.Fragment>
+        </li>
       )}
       style={{ width: 500 }}
       renderInput={(params) => (

From the implementation side of things, it would mean something close to this to have it work:

diff --git a/packages/material-ui-lab/src/Autocomplete/Autocomplete.d.ts b/packages/material-ui-lab/src/Autocomplete/Autocomplete.d.ts
index 03a025e822..097cf8fda8 100644
--- a/packages/material-ui-lab/src/Autocomplete/Autocomplete.d.ts
+++ b/packages/material-ui-lab/src/Autocomplete/Autocomplete.d.ts
@@ -165,11 +165,16 @@ export interface AutocompleteProps<
   /**
    * Render the option, use `getOptionLabel` by default.
    *
+   * @param {object} props The props to apply on the li element.
    * @param {T} option The option to render.
    * @param {object} state The state of the component.
    * @returns {ReactNode}
    */
-  renderOption?: (option: T, state: AutocompleteRenderOptionState) => React.ReactNode;
+  renderOption?: (
+    props: React.HTMLAttributes<HTMLLIElement>,
+    option: T,
+    state: AutocompleteRenderOptionState
+  ) => React.ReactNode;
   /**
    * Render the selected value.
    *
diff --git a/packages/material-ui-lab/src/Autocomplete/Autocomplete.js b/packages/material-ui-lab/src/Autocomplete/Autocomplete.js
index 88e1644283..076b4a9c0c 100644
--- a/packages/material-ui-lab/src/Autocomplete/Autocomplete.js
+++ b/packages/material-ui-lab/src/Autocomplete/Autocomplete.js
@@ -375,21 +375,20 @@ const Autocomplete = React.forwardRef(function Autocomplete(props, ref) {
       <ul className={classes.groupUl}>{params.children}</ul>
     </li>
   );
-
   const renderGroup = renderGroupProp || defaultRenderGroup;
-  const renderOption = renderOptionProp || getOptionLabel;
+
+  const defaultRenderOption = (props2, params) => (
+    <li {...props2}>{getOptionLabel(params.option)}</li>
+  );
+  const renderOption = renderOptionProp || defaultRenderOption;

   const renderListOption = (option, index) => {
     const optionProps = getOptionProps({ option, index });

-    return (
-      <li {...optionProps} className={classes.option}>
-        {renderOption(option, {
-          selected: optionProps['aria-selected'],
-          inputValue,
-        })}
-      </li>
-    );
+    return renderOption({ ...optionProps, className: classes.option }, option, {
+      selected: optionProps['aria-selected'],
+      inputValue,
+    });
   };

   const hasClearIcon = !disableClearable && !disabled;

Does anyone want to work on it? :)

Would it make sense to make this change? Userland:

I checked the changes and I think they make sense.

Does anyone want to work on it? :)

Looks like you already solved the problem and put the code, then what is remained? :D

Looks like you already solved the problem and put the code, then what is remained? :D

@ImanMahmoudinasab The hardest part is missing: to get it merged in the next branch. We would need a pull request, fix the tests, update the documentation, etc.

@oliviertassinari I'm going to start working on this.

Was this page helpful?
0 / 5 - 0 ratings