Skip to content

Commit a53e4df

Browse files
ktaborsktabors
authored
DatePicker persisting valid TimeField value (#4648)
* persisting valid DatePicker TimeField value * test that fails locally passes on production * test that fails locally passes on production * more lint and test cleanup * moving and refactoring the conditionally valid time logic * fixing a times called in a test * picking up the initial spin button change for time segments * lint equals fix * changing clearedSegment to a ref since we don't need it to render * adding current month and year to test
1 parent 973f40a commit a53e4df

File tree

4 files changed

+96
-24
lines changed

4 files changed

+96
-24
lines changed

‎packages/@react-spectrum/datepicker/stories/DatePicker.stories.tsx‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -363,7 +363,7 @@ function Example(props) {
363363
}
364364

365365
function ControlledExample(props) {
366-
let [value, setValue] = React.useState(null);
366+
let [value, setValue] = React.useState(props.value);
367367

368368
return (
369369
<Flex direction="column" alignItems="center" gap="size-150">

‎packages/@react-spectrum/datepicker/test/DatePicker.test.js‎

Lines changed: 70 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -519,7 +519,7 @@ describe('DatePicker', function () {
519519
expect(getTextValue(combobox)).toBe('2/4/2019, 1:45 AM');
520520
});
521521

522-
it('should not fire onChange until both date and time are selected', function () {
522+
it('should fire onChange until both date and time are selected', function () {
523523
let onChange = jest.fn();
524524
let {getByRole, getAllByRole, getAllByLabelText} = render(
525525
<Provider theme={theme}>
@@ -577,17 +577,18 @@ describe('DatePicker', function () {
577577

578578
expect(document.activeElement).toHaveAttribute('aria-valuetext', '00');
579579

580-
expect(onChange).not.toHaveBeenCalled();
581-
expectPlaceholder(combobox, 'mm/dd/yyyy, ––:–– AM');
580+
expect(onChange).toHaveBeenCalledTimes(1);
581+
let parts = formatter.formatToParts(today(getLocalTimeZone()).toDate(getLocalTimeZone()));
582+
let month = parts.find(p => p.type === 'month').value;
583+
let day = parts.find(p => p.type === 'day').value;
584+
let year = parts.find(p => p.type === 'year').value;
585+
expectPlaceholder(combobox, `${month}/${day}/${year}, 12:00 AM`);
582586

583587
fireEvent.keyDown(hour, {key: 'ArrowRight'});
584588
fireEvent.keyUp(hour, {key: 'ArrowRight'});
585589

586590
expect(document.activeElement).toHaveAttribute('aria-label', 'AM/PM, ');
587-
expect(document.activeElement).toHaveAttribute('aria-valuetext', 'Empty');
588-
589-
fireEvent.keyDown(document.activeElement, {key: 'ArrowUp'});
590-
fireEvent.keyUp(document.activeElement, {key: 'ArrowUp'});
591+
expect(document.activeElement).toHaveAttribute('aria-valuetext', 'AM');
591592

592593
expect(dialog).toBeVisible();
593594
expect(onChange).toHaveBeenCalledTimes(1);
@@ -670,6 +671,68 @@ describe('DatePicker', function () {
670671
expect(onChange).not.toHaveBeenCalled();
671672
});
672673

674+
it('should confirm valid date time on dialog close', function () {
675+
let onChange = jest.fn();
676+
let {getByRole, getAllByRole, getAllByLabelText} = render(
677+
<Provider theme={theme}>
678+
<DatePicker label="Date" granularity="minute" onChange={onChange} />
679+
</Provider>
680+
);
681+
682+
let combobox = getAllByRole('group')[0];
683+
expectPlaceholder(combobox, 'mm/dd/yyyy, ––:–– AM');
684+
685+
let button = getByRole('button');
686+
triggerPress(button);
687+
act(() => jest.runAllTimers());
688+
689+
let dialog = getByRole('dialog');
690+
expect(dialog).toBeVisible();
691+
692+
let cells = getAllByRole('gridcell');
693+
let todayCell = cells.find(cell => cell.firstChild.getAttribute('aria-label')?.startsWith('Today'));
694+
triggerPress(todayCell.firstChild);
695+
expect(todayCell).toHaveAttribute('aria-selected', 'true');
696+
expect(onChange).not.toHaveBeenCalled();
697+
698+
let timeField = getAllByLabelText('Time')[0];
699+
expectPlaceholder(timeField, '––:–– AM');
700+
701+
let hour = within(timeField).getByLabelText('hour,');
702+
expect(hour).toHaveAttribute('role', 'spinbutton');
703+
expect(hour).toHaveAttribute('aria-valuetext', 'Empty');
704+
705+
act(() => hour.focus());
706+
fireEvent.keyDown(hour, {key: 'ArrowUp'});
707+
fireEvent.keyUp(hour, {key: 'ArrowUp'});
708+
709+
expect(hour).toHaveAttribute('aria-valuetext', '12 AM');
710+
711+
let minute = within(timeField).getByLabelText('minute,');
712+
expect(minute).toHaveAttribute('role', 'spinbutton');
713+
expect(minute).toHaveAttribute('aria-valuetext', 'Empty');
714+
715+
act(() => minute.focus());
716+
fireEvent.keyDown(minute, {key: 'ArrowUp'});
717+
fireEvent.keyUp(minute, {key: 'ArrowUp'});
718+
fireEvent.keyDown(minute, {key: 'ArrowUp'});
719+
fireEvent.keyUp(minute, {key: 'ArrowUp'});
720+
721+
expect(minute).toHaveAttribute('aria-valuetext', '01');
722+
723+
userEvent.click(document.body);
724+
act(() => jest.runAllTimers());
725+
726+
expect(dialog).not.toBeInTheDocument();
727+
expect(onChange).toHaveBeenCalledTimes(2);
728+
let formatter = new Intl.DateTimeFormat('en-US', {year: 'numeric', month: 'numeric', day: 'numeric'});
729+
let parts = formatter.formatToParts(today(getLocalTimeZone()).toDate(getLocalTimeZone()));
730+
let month = parts.find(p => p.type === 'month').value;
731+
let day = parts.find(p => p.type === 'day').value;
732+
let year = parts.find(p => p.type === 'year').value;
733+
expectPlaceholder(combobox, `${month}/${day}/${year}, 12:01 AM`);
734+
});
735+
673736
it('should clear date and time when controlled value is set to null', function () {
674737
function ControlledDatePicker() {
675738
let [value, setValue] = React.useState(null);
@@ -715,8 +778,6 @@ describe('DatePicker', function () {
715778
fireEvent.keyDown(hour, {key: 'ArrowRight'});
716779
fireEvent.keyUp(hour, {key: 'ArrowRight'});
717780
expect(document.activeElement).toHaveAttribute('aria-label', 'AM/PM, ');
718-
fireEvent.keyDown(document.activeElement, {key: 'ArrowUp'});
719-
fireEvent.keyUp(document.activeElement, {key: 'ArrowUp'});
720781
expect(document.activeElement).toHaveAttribute('aria-valuetext', 'AM');
721782

722783
userEvent.click(document.body);

‎packages/@react-spectrum/datepicker/test/DateRangePicker.test.js‎

Lines changed: 14 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -584,7 +584,7 @@ describe('DateRangePicker', function () {
584584
expectPlaceholder(startDate, 'mm/dd/yyyy, ––:–– AM');
585585
expectPlaceholder(endDate, 'mm/dd/yyyy, ––:–– AM');
586586

587-
for (let timeField of [startTimeField, endTimeField]) {
587+
for (let [index, timeField] of [startTimeField, endTimeField].entries()) {
588588
let hour = within(timeField).getByLabelText('hour,');
589589
expect(hour).toHaveAttribute('role', 'spinbutton');
590590
expect(hour).toHaveAttribute('aria-valuetext', 'Empty');
@@ -609,18 +609,22 @@ describe('DateRangePicker', function () {
609609

610610
expect(document.activeElement).toHaveAttribute('aria-valuetext', '00');
611611

612-
expect(onChange).not.toHaveBeenCalled();
613-
expectPlaceholder(startDate, 'mm/dd/yyyy, ––:–– AM');
614-
expectPlaceholder(endDate, 'mm/dd/yyyy, ––:–– AM');
612+
if (index === 0) {
613+
expect(onChange).toHaveBeenCalledTimes(0);
614+
expectPlaceholder(startDate, 'mm/dd/yyyy, ––:–– AM');
615+
expectPlaceholder(endDate, 'mm/dd/yyyy, ––:–– AM');
616+
} else {
617+
let localTime = today(getLocalTimeZone());
618+
expect(onChange).toHaveBeenCalledTimes(1);
619+
expectPlaceholder(startDate, `${localTime.month}/1/${localTime.year}, 12:00 AM`);
620+
expectPlaceholder(endDate, `${localTime.month}/2/${localTime.year}, 12:00 AM`);
621+
}
615622

616-
fireEvent.keyDown(hour, {key: 'ArrowRight'});
617-
fireEvent.keyUp(hour, {key: 'ArrowRight'});
623+
fireEvent.keyDown(document.activeElement, {key: 'ArrowRight'});
624+
fireEvent.keyUp(document.activeElement, {key: 'ArrowRight'});
618625

619626
expect(document.activeElement).toHaveAttribute('aria-label', 'AM/PM, ');
620-
expect(document.activeElement).toHaveAttribute('aria-valuetext', 'Empty');
621-
622-
fireEvent.keyDown(document.activeElement, {key: 'ArrowUp'});
623-
fireEvent.keyUp(document.activeElement, {key: 'ArrowUp'});
627+
expect(document.activeElement).toHaveAttribute('aria-valuetext', 'AM');
624628
}
625629

626630
expect(dialog).toBeVisible();
@@ -761,8 +765,6 @@ describe('DateRangePicker', function () {
761765
fireEvent.keyDown(hour, {key: 'ArrowRight'});
762766
fireEvent.keyUp(hour, {key: 'ArrowRight'});
763767
expect(document.activeElement).toHaveAttribute('aria-label', 'AM/PM, ');
764-
fireEvent.keyDown(document.activeElement, {key: 'ArrowUp'});
765-
fireEvent.keyUp(document.activeElement, {key: 'ArrowUp'});
766768
}
767769

768770
userEvent.click(document.body);

‎packages/@react-stately/datepicker/src/useDateFieldState.ts‎

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -203,6 +203,8 @@ export function useDateFieldState<T extends DateValue = DateValue>(props: DateFi
203203
() => props.value || props.defaultValue ? {...allSegments} : {}
204204
);
205205

206+
let clearedSegment = useRef<string>();
207+
206208
// Reset placeholder when calendar changes
207209
let lastCalendarIdentifier = useRef(calendar.identifier);
208210
useEffect(() => {
@@ -235,19 +237,23 @@ export function useDateFieldState<T extends DateValue = DateValue>(props: DateFi
235237
if (props.isDisabled || props.isReadOnly) {
236238
return;
237239
}
240+
let validKeys = Object.keys(validSegments);
241+
let allKeys = Object.keys(allSegments);
238242

243+
// if all the segments are completed or a timefield with everything but am/pm set the time, also ignore when am/pm cleared
239244
if (newValue == null) {
240245
setDate(null);
241246
setPlaceholderDate(createPlaceholderDate(props.placeholderValue, granularity, calendar, defaultTimeZone));
242247
setValidSegments({});
243-
} else if (Object.keys(validSegments).length >= Object.keys(allSegments).length) {
248+
} else if (validKeys.length >= allKeys.length || (validKeys.length === allKeys.length - 1 && allSegments.dayPeriod && !validSegments.dayPeriod && clearedSegment.current !== 'dayPeriod')) {
244249
// The display calendar should not have any effect on the emitted value.
245250
// Emit dates in the same calendar as the original value, if any, otherwise gregorian.
246251
newValue = toCalendar(newValue, v?.calendar || new GregorianCalendar());
247252
setDate(newValue);
248253
} else {
249254
setPlaceholderDate(newValue);
250255
}
256+
clearedSegment.current = null;
251257
};
252258

253259
let dateValue = useMemo(() => displayValue.toDate(timeZone), [displayValue, timeZone]);
@@ -293,7 +299,9 @@ export function useDateFieldState<T extends DateValue = DateValue>(props: DateFi
293299
let adjustSegment = (type: Intl.DateTimeFormatPartTypes, amount: number) => {
294300
if (!validSegments[type]) {
295301
markValid(type);
296-
if (Object.keys(validSegments).length >= Object.keys(allSegments).length) {
302+
let validKeys = Object.keys(validSegments);
303+
let allKeys = Object.keys(allSegments);
304+
if (validKeys.length >= allKeys.length || (validKeys.length === allKeys.length - 1 && allSegments.dayPeriod && !validSegments.dayPeriod)) {
297305
setValue(displayValue);
298306
}
299307
} else {
@@ -349,6 +357,7 @@ export function useDateFieldState<T extends DateValue = DateValue>(props: DateFi
349357
},
350358
clearSegment(part) {
351359
delete validSegments[part];
360+
clearedSegment.current = part;
352361
setValidSegments({...validSegments});
353362

354363
let placeholder = createPlaceholderDate(props.placeholderValue, granularity, calendar, defaultTimeZone);

0 commit comments

Comments
 (0)