-
-
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] Add ability to render custom single value #45387
base: master
Are you sure you want to change the base?
[Autocomplete] Add ability to render custom single value #45387
Conversation
Netlify deploy preview@material-ui/core: parsed: +0.07% , gzip: +0.09% Bundle size reportDetails of bundle changes (Toolpad) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @ZeeshanTamboli, thanks for working on this!
I think the solution is the correct one but I'm wondering what's the best for the API design, the options I see:
- This PR's solution: add a
renderSingleValue
prop which is only used whenmultiple={false}
- Add a
renderValue
prop which works regardless ofmultiple
, and deprecaterenderTags
in favor of it - Use slots, like
slots.value
andslotProps.value
- Other
What do you think @aarongarciah?
@@ -513,6 +514,7 @@ const Autocomplete = React.forwardRef(function Autocomplete(inProps, ref) { | |||
focusedTag, | |||
anchorEl, | |||
setAnchorEl, | |||
tagProps, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
useAutocomplete
doesn't return tagProps
, did you mean getTagProps
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It now returns in this PR for a single tag: https://github.com/mui/material-ui/pull/45387/files#diff-a3e15b1f22a8ed2de23c8cee46ea92a1d2828d4718cb471c6404d9baadd2c68fR1154-R1158. Should I rename it to singleTagProps
? I avoided get*
since it doesn’t need to be a function.
I wouldn’t use the slots API since this is passed as |
@ZeeshanTamboli can you provide a code snippet of how it'd look like if we combined rendering single and multiple values in a single prop like |
@aarongarciah Check out the existing
If we were to combine them into a single
renderValue={(values, getTagProps) => {
if (Array.isArray(values)) {
return values.map((val, index) => {
const { key, ...tagProps } = getTagProps({ index });
return <Chip key={key} label={val.title} {...tagProps} />;
});
}
return <Chip label={values.title} />;
}}
I think it's possible but adds complexity. To avoid point 2 above, we'd need a conditional generic type based on |
@ZeeshanTamboli thanks for the explanation. What I find weird is that users can use Although there's no concept of "tags" in single selection mode by default in the Autocomplete (i.e. single values are represented as plain text in the input), I think Would it make sense for I have doubts about the best path forward. Let's not rush this since it's important to keep the Autocomplete API at a minimum (it's large enough already) and a workaround exists (not ideal though). |
This idea makes sense to me. I would rename it to |
I'll try it out. Let's see how it pans out. |
@aarongarciah @DiegoAndai Based on the discussion, the For single value, when multiple isn't provided: renderValue={(tagValue, getTagProps) => (
<Chip label={tagValue[0].title} {...getTagProps()} />
)} Note that For multiple values (same as before): renderValue={(tagValue, getTagProps) =>
tagValue.map((option, index) => (
<Chip
key={getTagProps({ index }).key}
label={option.title}
{...getTagProps({ index })}
/>
))
} Should I go ahead and make the changes? |
@ZeeshanTamboli looks good! I wonder if it should be called Also cc @michaldudak in case you have some feedback. |
I wonder if it wouldn't be possible to have the type of the parameter depend on the Autocomplete's |
Closes #26440
Based on the upvotes and requests in #26440, this PR introduces a new
renderSingleValue
prop to customize the display of the selected value. This is separate fromrenderTags
for clarity. I avoided naming itrenderTag
to prevent potential confusion and typos.Preview: https://deploy-preview-45387--material-ui.netlify.app/material-ui/react-autocomplete/#custom-single-value-rendering