Skip to content

Commit 09dfef6

Browse files
fzaninottofzaninotto
authored
Merge pull request #8039 from marmelab/fix-select-input-empty-required
Add ability to remove empty option in SelectInput for required fields
2 parents e7ce4ab + 924acec commit 09dfef6

File tree

3 files changed

+140
-34
lines changed

3 files changed

+140
-34
lines changed

‎packages/ra-ui-materialui/src/input/SelectInput.spec.tsx

+50-23
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,5 @@
11
import * as React from 'react';
2-
import {
3-
findByText,
4-
fireEvent,
5-
render,
6-
screen,
7-
waitFor,
8-
} from '@testing-library/react';
2+
import { fireEvent, render, screen, waitFor } from '@testing-library/react';
93
import {
104
required,
115
testDataProvider,
@@ -73,7 +67,7 @@ describe('<SelectInput />', () => {
7367
).toEqual('rea');
7468
});
7569

76-
it('should render disable choices marked so', () => {
70+
it('should render disabled choices marked so', () => {
7771
render(
7872
<AdminContext dataProvider={testDataProvider()}>
7973
<SimpleForm onSubmit={jest.fn()}>
@@ -98,6 +92,34 @@ describe('<SelectInput />', () => {
9892
screen.getByText('React').getAttribute('aria-disabled')
9993
).toEqual('true');
10094
});
95+
96+
it('should include an empty option by default', () => {
97+
render(
98+
<AdminContext dataProvider={testDataProvider()}>
99+
<SimpleForm onSubmit={jest.fn()}>
100+
<SelectInput {...defaultProps} />
101+
</SimpleForm>
102+
</AdminContext>
103+
);
104+
fireEvent.mouseDown(
105+
screen.getByLabelText('resources.posts.fields.language')
106+
);
107+
expect(screen.queryAllByRole('option')).toHaveLength(3);
108+
});
109+
110+
it('should not include an empty option if the field is required', () => {
111+
render(
112+
<AdminContext dataProvider={testDataProvider()}>
113+
<SimpleForm onSubmit={jest.fn()}>
114+
<SelectInput {...defaultProps} validate={required()} />
115+
</SimpleForm>
116+
</AdminContext>
117+
);
118+
fireEvent.mouseDown(
119+
screen.getByLabelText('resources.posts.fields.language *')
120+
);
121+
expect(screen.queryAllByRole('option')).toHaveLength(2);
122+
});
101123
});
102124

103125
describe('emptyText', () => {
@@ -375,12 +397,16 @@ describe('<SelectInput />', () => {
375397
defaultValues={{ language: 'ang' }}
376398
onSubmit={jest.fn()}
377399
>
378-
<SelectInput {...defaultProps} validate={required()} />
400+
<SelectInput
401+
{...defaultProps}
402+
helperText="helperText"
403+
validate={() => 'error'}
404+
/>
379405
</SimpleForm>
380406
</AdminContext>
381407
);
382-
const error = screen.queryAllByText('ra.validation.required');
383-
expect(error.length).toEqual(0);
408+
screen.getByText('helperText');
409+
expect(screen.queryAllByText('error')).toHaveLength(0);
384410
});
385411

386412
it('should not be displayed if field has been touched but is valid', () => {
@@ -391,18 +417,21 @@ describe('<SelectInput />', () => {
391417
mode="onBlur"
392418
onSubmit={jest.fn()}
393419
>
394-
<SelectInput {...defaultProps} validate={required()} />
420+
<SelectInput
421+
{...defaultProps}
422+
helperText="helperText"
423+
validate={() => undefined}
424+
/>
395425
</SimpleForm>
396426
</AdminContext>
397427
);
398428
const input = screen.getByLabelText(
399-
'resources.posts.fields.language *'
429+
'resources.posts.fields.language'
400430
);
401431
input.focus();
402432
input.blur();
403433

404-
const error = screen.queryAllByText('ra.validation.required');
405-
expect(error.length).toEqual(0);
434+
screen.getByText('helperText');
406435
});
407436

408437
it('should be displayed if field has been touched and is invalid', async () => {
@@ -411,27 +440,25 @@ describe('<SelectInput />', () => {
411440
<SimpleForm mode="onChange" onSubmit={jest.fn()}>
412441
<SelectInput
413442
{...defaultProps}
443+
helperText="helperText"
414444
emptyText="Empty"
415-
validate={required()}
445+
validate={() => 'error'}
416446
/>
417447
</SimpleForm>
418448
</AdminContext>
419449
);
420450

421451
const select = screen.getByLabelText(
422-
'resources.posts.fields.language *'
452+
'resources.posts.fields.language'
423453
);
424454
fireEvent.mouseDown(select);
425455

426456
const optionAngular = screen.getByText('Angular');
427457
fireEvent.click(optionAngular);
458+
select.blur();
428459

429-
const optionEmpty = screen.getByText('Empty');
430-
fireEvent.click(optionEmpty);
431-
432-
await waitFor(() => {
433-
expect(screen.queryByText('ra.validation.required'));
434-
});
460+
await screen.findByText('error');
461+
expect(screen.queryAllByText('helperText')).toHaveLength(0);
435462
});
436463
});
437464

‎packages/ra-ui-materialui/src/input/SelectInput.stories.tsx

+76-1
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import * as React from 'react';
22
import { createMemoryHistory } from 'history';
33
import { Admin, AdminContext } from 'react-admin';
4-
import { Resource } from 'ra-core';
4+
import { Resource, required } from 'ra-core';
55
import polyglotI18nProvider from 'ra-i18n-polyglot';
66
import englishMessages from 'ra-language-english';
77

@@ -24,6 +24,42 @@ export const Basic = () => (
2424
</Wrapper>
2525
);
2626

27+
export const DefaultValue = () => (
28+
<Wrapper>
29+
<SelectInput
30+
source="gender"
31+
choices={[
32+
{ id: 'M', name: 'Male ' },
33+
{ id: 'F', name: 'Female' },
34+
]}
35+
defaultValue="F"
36+
/>
37+
</Wrapper>
38+
);
39+
40+
export const InitialValue = () => (
41+
<AdminContext
42+
i18nProvider={i18nProvider}
43+
dataProvider={
44+
{
45+
getOne: () => Promise.resolve({ data: { id: 1, gender: 'F' } }),
46+
} as any
47+
}
48+
>
49+
<Edit resource="posts" id="1">
50+
<SimpleForm>
51+
<SelectInput
52+
source="gender"
53+
choices={[
54+
{ id: 'M', name: 'Male ' },
55+
{ id: 'F', name: 'Female' },
56+
]}
57+
/>
58+
</SimpleForm>
59+
</Edit>
60+
</AdminContext>
61+
);
62+
2763
export const Disabled = () => (
2864
<Wrapper>
2965
<SelectInput
@@ -37,6 +73,45 @@ export const Disabled = () => (
3773
</Wrapper>
3874
);
3975

76+
export const Validate = () => (
77+
<Wrapper>
78+
<SelectInput
79+
source="gender"
80+
choices={[
81+
{ id: 'M', name: 'Male ' },
82+
{ id: 'F', name: 'Female' },
83+
]}
84+
validate={() => 'error'}
85+
/>
86+
</Wrapper>
87+
);
88+
89+
export const Required = () => (
90+
<Wrapper>
91+
<SelectInput
92+
source="gender"
93+
choices={[
94+
{ id: 'M', name: 'Male ' },
95+
{ id: 'F', name: 'Female' },
96+
]}
97+
validate={required()}
98+
/>
99+
</Wrapper>
100+
);
101+
102+
export const EmptyText = () => (
103+
<Wrapper>
104+
<SelectInput
105+
source="gender"
106+
choices={[
107+
{ id: 'M', name: 'Male ' },
108+
{ id: 'F', name: 'Female' },
109+
]}
110+
emptyText="None"
111+
/>
112+
</Wrapper>
113+
);
114+
40115
const i18nProvider = polyglotI18nProvider(() => englishMessages);
41116

42117
const Wrapper = ({ children }) => (

‎packages/ra-ui-materialui/src/input/SelectInput.tsx

+14-10
Original file line numberDiff line numberDiff line change
@@ -219,8 +219,10 @@ export const SelectInput = (props: SelectInputProps) => {
219219
});
220220

221221
const createItem = create || onCreate ? getCreateItem() : null;
222-
const finalChoices =
223-
create || onCreate ? [...allChoices, createItem] : allChoices;
222+
let finalChoices = allChoices;
223+
if (create || onCreate) {
224+
finalChoices = [...finalChoices, createItem];
225+
}
224226

225227
const renderMenuItem = useCallback(
226228
choice => {
@@ -302,14 +304,16 @@ export const SelectInput = (props: SelectInputProps) => {
302304
margin={margin}
303305
{...sanitizeRestProps(rest)}
304306
>
305-
<MenuItem
306-
value={emptyValue}
307-
key="null"
308-
aria-label={translate('ra.action.clear_input_value')}
309-
title={translate('ra.action.clear_input_value')}
310-
>
311-
{renderEmptyItemOption()}
312-
</MenuItem>
307+
{!isRequired && (
308+
<MenuItem
309+
value={emptyValue}
310+
key="null"
311+
aria-label={translate('ra.action.clear_input_value')}
312+
title={translate('ra.action.clear_input_value')}
313+
>
314+
{renderEmptyItemOption()}
315+
</MenuItem>
316+
)}
313317
{finalChoices.map(renderMenuItem)}
314318
</StyledResettableTextField>
315319
{createElement}

0 commit comments

Comments
 (0)