Fix <TextInput> missing placeholder with MUI v5.4
#8439
Conversation
|
Thank you! |
| @@ -8,17 +8,18 @@ import { CommonInputProps } from './CommonInputProps'; | |||
| import { TextInput, TextInputProps } from './TextInput'; | |||
|
|
|||
| export const SearchInput = (props: SearchInputProps) => { | |||
| const { label, ...rest } = props; | |||
There was a problem hiding this comment.
I don't understand why you made this change
There was a problem hiding this comment.
To handle the case when specifying undefined for a SearchInput's label explicitly (like <SearchInput source="q" alwaysOn label={undefined} />) will result in the default label is being displayed.
Reproduction on codesandbox
There was a problem hiding this comment.
Also noticed, that it's possible to use SearchInput with 0 label like (added this case to repro on CodeSandbox):
<SearchInput source="q" alwaysOn label={0} />I think, it would be better to revert all changes in this PR, that related to the SearchInput's label overwriting logic and open a new issue to discuss, what and how to do about this case (IMHO it's better to prevent users from providing a label at all instead of throwing an error).
| throw new Error( | ||
| "<SearchInput> isn't designed to be used with a label prop. Use <TextInput> if you need a label." | ||
| ); | ||
| } | ||
|
|
||
| return ( | ||
| <StyledTextInput | ||
| hiddenLabel |
There was a problem hiding this comment.
My bad, added it back
| @@ -166,4 +166,36 @@ describe('<TextInput />', () => { | |||
| expect(input.value).toEqual('bar'); | |||
| }); | |||
| }); | |||
|
|
|||
| describe('label', () => { | |||
| it('should not render label when `label` prop is `false`', () => { | |||
There was a problem hiding this comment.
You're missing a test showing that a proper label prop renders a label.
There was a problem hiding this comment.
I've added two more tests: when label not specified and when label is a non-empty string
| import { FilterForm } from '../list'; | ||
|
|
||
| describe('<SearchInput />', () => { | ||
| it('should not render label if passed explicit `undefined` value', async () => { |
There was a problem hiding this comment.
I don't understand this test.
There was a problem hiding this comment.
Added to cover the case of passing explicit undefined for the SearchInput's label property (more info here)
I made a reproduction of the bug: And with screenshots:
But the placeholder is visible after clicking on the SearchInput:
With the proposed changes, the placeholder now displayed correctly (both until the SearchInput is focused, and after that):
Yes, there should be (since MUI made the label's check only a little stricter) |
|
Tested the fix in Thanks for this contribution and well done for the analysis! |
slax57
changed the title
label property<TextInput> missing placeholder with MUI v5.4
Starting with MUI 5.4.0, they changed the way of displaying the label for TextField. Now the label will not be rendered only if
null,undefinedor""value is provided, but react-admin passesfalseas the label (so it will be rendered). This often affects the SearchInput component, which doesn't (and shouldn't) have label: placeholder is shown only when SearchInput is active (because the label replaces the placeholder when the input is not active).Also, I fixed a bug, where when providing an explicit
undefinedvalue to the SearchInput'slabelproperty, the default label (based onsource) was shown: