Skip to content
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.

Sign up for GitHub

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

Jump to bottom

[fields] Implement empty visual state #8069

Merged
merged 8 commits into from
Mar 3, 2023
Merged

[fields] Implement empty visual state #8069

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
b9f7352
[fields] Implement empty input state
LukasTy Feb 27, 2023
f83ed96
Adapt tests to the change in behavior
LukasTy Feb 28, 2023
06008c8
Add `placeholder` in the field state
LukasTy Feb 28, 2023
770c2aa
Handle single input range fields
LukasTy Feb 28, 2023
4b04d12
Code review: Flavien
LukasTy Feb 28, 2023
1a35cff
Merge remote-tracking branch 'origin/next' into fields-empty-placeholder
LukasTy Mar 1, 2023
1b85f96
Avoid selection flicker
LukasTy Mar 1, 2023
38fc7b4
Fix test typing issues
LukasTy Mar 1, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 6 additions & 3 deletions ...s/x-date-pickers-pro/src/DesktopDateRangePicker/describes.DesktopDateRangePicker.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import {
adapterToUse,
buildFieldInteractions,
createPickerRenderer,
expectInputPlaceholder,
expectInputValue,
} from 'test/utils/pickers-utils';

Expand Down Expand Up @@ -45,10 +46,12 @@ describe('<DesktopDateRangePicker /> - Describes', () => {
assertRenderedValue: (expectedValues: any[]) => {
const textBoxes: HTMLInputElement[] = screen.getAllByRole('textbox');
expectedValues.forEach((value, index) => {
const expectedValueStr =
value == null ? 'MM/DD/YYYY' : adapterToUse.format(value, 'keyboardDate');
const input = textBoxes[index];
// TODO: Support single range input
expectInputValue(textBoxes[index], expectedValueStr, true);
if (!value) {
expectInputPlaceholder(input, 'MM/DD/YYYY');
}
expectInputValue(input, value ? adapterToUse.format(value, 'keyboardDate') : '', true);
});
},
setNewValue: (value, { isOpened, applySameValue, setEndDate = false } = {}) => {
Expand Down
9 changes: 6 additions & 3 deletions ...ges/x-date-pickers-pro/src/MobileDateRangePicker/describes.MobileDateRangePicker.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import { describeValue } from '@mui/x-date-pickers/tests/describeValue';
import {
adapterToUse,
createPickerRenderer,
expectInputPlaceholder,
expectInputValue,
openPicker,
} from 'test/utils/pickers-utils';
Expand Down Expand Up @@ -44,10 +45,12 @@ describe('<MobileDateRangePicker /> - Describes', () => {
screen.getByLabelText('End'),
];
expectedValues.forEach((value, index) => {
const expectedValueStr =
value == null ? 'MM/DD/YYYY' : adapterToUse.format(value, 'keyboardDate');
const input = textBoxes[index];
// TODO: Support single range input
expectInputValue(textBoxes[index], expectedValueStr, true);
if (!value) {
expectInputPlaceholder(input, 'MM/DD/YYYY');
}
expectInputValue(input, value ? adapterToUse.format(value, 'keyboardDate') : '', true);
});
},
setNewValue: (value, { isOpened, applySameValue, setEndDate = false } = {}) => {
Expand Down
16 changes: 10 additions & 6 deletions ...kers-pro/src/SingleInputDateRangeField/tests/selection.SingleInputDateRangeField.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,14 @@ describe('<SingleInputDateRangeField /> - Selection', () => {
position: number | string,
isSecondItem?: boolean,
) => {
act(() => {
fireEvent.mouseDown(input);
if (document.activeElement !== input) {
input.focus();
}
fireEvent.mouseUp(input);
});
clock.runToLast();
const clickPosition =
typeof position === 'string'
? input.value.indexOf(position, isSecondItem ? input.value.indexOf(position) + 1 : 0)
Expand All @@ -28,11 +36,6 @@ describe('<SingleInputDateRangeField /> - Selection', () => {
);
}
act(() => {
fireEvent.mouseDown(input);
if (document.activeElement !== input) {
input.focus();
}
fireEvent.mouseUp(input);
input.setSelectionRange(clickPosition, clickPosition);
fireEvent.click(input);

Expand All @@ -55,8 +58,9 @@ describe('<SingleInputDateRangeField /> - Selection', () => {
// Simulate a <Tab> focus interaction on desktop
act(() => {
input.focus();
input.select();
});
clock.runToLast();
input.select();

expectInputValue(input, 'MM / DD / YYYY – MM / DD / YYYY');
expect(getCleanedSelectedContent(input)).to.equal('MM / DD / YYYY – MM / DD / YYYY');
Expand Down
8 changes: 6 additions & 2 deletions packages/x-date-pickers/src/AdapterDateFns/localization.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,11 @@ import { DateTimePicker } from '@mui/x-date-pickers/DateTimePicker';
import { AdapterDateFns } from '@mui/x-date-pickers/AdapterDateFns';
import { screen } from '@mui/monorepo/test/utils/createRenderer';
import { expect } from 'chai';
import { createPickerRenderer, expectInputValue } from 'test/utils/pickers-utils';
import {
createPickerRenderer,
expectInputPlaceholder,
expectInputValue,
} from 'test/utils/pickers-utils';
import fr from 'date-fns/locale/fr';
import de from 'date-fns/locale/de';

Expand Down Expand Up @@ -37,7 +41,7 @@ describe('<AdapterDateFns />', () => {
it('should have correct placeholder', () => {
render(<DateTimePicker />);

expectInputValue(screen.getByRole('textbox'), localizedTexts[localeKey].placeholder, true);
expectInputPlaceholder(screen.getByRole('textbox'), localizedTexts[localeKey].placeholder);
});

it('should have well formatted value', () => {
Expand Down
8 changes: 6 additions & 2 deletions packages/x-date-pickers/src/AdapterDateFnsJalali/localization.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,11 @@ import { DateTimePicker } from '@mui/x-date-pickers/DateTimePicker';
import { AdapterDateFnsJalali } from '@mui/x-date-pickers/AdapterDateFnsJalali';
import { screen } from '@mui/monorepo/test/utils/createRenderer';
import { expect } from 'chai';
import { createPickerRenderer, expectInputValue } from 'test/utils/pickers-utils';
import {
createPickerRenderer,
expectInputPlaceholder,
expectInputValue,
} from 'test/utils/pickers-utils';
import enUS from 'date-fns/locale/en-US';
import faIR from 'date-fns-jalali/locale/fa-IR';
import faJalaliIR from 'date-fns-jalali/locale/fa-jalali-IR';
Expand Down Expand Up @@ -47,7 +51,7 @@ describe('<AdapterDateFnsJalali />', () => {
it('should have correct placeholder', () => {
render(<DateTimePicker />);

expectInputValue(screen.getByRole('textbox'), localizedTexts[localeKey].placeholder, true);
expectInputPlaceholder(screen.getByRole('textbox'), localizedTexts[localeKey].placeholder);
});

it('should have well formatted value', () => {
Expand Down
4 changes: 2 additions & 2 deletions packages/x-date-pickers/src/AdapterDayjs/AdapterDayjs.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import { expect } from 'chai';
import {
buildPickerDragInteractions,
createPickerRenderer,
expectInputPlaceholder,
expectInputValue,
MockedDataTransfer,
} from 'test/utils/pickers-utils';
Expand Down Expand Up @@ -51,10 +52,9 @@ describe('<AdapterDayjs />', () => {
it('should have correct placeholder', () => {
render(<DateTimePicker />);

expectInputValue(
expectInputPlaceholder(
screen.getByRole('textbox'),
localizedTexts[localeKey].placeholder,
true,
);
});

Expand Down
8 changes: 6 additions & 2 deletions packages/x-date-pickers/src/AdapterLuxon/localization.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,11 @@ import { DateTimePicker } from '@mui/x-date-pickers/DateTimePicker';
import { AdapterLuxon } from '@mui/x-date-pickers/AdapterLuxon';
import { screen } from '@mui/monorepo/test/utils/createRenderer';
import { expect } from 'chai';
import { createPickerRenderer, expectInputValue } from 'test/utils/pickers-utils';
import {
createPickerRenderer,
expectInputPlaceholder,
expectInputValue,
} from 'test/utils/pickers-utils';

const testDate = new Date(2018, 4, 15, 9, 35);
const localizedTexts = {
Expand Down Expand Up @@ -35,7 +39,7 @@ describe('<AdapterLuxon />', () => {
it('should have correct placeholder', () => {
render(<DateTimePicker />);

expectInputValue(screen.getByRole('textbox'), localizedTexts[localeKey].placeholder, true);
expectInputPlaceholder(screen.getByRole('textbox'), localizedTexts[localeKey].placeholder);
});

it('should have well formatted value', () => {
Expand Down
8 changes: 6 additions & 2 deletions packages/x-date-pickers/src/AdapterMoment/AdapterMoment.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,11 @@ import { DateTimePicker } from '@mui/x-date-pickers/DateTimePicker';
import { AdapterMoment } from '@mui/x-date-pickers/AdapterMoment';
import { screen } from '@mui/monorepo/test/utils/createRenderer';
import { expect } from 'chai';
import { createPickerRenderer, expectInputValue } from 'test/utils/pickers-utils';
import {
createPickerRenderer,
expectInputPlaceholder,
expectInputValue,
} from 'test/utils/pickers-utils';
import 'moment/locale/de';
import 'moment/locale/fr';

Expand Down Expand Up @@ -36,7 +40,7 @@ describe('<AdapterMoment />', () => {
it('should have correct placeholder', () => {
render(<DateTimePicker />);

expectInputValue(screen.getByRole('textbox'), localizedTexts[localeKey].placeholder, true);
expectInputPlaceholder(screen.getByRole('textbox'), localizedTexts[localeKey].placeholder);
});

it('should have well formatted value', () => {
Expand Down
8 changes: 6 additions & 2 deletions packages/x-date-pickers/src/AdapterMomentHijri/localization.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,11 @@ import { DateTimePicker } from '@mui/x-date-pickers/DateTimePicker';
import { AdapterMomentHijri } from '@mui/x-date-pickers/AdapterMomentHijri';
import { screen } from '@mui/monorepo/test/utils/createRenderer';
import { expect } from 'chai';
import { createPickerRenderer, expectInputValue } from 'test/utils/pickers-utils';
import {
createPickerRenderer,
expectInputPlaceholder,
expectInputValue,
} from 'test/utils/pickers-utils';
import 'moment/locale/ar';

const testDate = new Date(2018, 4, 15, 9, 35);
Expand All @@ -28,7 +32,7 @@ describe('<AdapterMomentHijri />', () => {
it('should have correct placeholder', () => {
render(<DateTimePicker />);

expectInputValue(screen.getByRole('textbox'), localizedTexts[localeKey].placeholder, true);
expectInputPlaceholder(screen.getByRole('textbox'), localizedTexts[localeKey].placeholder);
});

it('should have well formatted value', () => {
Expand Down
8 changes: 6 additions & 2 deletions packages/x-date-pickers/src/AdapterMomentJalaali/localization.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,11 @@ import { DateTimePicker } from '@mui/x-date-pickers/DateTimePicker';
import { AdapterMomentJalaali } from '@mui/x-date-pickers/AdapterMomentJalaali';
import { screen } from '@mui/monorepo/test/utils/createRenderer';
import { expect } from 'chai';
import { createPickerRenderer, expectInputValue } from 'test/utils/pickers-utils';
import {
createPickerRenderer,
expectInputPlaceholder,
expectInputValue,
} from 'test/utils/pickers-utils';
import 'moment/locale/fa';
import moment from 'moment';

Expand Down Expand Up @@ -38,7 +42,7 @@ describe('<AdapterMomentJalaali />', () => {
it('should have correct placeholder', () => {
render(<DateTimePicker />);

expectInputValue(screen.getByRole('textbox'), localizedTexts[localeKey].placeholder, true);
expectInputPlaceholder(screen.getByRole('textbox'), localizedTexts[localeKey].placeholder);
});

it('should have well formatted value', () => {
Expand Down
15 changes: 11 additions & 4 deletions packages/x-date-pickers/src/DateField/tests/describes.DateField.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import * as React from 'react';
import { describeConformance, screen, userEvent } from '@mui/monorepo/test/utils';
import { describeConformance, userEvent } from '@mui/monorepo/test/utils';
import TextField from '@mui/material/TextField';
import { describeValidation } from '@mui/x-date-pickers/tests/describeValidation';
import { describeValue } from '@mui/x-date-pickers/tests/describeValue';
Expand All @@ -11,6 +11,7 @@ import {
expectInputValue,
buildFieldInteractions,
getTextbox,
expectInputPlaceholder,
} from 'test/utils/pickers-utils';

describe('<DateField /> - Describes', () => {
Expand Down Expand Up @@ -50,9 +51,15 @@ describe('<DateField /> - Describes', () => {
emptyValue: null,
clock,
assertRenderedValue: (expectedValue: any) => {
const expectedValueStr =
expectedValue == null ? 'MM/DD/YYYY' : adapterToUse.format(expectedValue, 'keyboardDate');
expectInputValue(screen.getByRole('textbox'), expectedValueStr, true);
const input = getTextbox();
if (!expectedValue) {
expectInputPlaceholder(input, 'MM/DD/YYYY');
}
expectInputValue(
input,
expectedValue ? adapterToUse.format(expectedValue, 'keyboardDate') : '',
true,
);
},
setNewValue: (value) => {
const newValue = adapterToUse.addDays(value, 1);
Expand Down
10 changes: 5 additions & 5 deletions packages/x-date-pickers/src/DateField/tests/format.DateField.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import * as React from 'react';
import { expectInputValue, getTextbox } from 'test/utils/pickers-utils';
import { expectInputPlaceholder, expectInputValue, getTextbox } from 'test/utils/pickers-utils';
import { DateField } from '@mui/x-date-pickers/DateField';
import { describeAdapters } from '@mui/x-date-pickers/tests/describeAdapters';

Expand All @@ -11,7 +11,7 @@ describeAdapters('<DateField /> - Format', DateField, ({ render, adapter }) => {
<DateField format={`${startChar}Escaped${endChar} ${adapter.formats.year}`} />,
);
const input = getTextbox();
expectInputValue(input, 'Escaped YYYY');
expectInputPlaceholder(input, 'Escaped YYYY');

setProps({ value: adapter.date(new Date(2019, 0, 1)) });
expectInputValue(input, 'Escaped 2019');
Expand All @@ -26,7 +26,7 @@ describeAdapters('<DateField /> - Format', DateField, ({ render, adapter }) => {
/>,
);
const input = getTextbox();
expectInputValue(input, 'MMMM Escaped YYYY');
expectInputPlaceholder(input, 'MMMM Escaped YYYY');

setProps({ value: adapter.date(new Date(2019, 0, 1)) });
expectInputValue(input, 'January Escaped 2019');
Expand All @@ -47,7 +47,7 @@ describeAdapters('<DateField /> - Format', DateField, ({ render, adapter }) => {
/>,
);
const input = getTextbox();
expectInputValue(input, 'MMMM Escaped [ YYYY');
expectInputPlaceholder(input, 'MMMM Escaped [ YYYY');

setProps({ value: adapter.date(new Date(2019, 0, 1)) });
expectInputValue(input, 'January Escaped [ 2019');
Expand All @@ -63,7 +63,7 @@ describeAdapters('<DateField /> - Format', DateField, ({ render, adapter }) => {
/>,
);
const input = getTextbox();
expectInputValue(input, 'Escaped MMMM Escaped YYYY');
expectInputPlaceholder(input, 'Escaped MMMM Escaped YYYY');

setProps({ value: adapter.date(new Date(2019, 0, 1)) });
expectInputValue(input, 'Escaped January Escaped 2019');
Expand Down
26 changes: 16 additions & 10 deletions packages/x-date-pickers/src/DateField/tests/selection.DateField.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,18 +14,21 @@ describe('<DateField /> - Selection', () => {
const { render, clock } = createPickerRenderer({ clock: 'fake' });

const clickOnInput = (input: HTMLInputElement, position: number | string) => {
act(() => {
fireEvent.mouseDown(input);
if (document.activeElement !== input) {
input.focus();
}
fireEvent.mouseUp(input);
});
clock.runToLast();
const clickPosition = typeof position === 'string' ? input.value.indexOf(position) : position;
if (clickPosition === -1) {
throw new Error(
`Failed to find value to select "${position}" in input value: ${input.value}`,
);
}
act(() => {
fireEvent.mouseDown(input);
if (document.activeElement !== input) {
input.focus();
}
fireEvent.mouseUp(input);
input.setSelectionRange(clickPosition, clickPosition);
fireEvent.click(input);

Expand Down Expand Up @@ -56,8 +59,9 @@ describe('<DateField /> - Selection', () => {
// Simulate a <Tab> focus interaction on desktop
act(() => {
input.focus();
input.select();
});
clock.runToLast();
input.select();

expectInputValue(input, 'MM / DD / YYYY');
expect(getCleanedSelectedContent(input)).to.equal('MM / DD / YYYY');
Expand All @@ -69,8 +73,9 @@ describe('<DateField /> - Selection', () => {
// Simulate a <Tab> focus interaction on desktop
act(() => {
input.focus();
input.select();
});
clock.runToLast();
input.select();

expectInputValue(input, '- YYYY');
expect(getCleanedSelectedContent(input)).to.equal('- YYYY');
Expand All @@ -82,11 +87,12 @@ describe('<DateField /> - Selection', () => {
// Simulate a touch focus interaction on mobile
act(() => {
input.focus();
input.setSelectionRange(9, 10);
clock.runToLast();
});

clock.runToLast();
expectInputValue(input, 'MM / DD / YYYY');

input.setSelectionRange(9, 11);
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Was this a mistake previously, that somehow did not fail the test? 🤔 🤷

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think @oliviertassinari wrote the initial version of this test
We try to mimic a touch focus interaction, but I did not check what was the correct execution order of the events.

Copy link
Member

@oliviertassinari oliviertassinari Mar 4, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Correct, this test comes from #6207.

It seems that this test caught a regression, changing the test hides the bug. See how on https://deploy-preview-8069--material-ui-x.netlify.app/x/react-date-pickers/fields/#fields-to-edit-a-single-element, now I need to tap twice to select the year:

20230305-002805-333.mp4

One tap is enough on https://next.mui.com/x/react-date-pickers/fields/#fields-to-edit-a-single-element.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This behavior change is intended. It was a trade off between having a placeholder and being able to pick a section in a non focused input in a single click.

Copy link
Member

@oliviertassinari oliviertassinari Mar 5, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, OK, so using the <input> placeholder attribute over applying the placeholder color. This sounds simpler for developers when they provide a custom textbox.


expect(input.selectionStart).to.equal(9);
expect(input.selectionEnd).to.equal(11);
});
Expand Down
Loading
Loading