From 2cd5c7b094d49d5ad4f7e5c63207658f610d17e1 Mon Sep 17 00:00:00 2001 From: Eric Olkowski Date: Tue, 18 Jul 2023 11:48:54 -0400 Subject: [PATCH 1/3] fix(DatePicker): updated onBlur logic for empty date --- .../src/components/DatePicker/DatePicker.tsx | 34 +++++-- .../DatePicker/__tests__/DatePicker.test.tsx | 89 ++++++++++++++++++- .../DatePicker/examples/DatePicker.md | 17 ++++ .../examples/DatePickerRequired.tsx | 6 ++ 4 files changed, 136 insertions(+), 10 deletions(-) create mode 100644 packages/react-core/src/components/DatePicker/examples/DatePickerRequired.tsx diff --git a/packages/react-core/src/components/DatePicker/DatePicker.tsx b/packages/react-core/src/components/DatePicker/DatePicker.tsx index a10f4f9640e..47e38efae17 100644 --- a/packages/react-core/src/components/DatePicker/DatePicker.tsx +++ b/packages/react-core/src/components/DatePicker/DatePicker.tsx @@ -32,7 +32,7 @@ export interface DatePickerProps className?: string; /** How to format the date in the text input. */ dateFormat?: (date: Date) => string; - /** How to format the date in the text input. */ + /** How to parse the date in the text input. */ dateParse?: (value: string) => Date; /** Helper text to display alongside the date picker. Expects a HelperText component. */ helperText?: React.ReactNode; @@ -40,8 +40,12 @@ export interface DatePickerProps inputProps?: TextInputProps; /** Flag indicating the date picker is disabled. */ isDisabled?: boolean; - /** Error message to display when the text input cannot be parsed. */ + /** Flag indicating the date picker is required. */ + isRequired?: boolean; + /** Error message to display when the text input contains a non-empty value in an invalid format. */ invalidFormatText?: string; + /** Error message to display when the text input is empty and the isRequired prop is also passed in. */ + emptyDateText?: string; /** Callback called every time the text input loses focus. */ onBlur?: (event: any, value: string, date?: Date) => void; /** Callback called every time the text input value changes. */ @@ -88,6 +92,7 @@ const DatePickerBase = ( dateFormat = yyyyMMddFormat, dateParse = (val: string) => val.split('-').length === 3 && new Date(`${val}T00:00:00`), isDisabled = false, + isRequired = false, placeholder = 'YYYY-MM-DD', value: valueProp = '', 'aria-label': ariaLabel = 'Date picker', @@ -95,6 +100,7 @@ const DatePickerBase = ( onChange = (): any => undefined, onBlur = (): any => undefined, invalidFormatText = 'Invalid date', + emptyDateText = 'Date cannot be blank', helperText, appendTo = 'inline', popoverProps, @@ -153,17 +159,22 @@ const DatePickerBase = ( }; const onInputBlur = (event: any) => { - if (pristine) { - return; - } const newValueDate = dateParse(value); - if (isValidDate(newValueDate)) { - onBlur(event, value, new Date(newValueDate)); + const dateIsValid = isValidDate(newValueDate); + const onBlurDateArg = dateIsValid ? new Date(newValueDate) : undefined; + onBlur(event, value, onBlurDateArg); + + if (dateIsValid) { setError(newValueDate); - } else { - onBlur(event, value); + } + + if (!dateIsValid && !pristine) { setErrorText(invalidFormatText); } + + if (!dateIsValid && pristine && isRequired) { + setErrorText(emptyDateText); + } }; const onDateClick = (_event: React.MouseEvent, newValueDate: Date) => { @@ -236,6 +247,10 @@ const DatePickerBase = ( event.stopPropagation(); setPopoverOpen(false); hideFunction(); + // If datepicker is required and the popover is opened without the text input + // first receiving focus, we want to validate that the text input is not blank upon + // closing the popover + isRequired && !value && setErrorText(emptyDateText); } if (event.key === KeyTypes.Escape && popoverOpen) { event.stopPropagation(); @@ -254,6 +269,7 @@ const DatePickerBase = ( { expect(screen.getByText('Help me')).toBeVisible(); }); -test('Shows "Invalid date" instead of helperText when an error exists', async () => { +test('Shows "Invalid date" instead of helperText when text input contains invalid date', async () => { const user = userEvent.setup(); render( @@ -99,3 +99,90 @@ test('Shows "Invalid date" instead of helperText when an error exists', async () expect(screen.queryByText('Help me')).not.toBeInTheDocument(); expect(screen.getByText('Invalid date')).toBeVisible(); }); + +test('Does not render text input as invalid when isRequired is false', async () => { + const user = userEvent.setup(); + + render(); + + await user.click(screen.getByRole('textbox')); + await user.click(document.body); + + expect(screen.getByRole('textbox')).not.toHaveAttribute('aria-invalid', 'true'); +}); + +test('Does not render emptyDateText when isRequired is false', async () => { + const user = userEvent.setup(); + + render(); + + await user.click(screen.getByRole('textbox')); + await user.click(document.body); + + expect(screen.queryByText('Date cannot be blank')).not.toBeInTheDocument; +}); + +test('Renders text input as invalid on blur when isRequired is false', async () => { + const user = userEvent.setup(); + + render(); + + await user.click(screen.getByRole('textbox')); + await user.click(document.body); + + expect(screen.getByRole('textbox')).toHaveAttribute('aria-invalid', 'true'); +}); + +test('Renders default emptyDateText on blur when isRequired is passed', async () => { + const user = userEvent.setup(); + + render(); + + await user.click(screen.getByRole('textbox')); + await user.click(document.body); + + expect(screen.getByText('Date cannot be blank')).toBeInTheDocument(); +}); + +test('Renders custom emptyDateText when isRequired is true', async () => { + const user = userEvent.setup(); + + render(); + + await user.click(screen.getByRole('textbox')); + await user.click(document.body); + + expect(screen.getByText('Required in test')).toBeInTheDocument(); +}); + +test('Shows emptyDateText instead of helperText when text input is empty and isRequired is true', async () => { + const user = userEvent.setup(); + + render( + + Help me + + } + /> + ); + + await user.click(screen.getByRole('textbox')); + await user.click(document.body); + + expect(screen.queryByText('Help me')).not.toBeInTheDocument(); + expect(screen.getByText('Date cannot be blank')).toBeVisible(); +}); + +test('Renders text input as invalid when isRequired is false and popover is closed without selection', async () => { + const user = userEvent.setup(); + + render(); + + await user.click(screen.getByRole('button', { name: 'Toggle date picker' })); + await user.click(document.body); + + expect(screen.getByRole('textbox')).toHaveAttribute('aria-invalid', 'true'); +}); diff --git a/packages/react-core/src/components/DatePicker/examples/DatePicker.md b/packages/react-core/src/components/DatePicker/examples/DatePicker.md index 6648cc1a33e..78ad3062bcd 100644 --- a/packages/react-core/src/components/DatePicker/examples/DatePicker.md +++ b/packages/react-core/src/components/DatePicker/examples/DatePicker.md @@ -11,34 +11,51 @@ propComponents: ['DatePicker', 'CalendarFormat', 'DatePickerRef'] ### Basic ```ts file="./DatePickerBasic.tsx" + +``` + +### Required + +A datepicker with the `isRequired` property passed in will be invalid when the text input is empty and either the text input loses focus or the datepicker popover is closed. + +The error message can be customized via the `emptyDateText` property. + +```ts file="./DatePickerRequired.tsx" + ``` ### American format ```ts file="./DatePickerAmerican.tsx" + ``` ### Helper text ```ts file="./DatePickerHelperText.tsx" + ``` ### Min and max date ```ts file="./DatePickerMinMax.tsx" + ``` ### French ```ts file="./DatePickerFrench.tsx" + ``` ### Controlled ```ts file="./DatePickerControlled.tsx" + ``` ### Controlling the date picker calendar state ```ts file="./DatePickerControlledCalendar.tsx" + ``` diff --git a/packages/react-core/src/components/DatePicker/examples/DatePickerRequired.tsx b/packages/react-core/src/components/DatePicker/examples/DatePickerRequired.tsx new file mode 100644 index 00000000000..f2d6b0a89e9 --- /dev/null +++ b/packages/react-core/src/components/DatePicker/examples/DatePickerRequired.tsx @@ -0,0 +1,6 @@ +import React from 'react'; +import { DatePicker } from '@patternfly/react-core'; + +export const DatePickerRequired: React.FunctionComponent = () => ( + +); From c54b673070865d97f1ab9525dd50e4fb067b6c59 Mon Sep 17 00:00:00 2001 From: Eric Olkowski Date: Fri, 21 Jul 2023 10:08:14 -0400 Subject: [PATCH 2/3] Updated example description --- .../src/components/DatePicker/examples/DatePicker.md | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/packages/react-core/src/components/DatePicker/examples/DatePicker.md b/packages/react-core/src/components/DatePicker/examples/DatePicker.md index 78ad3062bcd..a3961ecd23c 100644 --- a/packages/react-core/src/components/DatePicker/examples/DatePicker.md +++ b/packages/react-core/src/components/DatePicker/examples/DatePicker.md @@ -16,7 +16,9 @@ propComponents: ['DatePicker', 'CalendarFormat', 'DatePickerRef'] ### Required -A datepicker with the `isRequired` property passed in will be invalid when the text input is empty and either the text input loses focus or the datepicker popover is closed. +To require users to select a date before continuing, use the `isRequired` property. + +A required date picker will be invalid when the text input is empty and either the text input loses focus or the date picker popover is closed. The error message can be customized via the `emptyDateText` property. From c62c19c0d3556d1385f4babcfd63ee3517dc1bbf Mon Sep 17 00:00:00 2001 From: Eric Olkowski Date: Tue, 8 Aug 2023 09:21:16 -0400 Subject: [PATCH 3/3] Grouped isRequired and emptyDateText props as one --- .../src/components/DatePicker/DatePicker.tsx | 24 ++++++++++++------- .../DatePicker/__tests__/DatePicker.test.tsx | 24 +++++++++---------- .../DatePicker/examples/DatePicker.md | 6 ++--- .../examples/DatePickerRequired.tsx | 2 +- 4 files changed, 31 insertions(+), 25 deletions(-) diff --git a/packages/react-core/src/components/DatePicker/DatePicker.tsx b/packages/react-core/src/components/DatePicker/DatePicker.tsx index 47e38efae17..4ab92e187dc 100644 --- a/packages/react-core/src/components/DatePicker/DatePicker.tsx +++ b/packages/react-core/src/components/DatePicker/DatePicker.tsx @@ -12,6 +12,14 @@ import { KeyTypes } from '../../helpers'; import { isValidDate } from '../../helpers/datetimeUtils'; import { HelperText, HelperTextItem } from '../HelperText'; +/** Props that customize the requirement of a date */ +export interface DatePickerRequiredObject { + /** Flag indicating the date is required. */ + isRequired?: boolean; + /** Error message to display when the text input is empty and the isRequired prop is also passed in. */ + emptyDateText?: string; +} + /** The main date picker component. */ export interface DatePickerProps @@ -40,12 +48,8 @@ export interface DatePickerProps inputProps?: TextInputProps; /** Flag indicating the date picker is disabled. */ isDisabled?: boolean; - /** Flag indicating the date picker is required. */ - isRequired?: boolean; /** Error message to display when the text input contains a non-empty value in an invalid format. */ invalidFormatText?: string; - /** Error message to display when the text input is empty and the isRequired prop is also passed in. */ - emptyDateText?: string; /** Callback called every time the text input loses focus. */ onBlur?: (event: any, value: string, date?: Date) => void; /** Callback called every time the text input value changes. */ @@ -54,6 +58,8 @@ export interface DatePickerProps placeholder?: string; /** Props to pass to the popover that contains the calendar month component. */ popoverProps?: Partial>; + /** Options to customize the requirement of a date */ + requiredDateOptions?: DatePickerRequiredObject; /** Functions that returns an error message if a date is invalid. */ validators?: ((date: Date) => string)[]; /** Value of the text input. */ @@ -92,7 +98,6 @@ const DatePickerBase = ( dateFormat = yyyyMMddFormat, dateParse = (val: string) => val.split('-').length === 3 && new Date(`${val}T00:00:00`), isDisabled = false, - isRequired = false, placeholder = 'YYYY-MM-DD', value: valueProp = '', 'aria-label': ariaLabel = 'Date picker', @@ -100,7 +105,7 @@ const DatePickerBase = ( onChange = (): any => undefined, onBlur = (): any => undefined, invalidFormatText = 'Invalid date', - emptyDateText = 'Date cannot be blank', + requiredDateOptions, helperText, appendTo = 'inline', popoverProps, @@ -128,6 +133,7 @@ const DatePickerBase = ( const buttonRef = React.useRef(); const datePickerWrapperRef = React.useRef(); const triggerRef = React.useRef(); + const emptyDateText = requiredDateOptions?.emptyDateText || 'Date cannot be blank'; React.useEffect(() => { setValue(valueProp); @@ -172,7 +178,7 @@ const DatePickerBase = ( setErrorText(invalidFormatText); } - if (!dateIsValid && pristine && isRequired) { + if (!dateIsValid && pristine && requiredDateOptions?.isRequired) { setErrorText(emptyDateText); } }; @@ -250,7 +256,7 @@ const DatePickerBase = ( // If datepicker is required and the popover is opened without the text input // first receiving focus, we want to validate that the text input is not blank upon // closing the popover - isRequired && !value && setErrorText(emptyDateText); + requiredDateOptions?.isRequired && !value && setErrorText(emptyDateText); } if (event.key === KeyTypes.Escape && popoverOpen) { event.stopPropagation(); @@ -269,7 +275,7 @@ const DatePickerBase = ( { +test('Does not render text input as invalid when requiredDateOptions.isRequired is false', async () => { const user = userEvent.setup(); render(); @@ -111,7 +111,7 @@ test('Does not render text input as invalid when isRequired is false', async () expect(screen.getByRole('textbox')).not.toHaveAttribute('aria-invalid', 'true'); }); -test('Does not render emptyDateText when isRequired is false', async () => { +test('Does not render emptyDateText when requiredDateOptions.isRequired is false', async () => { const user = userEvent.setup(); render(); @@ -122,10 +122,10 @@ test('Does not render emptyDateText when isRequired is false', async () => { expect(screen.queryByText('Date cannot be blank')).not.toBeInTheDocument; }); -test('Renders text input as invalid on blur when isRequired is false', async () => { +test('Renders text input as invalid on blur when requiredDateOptions.isRequired is true', async () => { const user = userEvent.setup(); - render(); + render(); await user.click(screen.getByRole('textbox')); await user.click(document.body); @@ -133,10 +133,10 @@ test('Renders text input as invalid on blur when isRequired is false', async () expect(screen.getByRole('textbox')).toHaveAttribute('aria-invalid', 'true'); }); -test('Renders default emptyDateText on blur when isRequired is passed', async () => { +test('Renders default emptyDateText on blur when requiredDateOptions.isRequired is true', async () => { const user = userEvent.setup(); - render(); + render(); await user.click(screen.getByRole('textbox')); await user.click(document.body); @@ -144,10 +144,10 @@ test('Renders default emptyDateText on blur when isRequired is passed', async () expect(screen.getByText('Date cannot be blank')).toBeInTheDocument(); }); -test('Renders custom emptyDateText when isRequired is true', async () => { +test('Renders custom emptyDateText when requiredDateOptions.isRequired is true', async () => { const user = userEvent.setup(); - render(); + render(); await user.click(screen.getByRole('textbox')); await user.click(document.body); @@ -155,12 +155,12 @@ test('Renders custom emptyDateText when isRequired is true', async () => { expect(screen.getByText('Required in test')).toBeInTheDocument(); }); -test('Shows emptyDateText instead of helperText when text input is empty and isRequired is true', async () => { +test('Shows emptyDateText instead of helperText when text input is empty and requiredDateOptions.isRequired is true', async () => { const user = userEvent.setup(); render( Help me @@ -176,10 +176,10 @@ test('Shows emptyDateText instead of helperText when text input is empty and isR expect(screen.getByText('Date cannot be blank')).toBeVisible(); }); -test('Renders text input as invalid when isRequired is false and popover is closed without selection', async () => { +test('Renders text input as invalid when requiredDateOptions.isRequired is true and popover is closed without selection', async () => { const user = userEvent.setup(); - render(); + render(); await user.click(screen.getByRole('button', { name: 'Toggle date picker' })); await user.click(document.body); diff --git a/packages/react-core/src/components/DatePicker/examples/DatePicker.md b/packages/react-core/src/components/DatePicker/examples/DatePicker.md index a3961ecd23c..4d5959b29b0 100644 --- a/packages/react-core/src/components/DatePicker/examples/DatePicker.md +++ b/packages/react-core/src/components/DatePicker/examples/DatePicker.md @@ -3,7 +3,7 @@ id: Date picker section: components subsection: date-and-time cssPrefix: pf-v5-c-date-picker -propComponents: ['DatePicker', 'CalendarFormat', 'DatePickerRef'] +propComponents: ['DatePicker', 'CalendarFormat', 'DatePickerRef', 'DatePickerRequiredObject'] --- ## Examples @@ -16,11 +16,11 @@ propComponents: ['DatePicker', 'CalendarFormat', 'DatePickerRef'] ### Required -To require users to select a date before continuing, use the `isRequired` property. +To require users to select a date before continuing, use the `requiredDateOptions.isRequired` property. A required date picker will be invalid when the text input is empty and either the text input loses focus or the date picker popover is closed. -The error message can be customized via the `emptyDateText` property. +The error message can be customized via the `requiredDateOptions.emptyDateText` property. ```ts file="./DatePickerRequired.tsx" diff --git a/packages/react-core/src/components/DatePicker/examples/DatePickerRequired.tsx b/packages/react-core/src/components/DatePicker/examples/DatePickerRequired.tsx index f2d6b0a89e9..fbeb0f1febb 100644 --- a/packages/react-core/src/components/DatePicker/examples/DatePickerRequired.tsx +++ b/packages/react-core/src/components/DatePicker/examples/DatePickerRequired.tsx @@ -2,5 +2,5 @@ import React from 'react'; import { DatePicker } from '@patternfly/react-core'; export const DatePickerRequired: React.FunctionComponent = () => ( - + );