From 480939074bc61a1bee3aa75246fe84bf5ece2d9d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Connor=20B=C3=A4r?= Date: Mon, 21 Oct 2024 19:49:15 +0200 Subject: [PATCH] Add more unit tests and improve validation hint --- .../components/DateInput/DateInput.module.css | 5 + .../components/DateInput/DateInput.spec.tsx | 294 +++++++++++++----- .../components/DateInput/DateInput.tsx | 35 ++- .../circuit-ui/components/DateInput/hooks.ts | 23 +- 4 files changed, 264 insertions(+), 93 deletions(-) diff --git a/packages/circuit-ui/components/DateInput/DateInput.module.css b/packages/circuit-ui/components/DateInput/DateInput.module.css index 623b68465c..ea827c1978 100644 --- a/packages/circuit-ui/components/DateInput/DateInput.module.css +++ b/packages/circuit-ui/components/DateInput/DateInput.module.css @@ -96,6 +96,11 @@ box-shadow: inset 0 0 0 1px var(--cui-border-focus); } +.calendar-button:active, +.calendar-button[aria-expanded="true"] { + z-index: calc(var(--cui-z-index-absolute) + 1); +} + .content { color: var(--cui-fg-normal); background-color: var(--cui-bg-elevated); diff --git a/packages/circuit-ui/components/DateInput/DateInput.spec.tsx b/packages/circuit-ui/components/DateInput/DateInput.spec.tsx index 2ede39fcf7..35a4b56c9a 100644 --- a/packages/circuit-ui/components/DateInput/DateInput.spec.tsx +++ b/packages/circuit-ui/components/DateInput/DateInput.spec.tsx @@ -24,28 +24,28 @@ import { DateInput } from './DateInput.js'; describe('DateInput', () => { const baseProps = { onChange: vi.fn(), - label: 'Date', - prevMonthButtonLabel: 'Previous month', - nextMonthButtonLabel: 'Previous month', - openCalendarButtonLabel: 'Change date', - closeCalendarButtonLabel: 'Close', - applyDateButtonLabel: 'Apply', - clearDateButtonLabel: 'Clear', + label: 'Date of birth', yearInputLabel: 'Year', monthInputLabel: 'Month', dayInputLabel: 'Day', + openCalendarButtonLabel: 'Change date', + closeCalendarButtonLabel: 'Close calendar', + prevMonthButtonLabel: 'Previous month', + nextMonthButtonLabel: 'Previous month', + applyDateButtonLabel: 'Apply date', + clearDateButtonLabel: 'Clear date', }; beforeAll(() => { MockDate.set('2000-01-01'); }); - // TODO: Move ref to outermost div? it('should forward a ref', () => { - const ref = createRef(); - render(); - const fieldset = screen.getByRole('group'); - expect(ref.current).toBe(fieldset); + const ref = createRef(); + const { container } = render(); + // eslint-disable-next-line testing-library/no-container + const wrapper = container.querySelectorAll('div')[0]; + expect(ref.current).toBe(wrapper); }); it('should merge a custom class name with the default ones', () => { @@ -58,93 +58,239 @@ describe('DateInput', () => { expect(wrapper?.className).toContain(className); }); - it('should select a calendar date', async () => { - const onChange = vi.fn(); + describe('semantics', () => { + it('should optionally have an accessible description', () => { + const description = 'Description'; + render(); + const fieldset = screen.getByRole('group'); + const inputs = screen.getAllByRole('spinbutton'); + + expect(fieldset).toHaveAccessibleDescription(description); + expect(inputs[0]).toHaveAccessibleDescription(description); + expect(inputs[1]).not.toHaveAccessibleDescription(); + expect(inputs[2]).not.toHaveAccessibleDescription(); + }); - render(); + it('should accept a custom description via aria-describedby', () => { + const customDescription = 'Custom description'; + const customDescriptionId = 'customDescriptionId'; + render( + <> + , + {customDescription} + , + ); + const fieldset = screen.getByRole('group'); + const inputs = screen.getAllByRole('spinbutton'); - const openCalendarButton = screen.getByRole('button', { - name: /change date/i, + expect(fieldset).toHaveAccessibleDescription(customDescription); + expect(inputs[0]).toHaveAccessibleDescription(customDescription); + expect(inputs[1]).not.toHaveAccessibleDescription(); + expect(inputs[2]).not.toHaveAccessibleDescription(); }); - await userEvent.click(openCalendarButton); + it('should accept a custom description in addition to a validationHint', () => { + const customDescription = 'Custom description'; + const customDescriptionId = 'customDescriptionId'; + const description = 'Description'; + render( + <> + + {customDescription}, + , + ); + const fieldset = screen.getByRole('group'); + const inputs = screen.getAllByRole('spinbutton'); - const calendarDialog = screen.getByRole('dialog'); + expect(fieldset).toHaveAccessibleDescription( + `${customDescription} ${description}`, + ); + expect(inputs[0]).toHaveAccessibleDescription( + `${customDescription} ${description}`, + ); + expect(inputs[1]).not.toHaveAccessibleDescription(); + expect(inputs[2]).not.toHaveAccessibleDescription(); + }); - expect(calendarDialog).toBeVisible(); + it('should render as disabled', async () => { + render(); + expect(screen.getByLabelText(/day/i)).toBeDisabled(); + expect(screen.getByLabelText(/month/i)).toBeDisabled(); + expect(screen.getByLabelText(/year/i)).toBeDisabled(); + expect( + screen.getByRole('button', { name: baseProps.openCalendarButtonLabel }), + ).toHaveAttribute('aria-disabled', 'true'); + }); - const dateButton = screen.getByRole('button', { name: /12/ }); + it('should render as read-only', async () => { + render(); + expect(screen.getByLabelText(/day/i)).toHaveAttribute('readonly'); + expect(screen.getByLabelText(/month/i)).toHaveAttribute('readonly'); + expect(screen.getByLabelText(/year/i)).toHaveAttribute('readonly'); + expect( + screen.getByRole('button', { name: baseProps.openCalendarButtonLabel }), + ).toHaveAttribute('aria-disabled', 'true'); + }); - await userEvent.click(dateButton); + it('should render as invalid', async () => { + render(); + expect(screen.getByLabelText(/day/i)).toBeInvalid(); + expect(screen.getByLabelText(/month/i)).toBeInvalid(); + expect(screen.getByLabelText(/year/i)).toBeInvalid(); + }); - expect(onChange).toHaveBeenCalledWith('2000-01-12'); - }); + it('should render as required', async () => { + render(); + expect(screen.getByLabelText(/day/i)).toBeRequired(); + expect(screen.getByLabelText(/month/i)).toBeRequired(); + expect(screen.getByLabelText(/year/i)).toBeRequired(); + }); - it('should display the initial value correctly', () => { - render(); + it('should have relevant minimum input values', () => { + render(); + expect(screen.getByLabelText(/day/i)).toHaveAttribute('min', '1'); + expect(screen.getByLabelText(/month/i)).toHaveAttribute('min', '1'); + expect(screen.getByLabelText(/year/i)).toHaveAttribute('min', '2000'); + }); - expect(screen.getByLabelText(/day/i)).toHaveValue(12); - expect(screen.getByLabelText(/month/i)).toHaveValue(1); - expect(screen.getByLabelText(/year/i)).toHaveValue(2000); - }); + it('should have relevant maximum input values', () => { + render(); + expect(screen.getByLabelText(/day/i)).toHaveAttribute('max', '31'); + expect(screen.getByLabelText(/month/i)).toHaveAttribute('max', '12'); + expect(screen.getByLabelText(/year/i)).toHaveAttribute('max', '2001'); + }); - it('should render a disabled input', () => { - render(); - expect(screen.getByLabelText(/day/i)).toBeDisabled(); - expect(screen.getByLabelText(/month/i)).toBeDisabled(); - expect(screen.getByLabelText(/year/i)).toBeDisabled(); - expect( - screen.getByRole('button', { name: baseProps.openCalendarButtonLabel }), - ).toHaveAttribute('aria-disabled', 'true'); - }); + it('should mark the year input as readonly when the minimum and maximum dates have the same year', () => { + render(); + expect(screen.getByLabelText(/year/i)).toHaveAttribute('readonly'); + expect(screen.getByLabelText(/month/i)).toHaveAttribute('min', '4'); + expect(screen.getByLabelText(/month/i)).toHaveAttribute('max', '6'); + }); - it('should handle min/max dates', () => { - render(); - expect(screen.getByLabelText(/day/i)).toHaveAttribute('min', '1'); - expect(screen.getByLabelText(/day/i)).toHaveAttribute('max', '31'); - expect(screen.getByLabelText(/month/i)).toHaveAttribute('min', '1'); - expect(screen.getByLabelText(/month/i)).toHaveAttribute('max', '12'); - expect(screen.getByLabelText(/year/i)).toHaveAttribute('min', '2000'); - expect(screen.getByLabelText(/year/i)).toHaveAttribute('max', '2001'); + it('should mark the year and month inputs as readonly when the minimum and maximum dates have the same year and month', () => { + render(); + expect(screen.getByLabelText(/year/i)).toHaveAttribute('readonly'); + expect(screen.getByLabelText(/month/i)).toHaveAttribute('readonly'); + expect(screen.getByLabelText(/day/i)).toHaveAttribute('min', '9'); + expect(screen.getByLabelText(/day/i)).toHaveAttribute('max', '27'); + }); }); - it('should handle min/max dates as the user types year', async () => { - render(); + describe('state', () => { + it('should display a default value', () => { + render(); - await userEvent.type(screen.getByLabelText(/year/i), '2001'); - /* expect(screen.getByLabelText(/day/i)).toHaveAttribute('min', '1'); - expect(screen.getByLabelText(/day/i)).toHaveAttribute('max', '1'); */ - expect(screen.getByLabelText(/month/i)).toHaveAttribute('min', '1'); - expect(screen.getByLabelText(/month/i)).toHaveAttribute('max', '2'); - }); + expect(screen.getByLabelText(/day/i)).toHaveValue(12); + expect(screen.getByLabelText(/month/i)).toHaveValue(1); + expect(screen.getByLabelText(/year/i)).toHaveValue(2000); + }); - it('should handle min/max dates as the user types year and month', async () => { - render(); + it('should display an initial value', () => { + render(); - await userEvent.type(screen.getByLabelText(/year/i), '2001'); - await userEvent.type(screen.getByLabelText(/month/i), '02'); + expect(screen.getByLabelText(/day/i)).toHaveValue(12); + expect(screen.getByLabelText(/month/i)).toHaveValue(1); + expect(screen.getByLabelText(/year/i)).toHaveValue(2000); + }); - expect(screen.getByLabelText(/day/i)).toHaveAttribute('min', '1'); - expect(screen.getByLabelText(/day/i)).toHaveAttribute('max', '15'); - }); + it('should update the displayed value', () => { + const { rerender } = render( + , + ); - it('years field should be readonly if min/max dates have the same year', () => { - render(); - expect(screen.getByLabelText(/year/i)).toHaveAttribute('readonly'); - expect(screen.getByLabelText(/month/i)).toHaveAttribute('min', '4'); - expect(screen.getByLabelText(/month/i)).toHaveAttribute('max', '6'); + rerender(); + + expect(screen.getByLabelText(/day/i)).toHaveValue(15); + expect(screen.getByLabelText(/month/i)).toHaveValue(1); + expect(screen.getByLabelText(/year/i)).toHaveValue(2000); + }); }); - it('years and months fields should render as readonly if min/max dates have the same year and same month', () => { - render(); - expect(screen.getByLabelText(/year/i)).toHaveAttribute('readonly'); - expect(screen.getByLabelText(/month/i)).toHaveAttribute('readonly'); + describe('user interactions', () => { + it('should focus the first input when clicking the label', async () => { + render(); + + await userEvent.click(screen.getByText('Date of birth')); + + expect(screen.getAllByRole('spinbutton')[0]).toHaveFocus(); + }); + + it('should allow users to type a date', async () => { + const onChange = vi.fn(); + + render(); + + await userEvent.type(screen.getByLabelText('Year'), '2017'); + await userEvent.type(screen.getByLabelText('Month'), '8'); + await userEvent.type(screen.getByLabelText('Day'), '28'); + + expect(onChange).toHaveBeenCalledWith('2017-08-28'); + }); + + it('should update the minimum and maximum input values as the user types', async () => { + render(); + + await userEvent.type(screen.getByLabelText(/year/i), '2001'); + + expect(screen.getByLabelText(/month/i)).toHaveAttribute('min', '1'); + expect(screen.getByLabelText(/month/i)).toHaveAttribute('max', '2'); + + await userEvent.type(screen.getByLabelText(/month/i), '2'); + + expect(screen.getByLabelText(/day/i)).toHaveAttribute('min', '1'); + expect(screen.getByLabelText(/day/i)).toHaveAttribute('max', '15'); + }); + + it('should allow users to select a date on a calendar', async () => { + const onChange = vi.fn(); + + render(); + + const openCalendarButton = screen.getByRole('button', { + name: /change date/i, + }); + await userEvent.click(openCalendarButton); + + const calendarDialog = screen.getByRole('dialog'); + expect(calendarDialog).toBeVisible(); + + const dateButton = screen.getByRole('button', { name: /12/ }); + await userEvent.click(dateButton); + + expect(onChange).toHaveBeenCalledWith('2000-01-12'); + }); - expect(screen.getByLabelText(/day/i)).toHaveAttribute('min', '9'); - expect(screen.getByLabelText(/day/i)).toHaveAttribute('max', '27'); + it('should allow users to clear the date', async () => { + const onChange = vi.fn(); + + render( + , + ); + + const openCalendarButton = screen.getByRole('button', { + name: /change date/i, + }); + await userEvent.click(openCalendarButton); + + const calendarDialog = screen.getByRole('dialog'); + expect(calendarDialog).toBeVisible(); + + const clearButton = screen.getByRole('button', { name: /clear/ }); + await userEvent.click(clearButton); + + expect(onChange).toHaveBeenCalledWith(''); + }); }); - describe('Status messages', () => { + describe('status messages', () => { it('should render an empty live region on mount', () => { render(); const liveRegionEl = screen.getByRole('status'); diff --git a/packages/circuit-ui/components/DateInput/DateInput.tsx b/packages/circuit-ui/components/DateInput/DateInput.tsx index f92bd719d0..faae5a4cf0 100644 --- a/packages/circuit-ui/components/DateInput/DateInput.tsx +++ b/packages/circuit-ui/components/DateInput/DateInput.tsx @@ -170,6 +170,7 @@ export const DateInput = forwardRef( hasWarning, showValid, validationHint, + 'aria-describedby': descriptionId, optionalLabel, openCalendarButtonLabel, closeCalendarButtonLabel, @@ -191,9 +192,12 @@ export const DateInput = forwardRef( const floatingRef = useRef(null); const calendarRef = useRef(null); + const dialogId = useId(); const headlineId = useId(); const validationHintId = useId(); + const descriptionIds = clsx(descriptionId, validationHintId); + const minDate = toPlainDate(min); const maxDate = toPlainDate(max); @@ -241,12 +245,8 @@ export const DateInput = forwardRef( // Focus the first date segment when clicking anywhere on the field... const handleClick = (event: ClickEvent) => { const element = event.target as HTMLElement; - // ...except when clicking on a specific segment or the calendar button. - if ( - element.tagName === 'INPUT' || - element.tagName === 'BUTTON' || - element.matches('button :scope') - ) { + // ...except when clicking on a specific segment. + if (element.tagName === 'INPUT') { return; } focusHandlers.next(); @@ -294,6 +294,12 @@ export const DateInput = forwardRef( const dialogStyles = isMobile ? mobileStyles : floatingStyles; if (process.env.NODE_ENV !== 'production') { + if (!isSufficientlyLabelled(label)) { + throw new AccessibilityError( + 'DateInput', + 'The `label` prop is missing or invalid.', + ); + } if (!isSufficientlyLabelled(openCalendarButtonLabel)) { throw new AccessibilityError( 'DateInput', @@ -359,8 +365,8 @@ export const DateInput = forwardRef( readOnly={readOnly} {...props} /> */} -
- +
+ ( optionalLabel={optionalLabel} /> -
+ {/* biome-ignore lint/a11y/useKeyWithClickEvents: */} +
( ref={referenceRef} > {segments.map((segment, index) => { + // Only the first segment should be associated with the validation hint to reduce verbosity. + const validationProps = + index === 0 ? { 'aria-describedby': descriptionIds } : {}; switch (segment.type) { case 'year': return ( @@ -392,6 +402,7 @@ export const DateInput = forwardRef( autoComplete={ autoComplete === 'bday' ? 'bday-year' : undefined } + {...validationProps} {...focusProps} {...yearProps} /> @@ -408,6 +419,7 @@ export const DateInput = forwardRef( autoComplete={ autoComplete === 'bday' ? 'bday-month' : undefined } + {...validationProps} {...focusProps} {...monthProps} /> @@ -424,6 +436,7 @@ export const DateInput = forwardRef( autoComplete={ autoComplete === 'bday' ? 'bday-day' : undefined } + {...validationProps} {...focusProps} {...dayProps} /> @@ -450,6 +463,9 @@ export const DateInput = forwardRef( onClick={openCalendar} className={classes['calendar-button']} disabled={disabled || readOnly} + aria-expanded={open} + aria-haspopup="true" + aria-controls={dialogId} > {calendarButtonLabel} @@ -465,6 +481,7 @@ export const DateInput = forwardRef(
999 && isNumber(month) && isNumber(day)) { + if (isNumber(year) && isNumber(month) && isNumber(day)) { const plainDate = new Temporal.PlainDate(year, month, day); onChange?.(plainDate.toString()); } else { @@ -171,9 +171,10 @@ export function useYearSegment( const onChange = (event: FormEvent) => { const year = Number.parseInt(event.currentTarget.value, 10) || ''; state.update({ year }); - if (year && year > 999) { - focus.next(); - } + // FIXME: Don't advance when changing the value using arrow keys + // if (year && year > 999) { + // focus.next(); + // } }; return { ...props, value, placeholder, min, max, onChange }; @@ -219,9 +220,10 @@ export function useMonthSegment( const onChange = (event: FormEvent) => { const month = Number.parseInt(event.currentTarget.value, 10) || ''; state.update({ month }); - if (month && month > 1) { - focus.next(); - } + // FIXME: Don't advance when changing the value using arrow keys + // if (month && month > 1) { + // focus.next(); + // } }; return { ...props, value, placeholder, min, max, onChange }; @@ -277,9 +279,10 @@ export function useDaySegment( const onChange = (event: FormEvent) => { const day = Number.parseInt(event.currentTarget.value, 10) || ''; state.update({ day }); - if (day && day > 3) { - focus.next(); - } + // FIXME: Don't advance when changing the value using arrow keys + // if (day && day > 3) { + // focus.next(); + // } }; return { ...props, value, placeholder, min, max, onChange };