From 70da8b05fb1bd807ee1b58e3058ee2d74ec0cfc3 Mon Sep 17 00:00:00 2001 From: Esteban Munoz Date: Tue, 30 May 2023 15:32:52 -0700 Subject: [PATCH 1/6] fix: Add null to value prop to handle controlled cases. --- .../etc/react-datepicker-compat.api.md | 9 +--- .../components/DatePicker/DatePicker.test.tsx | 47 ++++++++++++++++++- .../components/DatePicker/DatePicker.types.ts | 5 +- .../components/DatePicker/useDatePicker.tsx | 27 +++++------ .../DatePickerControlled.stories.tsx | 12 +++-- ...DatePickerCustomDateFormatting.stories.tsx | 4 +- 6 files changed, 70 insertions(+), 34 deletions(-) diff --git a/packages/react-components/react-datepicker-compat/etc/react-datepicker-compat.api.md b/packages/react-components/react-datepicker-compat/etc/react-datepicker-compat.api.md index f6efab232fb88..ff299deaf9d18 100644 --- a/packages/react-components/react-datepicker-compat/etc/react-datepicker-compat.api.md +++ b/packages/react-components/react-datepicker-compat/etc/react-datepicker-compat.api.md @@ -6,14 +6,7 @@ /// -import type { ComponentProps } from '@fluentui/react-utilities'; -import type { ComponentState } from '@fluentui/react-utilities'; -import type { ForwardRefComponent } from '@fluentui/react-utilities'; -import { Input } from '@fluentui/react-input'; -import type { PositioningProps } from '@fluentui/react-positioning'; import * as React_2 from 'react'; -import type { Slot } from '@fluentui/react-utilities'; -import type { SlotClassNames } from '@fluentui/react-utilities'; // @public export function addDays(date: Date, days: number): Date; @@ -175,7 +168,7 @@ export type DatePickerProps = Omit>, 'de positioning?: PositioningProps; placeholder?: string; today?: Date; - value?: Date; + value?: Date | null; formatDate?: (date?: Date) => string; parseDateFromString?: (dateStr: string) => Date | null; firstDayOfWeek?: DayOfWeek; diff --git a/packages/react-components/react-datepicker-compat/src/components/DatePicker/DatePicker.test.tsx b/packages/react-components/react-datepicker-compat/src/components/DatePicker/DatePicker.test.tsx index b11b6394a759e..35304fe698997 100644 --- a/packages/react-components/react-datepicker-compat/src/components/DatePicker/DatePicker.test.tsx +++ b/packages/react-components/react-datepicker-compat/src/components/DatePicker/DatePicker.test.tsx @@ -16,13 +16,32 @@ function queryByRoleDialog(result: RenderResult) { } } -const getDatepickerPopoverElement = (result: RenderResult) => { +const getDatepickerPopupElement = (result: RenderResult) => { result.getByRole('combobox').click(); const dialog = queryByRoleDialog(result); expect(dialog).not.toBeNull(); return dialog!; }; +const ControlledDatePicker = (props: Partial>) => { + const [value, setValue] = React.useState(null); + + return ( + { + props.formatDate?.(); + return !date ? '' : date.getDate() + '/' + (date.getMonth() + 1) + '/' + (date.getFullYear() % 100); + }} + onSelectDate={date => { + props.onSelectDate?.(date); + date !== undefined && setValue(date); + }} + /> + ); +}; + describe('DatePicker', () => { beforeEach(() => { resetIdsForTests(); @@ -41,7 +60,7 @@ describe('DatePicker', () => { popupSurface: datePickerClassNames.popupSurface, calendar: datePickerClassNames.calendar, }, - getPortalElement: getDatepickerPopoverElement, + getPortalElement: getDatepickerPopupElement, }, ], }, @@ -136,4 +155,28 @@ describe('DatePicker', () => { expect(input.getAttribute('value')).toBe('15/1/20'); }); + + it('calls onSelectDate when controlled', () => { + const onSelectDate = jest.fn(); + const result = render( onSelectDate()} />); + + fireEvent.click(result.getByRole('combobox')); + result.getAllByRole('gridcell')[10].click(); + + expect(onSelectDate).toHaveBeenCalledTimes(1); + }); + + it('calls onSelectDate and formatDate when controlled', () => { + const onSelectDate = jest.fn(); + const formatDate = jest.fn(); + const result = render( + formatDate()} onSelectDate={date => onSelectDate()} />, + ); + + fireEvent.click(result.getByRole('combobox')); + result.getAllByRole('gridcell')[10].click(); + + expect(onSelectDate).toHaveBeenCalledTimes(1); + expect(formatDate).toHaveBeenCalledTimes(1); + }); }); diff --git a/packages/react-components/react-datepicker-compat/src/components/DatePicker/DatePicker.types.ts b/packages/react-components/react-datepicker-compat/src/components/DatePicker/DatePicker.types.ts index 7418c522045b4..e2d656ed1add5 100644 --- a/packages/react-components/react-datepicker-compat/src/components/DatePicker/DatePicker.types.ts +++ b/packages/react-components/react-datepicker-compat/src/components/DatePicker/DatePicker.types.ts @@ -135,8 +135,11 @@ export type DatePickerProps = Omit>, 'de /** * Default value of the DatePicker, if any + * + * When the component is controlled, `null` should be used instead of `undefined` to avoid controlled vs. uncontrolled + * ambiguity. */ - value?: Date; + value?: Date | null; /** * Optional method to format the chosen date to a string to display in the DatePicker diff --git a/packages/react-components/react-datepicker-compat/src/components/DatePicker/useDatePicker.tsx b/packages/react-components/react-datepicker-compat/src/components/DatePicker/useDatePicker.tsx index 1c1f1a2f3615e..3c2509714b4d8 100644 --- a/packages/react-components/react-datepicker-compat/src/components/DatePicker/useDatePicker.tsx +++ b/packages/react-components/react-datepicker-compat/src/components/DatePicker/useDatePicker.tsx @@ -66,27 +66,22 @@ function usePopupVisibility(props: DatePickerProps) { } function useSelectedDate({ formatDate, onSelectDate, value }: DatePickerProps) { - const [selectedDate, setSelectedDateState] = useControllableState({ - initialState: undefined, + const [selectedDate, setSelectedDateState] = useControllableState({ + initialState: null, state: value, }); - const [formattedDate, setFormattedDate] = React.useState(() => (value && formatDate ? formatDate(value) : '')); - - const setSelectedDate = (newDate: Date | undefined) => { - if ( - (selectedDate === undefined && newDate !== undefined) || - (selectedDate !== undefined && newDate === undefined) || - (newDate && selectedDate && (newDate > selectedDate || newDate < selectedDate)) - ) { - onSelectDate?.(newDate); - } + const [formattedDate, setFormattedDate] = React.useState(() => + value !== null && value && formatDate ? formatDate(value) : '', + ); + const setSelectedDate = (newDate: Date | null | undefined) => { + onSelectDate?.(newDate); setSelectedDateState(newDate); - setFormattedDate(newDate && formatDate ? formatDate(newDate) : ''); + setFormattedDate(newDate !== null && newDate && formatDate ? formatDate(newDate) : ''); }; React.useEffect(() => { - setFormattedDate(value && formatDate ? formatDate(value) : ''); + setFormattedDate(value !== null && value && formatDate ? formatDate(value) : ''); }, [formatDate, value]); return [selectedDate, formattedDate, setSelectedDate, setFormattedDate] as const; @@ -215,7 +210,7 @@ export const useDatePicker_unstable = (props: DatePickerProps, ref: React.Ref { + (newlySelectedDate?: Date | null): void => { if (open) { setOpen(false); @@ -460,7 +455,7 @@ export const useDatePicker_unstable = (props: DatePickerProps, ref: React.Ref { const styles = useStyles(); - const [selectedDate, setSelectedDate] = React.useState(new Date()); + const [selectedDate, setSelectedDate] = React.useState(null); const goPrevious = React.useCallback(() => { - setSelectedDate(prevSelectedDate => (prevSelectedDate ? addDays(prevSelectedDate, -1) : undefined)); + setSelectedDate(prevSelectedDate => (prevSelectedDate ? addDays(prevSelectedDate, -1) : null)); }, []); const goNext = React.useCallback(() => { - setSelectedDate(prevSelectedDate => (prevSelectedDate ? addDays(prevSelectedDate, 1) : undefined)); + setSelectedDate(prevSelectedDate => (prevSelectedDate ? addDays(prevSelectedDate, 1) : null)); }, []); return ( @@ -34,7 +34,7 @@ export const Controlled = () => { void} + onSelectDate={setSelectedDate} placeholder="Select a date..." className={styles.control} /> @@ -54,7 +54,9 @@ export const Controlled = () => { Controlled.parameters = { docs: { description: { - story: 'A DatePicker can be controlled by manually keeping track of the state and updating it.', + story: + 'A DatePicker can be controlled by manually keeping track of the state and updating it. When controlled,' + + ' the value prop should use null instead of undefined.', }, }, }; diff --git a/packages/react-components/react-datepicker-compat/stories/DatePicker/DatePickerCustomDateFormatting.stories.tsx b/packages/react-components/react-datepicker-compat/stories/DatePicker/DatePickerCustomDateFormatting.stories.tsx index e4de796c80b62..3e15e1f02ced7 100644 --- a/packages/react-components/react-datepicker-compat/stories/DatePicker/DatePickerCustomDateFormatting.stories.tsx +++ b/packages/react-components/react-datepicker-compat/stories/DatePicker/DatePickerCustomDateFormatting.stories.tsx @@ -24,11 +24,11 @@ const onFormatDate = (date?: Date): string => { export const CustomDateFormatting = () => { const styles = useStyles(); - const [value, setValue] = React.useState(); + const [value, setValue] = React.useState(null); const datePickerRef = React.useRef(null); const onClick = React.useCallback((): void => { - setValue(undefined); + setValue(null); datePickerRef.current?.focus(); }, []); From 2a4752ec0b41c11c50cdb9ea9a8e154287f7b3d4 Mon Sep 17 00:00:00 2001 From: Esteban Munoz Date: Tue, 30 May 2023 15:42:37 -0700 Subject: [PATCH 2/6] change file --- ...picker-compat-6da0cd91-e9c0-4bdc-b670-d3dca86fb9a8.json | 7 +++++++ 1 file changed, 7 insertions(+) create mode 100644 change/@fluentui-react-datepicker-compat-6da0cd91-e9c0-4bdc-b670-d3dca86fb9a8.json diff --git a/change/@fluentui-react-datepicker-compat-6da0cd91-e9c0-4bdc-b670-d3dca86fb9a8.json b/change/@fluentui-react-datepicker-compat-6da0cd91-e9c0-4bdc-b670-d3dca86fb9a8.json new file mode 100644 index 0000000000000..a2be4b6320f6c --- /dev/null +++ b/change/@fluentui-react-datepicker-compat-6da0cd91-e9c0-4bdc-b670-d3dca86fb9a8.json @@ -0,0 +1,7 @@ +{ + "type": "patch", + "comment": "feat: Add null to value prop for controlled cases to work correctly.", + "packageName": "@fluentui/react-datepicker-compat", + "email": "esteban.230@hotmail.com", + "dependentChangeType": "patch" +} From 74b3ab5e019803c2148e3cc8c53ccf365c559512 Mon Sep 17 00:00:00 2001 From: Esteban Munoz Date: Wed, 31 May 2023 13:29:34 -0700 Subject: [PATCH 3/6] fixing api file --- .../etc/react-datepicker-compat.api.md | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/packages/react-components/react-datepicker-compat/etc/react-datepicker-compat.api.md b/packages/react-components/react-datepicker-compat/etc/react-datepicker-compat.api.md index ff299deaf9d18..74dac022259fb 100644 --- a/packages/react-components/react-datepicker-compat/etc/react-datepicker-compat.api.md +++ b/packages/react-components/react-datepicker-compat/etc/react-datepicker-compat.api.md @@ -6,7 +6,14 @@ /// +import type { ComponentProps } from '@fluentui/react-utilities'; +import type { ComponentState } from '@fluentui/react-utilities'; +import type { ForwardRefComponent } from '@fluentui/react-utilities'; +import { Input } from '@fluentui/react-input'; +import type { PositioningProps } from '@fluentui/react-positioning'; import * as React_2 from 'react'; +import type { Slot } from '@fluentui/react-utilities'; +import type { SlotClassNames } from '@fluentui/react-utilities'; // @public export function addDays(date: Date, days: number): Date; From 022e698509deffb41f4ff23ad9ca0b2b8156ebd9 Mon Sep 17 00:00:00 2001 From: Esteban Munoz Date: Wed, 31 May 2023 14:47:28 -0700 Subject: [PATCH 4/6] updating lock file --- yarn.lock | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/yarn.lock b/yarn.lock index 3d8277cdbe1f2..756a7bfc7a2f9 100644 --- a/yarn.lock +++ b/yarn.lock @@ -23109,7 +23109,7 @@ semver@7.3.4: dependencies: lru-cache "^6.0.0" -semver@7.x, semver@^7.0.0, semver@^7.1.1, semver@^7.2.1, semver@^7.3.2, semver@^7.3.4, semver@^7.3.5, semver@^7.3.7, semver@~7.3.0, semver@^7.3.8: +semver@7.x, semver@^7.0.0, semver@^7.1.1, semver@^7.2.1, semver@^7.3.2, semver@^7.3.4, semver@^7.3.5, semver@^7.3.7, semver@^7.3.8, semver@~7.3.0: version "7.3.8" resolved "https://registry.yarnpkg.com/semver/-/semver-7.3.8.tgz#07a78feafb3f7b32347d725e33de7e2a2df67798" integrity sha512-NB1ctGL5rlHrPJtFDVIVzTyQylMLu9N9VICA6HSFJo8MCGVTMW6gfpicwKmmK/dAjTOrqu5l63JJOpDSrAis3A== @@ -23121,7 +23121,6 @@ semver@^6.0.0, semver@^6.1.0, semver@^6.1.1, semver@^6.1.2, semver@^6.2.0, semve resolved "https://registry.yarnpkg.com/semver/-/semver-6.3.0.tgz#ee0a64c8af5e8ceea67687b133761e1becbd1d3d" integrity sha512-b39TBaTSfV6yBrapU89p5fKekE2m/NwnDocOVruQFS1/veMgdzuPcnOM34M6CwxW8jH/lxEa5rBoDeUwu5HHTw== - send@0.17.2: version "0.17.2" resolved "https://registry.yarnpkg.com/send/-/send-0.17.2.tgz#926622f76601c41808012c8bf1688fe3906f7820" From 3a20225ca6a70ac931a5a59fc98eb247d2984a30 Mon Sep 17 00:00:00 2001 From: Esteban Munoz Date: Wed, 31 May 2023 14:50:21 -0700 Subject: [PATCH 5/6] revert yarn.lock --- yarn.lock | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/yarn.lock b/yarn.lock index 756a7bfc7a2f9..3d8277cdbe1f2 100644 --- a/yarn.lock +++ b/yarn.lock @@ -23109,7 +23109,7 @@ semver@7.3.4: dependencies: lru-cache "^6.0.0" -semver@7.x, semver@^7.0.0, semver@^7.1.1, semver@^7.2.1, semver@^7.3.2, semver@^7.3.4, semver@^7.3.5, semver@^7.3.7, semver@^7.3.8, semver@~7.3.0: +semver@7.x, semver@^7.0.0, semver@^7.1.1, semver@^7.2.1, semver@^7.3.2, semver@^7.3.4, semver@^7.3.5, semver@^7.3.7, semver@~7.3.0, semver@^7.3.8: version "7.3.8" resolved "https://registry.yarnpkg.com/semver/-/semver-7.3.8.tgz#07a78feafb3f7b32347d725e33de7e2a2df67798" integrity sha512-NB1ctGL5rlHrPJtFDVIVzTyQylMLu9N9VICA6HSFJo8MCGVTMW6gfpicwKmmK/dAjTOrqu5l63JJOpDSrAis3A== @@ -23121,6 +23121,7 @@ semver@^6.0.0, semver@^6.1.0, semver@^6.1.1, semver@^6.1.2, semver@^6.2.0, semve resolved "https://registry.yarnpkg.com/semver/-/semver-6.3.0.tgz#ee0a64c8af5e8ceea67687b133761e1becbd1d3d" integrity sha512-b39TBaTSfV6yBrapU89p5fKekE2m/NwnDocOVruQFS1/veMgdzuPcnOM34M6CwxW8jH/lxEa5rBoDeUwu5HHTw== + send@0.17.2: version "0.17.2" resolved "https://registry.yarnpkg.com/send/-/send-0.17.2.tgz#926622f76601c41808012c8bf1688fe3906f7820" From 6b67ea1ac36336f9a6b7da78e38eb7d35e982b6c Mon Sep 17 00:00:00 2001 From: Esteban Munoz Date: Wed, 31 May 2023 16:05:55 -0700 Subject: [PATCH 6/6] requested changes --- .../src/components/DatePicker/DatePicker.test.tsx | 6 ++---- .../src/components/DatePicker/useDatePicker.tsx | 8 +++----- .../stories/DatePicker/DatePickerControlled.stories.tsx | 2 +- 3 files changed, 6 insertions(+), 10 deletions(-) diff --git a/packages/react-components/react-datepicker-compat/src/components/DatePicker/DatePicker.test.tsx b/packages/react-components/react-datepicker-compat/src/components/DatePicker/DatePicker.test.tsx index 35304fe698997..0fca4ef12829c 100644 --- a/packages/react-components/react-datepicker-compat/src/components/DatePicker/DatePicker.test.tsx +++ b/packages/react-components/react-datepicker-compat/src/components/DatePicker/DatePicker.test.tsx @@ -158,7 +158,7 @@ describe('DatePicker', () => { it('calls onSelectDate when controlled', () => { const onSelectDate = jest.fn(); - const result = render( onSelectDate()} />); + const result = render(); fireEvent.click(result.getByRole('combobox')); result.getAllByRole('gridcell')[10].click(); @@ -169,9 +169,7 @@ describe('DatePicker', () => { it('calls onSelectDate and formatDate when controlled', () => { const onSelectDate = jest.fn(); const formatDate = jest.fn(); - const result = render( - formatDate()} onSelectDate={date => onSelectDate()} />, - ); + const result = render(); fireEvent.click(result.getByRole('combobox')); result.getAllByRole('gridcell')[10].click(); diff --git a/packages/react-components/react-datepicker-compat/src/components/DatePicker/useDatePicker.tsx b/packages/react-components/react-datepicker-compat/src/components/DatePicker/useDatePicker.tsx index 3c2509714b4d8..bd257f1f5dd7d 100644 --- a/packages/react-components/react-datepicker-compat/src/components/DatePicker/useDatePicker.tsx +++ b/packages/react-components/react-datepicker-compat/src/components/DatePicker/useDatePicker.tsx @@ -70,18 +70,16 @@ function useSelectedDate({ formatDate, onSelectDate, value }: DatePickerProps) { initialState: null, state: value, }); - const [formattedDate, setFormattedDate] = React.useState(() => - value !== null && value && formatDate ? formatDate(value) : '', - ); + const [formattedDate, setFormattedDate] = React.useState(() => (value && formatDate ? formatDate(value) : '')); const setSelectedDate = (newDate: Date | null | undefined) => { onSelectDate?.(newDate); setSelectedDateState(newDate); - setFormattedDate(newDate !== null && newDate && formatDate ? formatDate(newDate) : ''); + setFormattedDate(newDate && formatDate ? formatDate(newDate) : ''); }; React.useEffect(() => { - setFormattedDate(value !== null && value && formatDate ? formatDate(value) : ''); + setFormattedDate(value && formatDate ? formatDate(value) : ''); }, [formatDate, value]); return [selectedDate, formattedDate, setSelectedDate, setFormattedDate] as const; diff --git a/packages/react-components/react-datepicker-compat/stories/DatePicker/DatePickerControlled.stories.tsx b/packages/react-components/react-datepicker-compat/stories/DatePicker/DatePickerControlled.stories.tsx index 956783d539735..31062666a944e 100644 --- a/packages/react-components/react-datepicker-compat/stories/DatePicker/DatePickerControlled.stories.tsx +++ b/packages/react-components/react-datepicker-compat/stories/DatePicker/DatePickerControlled.stories.tsx @@ -56,7 +56,7 @@ Controlled.parameters = { description: { story: 'A DatePicker can be controlled by manually keeping track of the state and updating it. When controlled,' + - ' the value prop should use null instead of undefined.', + ' the value prop should use null instead of undefined to clear the value of the DatePicker.', }, }, };