-
-
Notifications
You must be signed in to change notification settings - Fork 32.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[Autocomplete] Simplify tooltip usage in options #19254
Comments
I have added the |
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:
|
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? :) |
I checked the changes and I think they make sense.
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 |
@oliviertassinari I'm going to start working on this. |
I had this issue. Couldn't find any updates?
|
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 theli
. This is an issue because #11601 you are passing disabled directly from that which doesn't allow for theTooltip
workaround on the disabled elements hereI'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 theli
to anything you need:Heres a demo for the above screenshot:
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:Steps:
Autocomplete
like above ^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.
The text was updated successfully, but these errors were encountered: