[TablePagination] Fix select variant not working #33974
Conversation
ZeeshanTamboli
added
bug 🐛
| @@ -199,7 +199,7 @@ const TablePagination = React.forwardRef(function TablePagination(inProps, ref) | |||
| {rowsPerPageOptions.length > 1 && ( | |||
| <TablePaginationSelect | |||
| variant="standard" | |||
| input={<InputBase />} | |||
| {...(!SelectProps.variant && { input: <InputBase /> })} | |||
There was a problem hiding this comment.
InputBase should not always be rendered in TablePaginationSelect. If variant is passed through SelectProps prop, that particular variant input should be rendered instead.
There was a problem hiding this comment.
I wonder why the <InputBase /> is needed in the first place 🤔
There was a problem hiding this comment.
It's needed. Otherwise, <Input /> is rendered which is different visually. So InputBase should be rendered when no Select variant is provided.
And if variant standard is removed and input={<InputBase />} is kept, there is a slight difference in UI and argos detects changes.
There was a problem hiding this comment.
Confirmed, the InputBase is needed.
| @@ -114,7 +114,7 @@ const Select = React.forwardRef(function Select(inProps, ref) { | |||
| ...(multiple && native && variant === 'outlined' ? { notched: true } : {}), | |||
| ref: inputComponentRef, | |||
| className: clsx(InputComponent.props.className, className), | |||
| variant, | |||
| ...(!input && { variant }), | |||
There was a problem hiding this comment.
Do not propagate variant to root div in select input if custom input element is provided via Select's input prop. Basically, do not let variant be set like in the image below:
There was a problem hiding this comment.
Nice, it does not introduce a regression if I do this:
<Select input={<OutlinedInput />}>So, looks good.
| @@ -114,7 +114,7 @@ const Select = React.forwardRef(function Select(inProps, ref) { | |||
| ...(multiple && native && variant === 'outlined' ? { notched: true } : {}), | |||
| ref: inputComponentRef, | |||
| className: clsx(InputComponent.props.className, className), | |||
| variant, | |||
| ...(!input && { variant }), | |||
There was a problem hiding this comment.
@ZeeshanTamboli I think it deserves a comment. Can you add a comment that explain the consition and maybe link to the issue?
There was a problem hiding this comment.
@siriwatknp I added the comment.
* add test cases * apply Select variant if provided * do not attach variant as an attribute to select root div * update test * test: parentNode -> parentElement * fix visual regressions by allowing variant in inputProps * add back variant standard * add comment
Fixes #33894
Both the comments below are fixes of #33894.