From e228ed814b58e162779fb8c2f7270c8b8e844c15 Mon Sep 17 00:00:00 2001 From: Yihui Liao <44729383+yihuiliao@users.noreply.github.com> Date: Tue, 28 Jan 2025 20:06:50 -0500 Subject: [PATCH] fix: correctly format date/time in RTL (#7423) * bdo on timefield, reverse segments on timefield in datefield * fix lint * make things inline * use unicode character to wrap segments * fix test * append unicode to text in hooks, update rac * add comment * skip failing test for now * update keyboard nav * update logic of how unicode is applied * fix spacing * add comments * update tests * undo some previous changes * wrap time segments in lri, wrap fields in unicode isolate * fix ssr test * fix spacing * fix css logic * fix lint * fix keyboard nav in rac datepicker popover * fix lint * prevent overflow in date range picker * move overflow hidden to separate new div to fix weird focus ring around the button * this time actually fix the overflow and focus ring issue * update var names to be nicer * fix japanese placeholder for extra space * fix css positioning * fix custom width * small css changes so that rtl will format properly * memo ordering of segments for keyboard navigation * add chromatic tests * fix lint * add tests to rsp date components * add tests to rac * fix tests * remove comment * fix chromatic stories * add chromatic story * remove style tests * fix lint * update to uselayouteffect and update keyboard nav test * make date input more consistent with using display inline * update timefield docs css to use display inline * fix showFormatHelpText * small change * fix lint * add divs to keyboard navigation so it works with older versions * fix lint + fix tests --- .../datepicker/docs/useDateField.mdx | 6 +- .../datepicker/src/useDateField.ts | 3 + .../datepicker/src/useDatePickerGroup.ts | 73 ++++++++- .../datepicker/src/useDateSegment.ts | 18 +- .../chromatic/DateField.stories.tsx | 12 +- .../chromatic/DatePicker.stories.tsx | 76 ++++++++- .../chromatic/DateRangePicker.stories.tsx | 80 ++++++++- .../chromatic/TimeField.stories.tsx | 12 +- .../datepicker/src/DatePickerField.tsx | 4 +- .../datepicker/src/DatePickerSegment.tsx | 4 +- .../datepicker/src/DateRangePicker.tsx | 58 +++---- .../@react-spectrum/datepicker/src/styles.css | 7 +- .../datepicker/src/{utils.ts => utils.tsx} | 16 +- .../datepicker/stories/DateField.stories.tsx | 2 +- .../datepicker/test/DateField.test.js | 4 +- .../datepicker/test/DatePicker.test.js | 8 +- .../datepicker/test/DatePickerBase.test.js | 154 ++++++++++++++---- .../datepicker/test/DateRangePicker.test.js | 6 +- .../datepicker/src/placeholders.ts | 2 +- .../datepicker/src/useDateFieldState.ts | 83 +++++++--- .../react-aria-components/docs/DateField.mdx | 4 +- .../react-aria-components/docs/TimeField.mdx | 4 +- .../react-aria-components/example/index.css | 1 - .../react-aria-components/src/DateField.tsx | 3 +- .../react-aria-components/src/Popover.tsx | 10 +- .../stories/DatePicker.stories.tsx | 8 +- .../test/DatePicker.test.js | 2 +- .../test/TimeField.test.js | 2 +- 28 files changed, 526 insertions(+), 136 deletions(-) rename packages/@react-spectrum/datepicker/src/{utils.ts => utils.tsx} (89%) diff --git a/packages/@react-aria/datepicker/docs/useDateField.mdx b/packages/@react-aria/datepicker/docs/useDateField.mdx index b8a66e9e06f..f82dc860993 100644 --- a/packages/@react-aria/datepicker/docs/useDateField.mdx +++ b/packages/@react-aria/datepicker/docs/useDateField.mdx @@ -130,12 +130,12 @@ function DateSegment({segment, state}) { let {segmentProps} = useDateSegment(segment, state, ref); return ( -
{segment.text} -
+ ); } @@ -153,7 +153,7 @@ function DateSegment({segment, state}) { } .field { - display: inline-flex; + display: block; padding: 2px 4px; border-radius: 2px; border: 1px solid var(--gray); diff --git a/packages/@react-aria/datepicker/src/useDateField.ts b/packages/@react-aria/datepicker/src/useDateField.ts index 877678b3ebe..cf4e64216f4 100644 --- a/packages/@react-aria/datepicker/src/useDateField.ts +++ b/packages/@react-aria/datepicker/src/useDateField.ts @@ -181,6 +181,9 @@ export function useDateField(props: AriaDateFieldOptions if (props.onKeyUp) { props.onKeyUp(e); } + }, + style: { + unicodeBidi: 'isolate' } }), inputProps, diff --git a/packages/@react-aria/datepicker/src/useDatePickerGroup.ts b/packages/@react-aria/datepicker/src/useDatePickerGroup.ts index 43f8521f02d..478012de5fc 100644 --- a/packages/@react-aria/datepicker/src/useDatePickerGroup.ts +++ b/packages/@react-aria/datepicker/src/useDatePickerGroup.ts @@ -1,14 +1,48 @@ import {createFocusManager, getFocusableTreeWalker} from '@react-aria/focus'; import {DateFieldState, DatePickerState, DateRangePickerState} from '@react-stately/datepicker'; import {FocusableElement, KeyboardEvent, RefObject} from '@react-types/shared'; -import {mergeProps} from '@react-aria/utils'; +import {mergeProps, useLayoutEffect} from '@react-aria/utils'; import {useLocale} from '@react-aria/i18n'; -import {useMemo} from 'react'; +import {useMemo, useRef} from 'react'; import {usePress} from '@react-aria/interactions'; export function useDatePickerGroup(state: DatePickerState | DateRangePickerState | DateFieldState, ref: RefObject, disableArrowNavigation?: boolean) { let {direction} = useLocale(); let focusManager = useMemo(() => createFocusManager(ref), [ref]); + let segments = useRef(undefined); + useLayoutEffect(() => { + if (ref?.current) { + + let update = () => { + if (ref.current) { + // TODO: For now, just querying this list of elements. However, it's possible that either through hooks or RAC that some users may include other focusable items that they would want to able to keyboard navigate to. In that case, we might want to utilize focusableElements in isFocusable.ts + let editableSegments: NodeListOf | undefined = ref.current?.querySelectorAll('span[role="spinbutton"], span[role="textbox"], button, div[role="spinbutton"], div[role="textbox"]'); + + let segmentsArr = Array.from(editableSegments as NodeListOf).filter(Boolean).map(node => { + return { + element: node as FocusableElement, + rectX: node.getBoundingClientRect().left + }; + }); + + let orderedSegments = segmentsArr.sort((a, b) => a.rectX - b.rectX).map((item => item.element)); + segments.current = orderedSegments; + } + }; + + update(); + + let observer = new MutationObserver(update); + observer.observe(ref.current, { + subtree: true, + childList: true + }); + + return () => { + observer.disconnect(); + }; + } + }, []); // Open the popover on alt + arrow down let onKeyDown = (e: KeyboardEvent) => { @@ -31,7 +65,21 @@ export function useDatePickerGroup(state: DatePickerState | DateRangePickerState e.preventDefault(); e.stopPropagation(); if (direction === 'rtl') { - focusManager.focusNext(); + if (segments.current) { + let orderedSegments = segments.current; + let target = e.target as FocusableElement; + let index = orderedSegments.indexOf(target); + + if (index === 0) { + target = orderedSegments[0] || target; + } else { + target = orderedSegments[index - 1] || target; + } + + if (target) { + target.focus(); + } + } } else { focusManager.focusPrevious(); } @@ -40,7 +88,24 @@ export function useDatePickerGroup(state: DatePickerState | DateRangePickerState e.preventDefault(); e.stopPropagation(); if (direction === 'rtl') { - focusManager.focusPrevious(); + if (segments.current) { + let orderedSegments = segments.current; + let target = e.target as FocusableElement; + let index = orderedSegments.indexOf(target); + + if (index === orderedSegments.length - 1) { + target = orderedSegments[orderedSegments.length - 1] || target; + } else { + target = orderedSegments[index - 1] || target; + } + + + target = orderedSegments[index + 1] || target; + + if (target) { + target.focus(); + } + } } else { focusManager.focusNext(); } diff --git a/packages/@react-aria/datepicker/src/useDateSegment.ts b/packages/@react-aria/datepicker/src/useDateSegment.ts index d022ece89e0..4a193ac3a02 100644 --- a/packages/@react-aria/datepicker/src/useDateSegment.ts +++ b/packages/@react-aria/datepicker/src/useDateSegment.ts @@ -15,7 +15,7 @@ import {DateFieldState, DateSegment} from '@react-stately/datepicker'; import {getScrollParent, isIOS, isMac, mergeProps, scrollIntoViewport, useEvent, useId, useLabels, useLayoutEffect} from '@react-aria/utils'; import {hookData} from './useDateField'; import {NumberParser} from '@internationalized/number'; -import React, {useMemo, useRef} from 'react'; +import React, {CSSProperties, useMemo, useRef} from 'react'; import {RefObject} from '@react-types/shared'; import {useDateFormatter, useFilter, useLocale} from '@react-aria/i18n'; import {useDisplayNames} from './useDisplayNames'; @@ -33,7 +33,7 @@ export interface DateSegmentAria { */ export function useDateSegment(segment: DateSegment, state: DateFieldState, ref: RefObject): DateSegmentAria { let enteredKeys = useRef(''); - let {locale} = useLocale(); + let {locale, direction} = useLocale(); let displayNames = useDisplayNames(); let {ariaLabel, ariaLabelledBy, ariaDescribedBy, focusManager} = hookData.get(state)!; @@ -385,6 +385,16 @@ export function useDateSegment(segment: DateSegment, state: DateFieldState, ref: }; } + let dateSegments = ['day', 'month', 'year']; + let segmentStyle : CSSProperties = {caretColor: 'transparent'}; + if (direction === 'rtl') { + if (dateSegments.includes(segment.type)) { + segmentStyle = {caretColor: 'transparent', direction: 'ltr', unicodeBidi: 'embed'}; + } else if (segment.type === 'timeZoneName') { + segmentStyle = {caretColor: 'transparent', unicodeBidi: 'embed'}; + } + } + return { segmentProps: mergeProps(spinButtonProps, labelProps, { id, @@ -403,9 +413,7 @@ export function useDateSegment(segment: DateSegment, state: DateFieldState, ref: tabIndex: state.isDisabled ? undefined : 0, onKeyDown, onFocus, - style: { - caretColor: 'transparent' - }, + style: segmentStyle, // Prevent pointer events from reaching useDatePickerGroup, and allow native browser behavior to focus the segment. onPointerDown(e) { e.stopPropagation(); diff --git a/packages/@react-spectrum/datepicker/chromatic/DateField.stories.tsx b/packages/@react-spectrum/datepicker/chromatic/DateField.stories.tsx index 23cf9880a2a..363a9c17424 100644 --- a/packages/@react-spectrum/datepicker/chromatic/DateField.stories.tsx +++ b/packages/@react-spectrum/datepicker/chromatic/DateField.stories.tsx @@ -21,7 +21,7 @@ export default { title: 'DateField', parameters: { chromaticProvider: { - locales: ['en-US', 'ar-EG', 'ja-JP'] + locales: ['en-US', 'ar-EG', 'ja-JP', 'he-IL'] } } }; @@ -41,6 +41,16 @@ PlaceholderFocus.parameters = { } }; +export const PlaceholderFocusRTL = () => ; +PlaceholderFocusRTL.parameters = { + chromaticProvider: { + locales: ['he-IL'], + scales: ['medium'], + colorSchemes: ['light'], + express: false + } +}; + export const PlaceholderFocusExpress = () => ; PlaceholderFocusExpress.parameters = { chromaticProvider: { diff --git a/packages/@react-spectrum/datepicker/chromatic/DatePicker.stories.tsx b/packages/@react-spectrum/datepicker/chromatic/DatePicker.stories.tsx index 0c9c8c5a3e1..e928ab54be3 100644 --- a/packages/@react-spectrum/datepicker/chromatic/DatePicker.stories.tsx +++ b/packages/@react-spectrum/datepicker/chromatic/DatePicker.stories.tsx @@ -16,12 +16,13 @@ import {ContextualHelp} from '@react-spectrum/contextualhelp'; import {DatePicker} from '../'; import {Heading} from '@react-spectrum/text'; import React from 'react'; +import {userEvent, within} from '@storybook/testing-library'; export default { title: 'DatePicker', parameters: { chromaticProvider: { - locales: ['en-US'/* , 'ar-EG', 'ja-JP' */] + locales: ['en-US', 'ar-EG', 'ja-JP', 'he-IL'] } } }; @@ -37,6 +38,17 @@ const focusParams = { const openParams = { chromaticProvider: { + locales: ['en-US'], + colorSchemes: ['light'], + scales: ['medium'], + disableAnimations: true, + express: false + } +}; + +const openParamsRTL = { + chromaticProvider: { + locales: ['he-IL'], colorSchemes: ['light'], scales: ['medium'], disableAnimations: true, @@ -54,6 +66,16 @@ export const Placeholder = () => ; PlaceholderFocus.parameters = focusParams; +export const PlaceholderFocusRTL = () => ; +PlaceholderFocusRTL.parameters = { + chromaticProvider: { + locales: ['ar-EG'], + scales: ['medium'], + colorSchemes: ['light'], + express: false + } +}; + export const PlaceholderFocusExpress = () => ; PlaceholderFocusExpress.parameters = { chromaticProvider: { @@ -110,10 +132,18 @@ export const OpenPlaceholder = () => ; +OpenPlaceholderRTL.parameters = openParamsRTL; +OpenPlaceholderRTL.decorators = openDecorators; + export const OpenValue = () => ; OpenValue.parameters = openParams; OpenValue.decorators = openDecorators; +export const OpenValueRTL = () => ; +OpenValueRTL.parameters = openParamsRTL; +OpenValueRTL.decorators = openDecorators; + export const OpenTime = () => ; OpenTime.parameters = openParams; OpenTime.decorators = openDecorators; @@ -147,6 +177,50 @@ OpenExpress.parameters = { }; OpenExpress.decorators = openDecorators; +export const OpenLTRInteractions = () => ; +OpenLTRInteractions.parameters = { + chromaticProvider: { + locales: ['en-US'], + scales: ['medium'], + colorSchemes: ['light'], + express: false + } +}; +OpenLTRInteractions.decorators = openDecorators; + +OpenLTRInteractions.play = async ({canvasElement}) => { + await userEvent.tab(); + await userEvent.keyboard('[ArrowRight]'); + await userEvent.keyboard('[ArrowRight]'); + await userEvent.keyboard('[ArrowRight]'); + await userEvent.keyboard('[Enter]]'); + let body = canvasElement.ownerDocument.body; + await within(body).findByRole('dialog'); + await userEvent.keyboard('[ArrowRight]'); +}; + +export const OpenRTLInteractions = () => ; +OpenRTLInteractions.parameters = { + chromaticProvider: { + locales: ['ar-EG'], + scales: ['medium'], + colorSchemes: ['light'], + express: false + } +}; +OpenRTLInteractions.decorators = openDecorators; + +OpenRTLInteractions.play = async ({canvasElement}) => { + await userEvent.tab(); + await userEvent.keyboard('[ArrowLeft]'); + await userEvent.keyboard('[ArrowLeft]'); + await userEvent.keyboard('[ArrowLeft]'); + await userEvent.keyboard('[Enter]]'); + let body = canvasElement.ownerDocument.body; + await within(body).findByRole('dialog'); + await userEvent.keyboard('[ArrowLeft]'); +}; + export const MultipleMonths = () => ; MultipleMonths.parameters = openParams; MultipleMonths.decorators = [Story =>
]; diff --git a/packages/@react-spectrum/datepicker/chromatic/DateRangePicker.stories.tsx b/packages/@react-spectrum/datepicker/chromatic/DateRangePicker.stories.tsx index e70a6d46067..411030f8d1c 100644 --- a/packages/@react-spectrum/datepicker/chromatic/DateRangePicker.stories.tsx +++ b/packages/@react-spectrum/datepicker/chromatic/DateRangePicker.stories.tsx @@ -16,12 +16,13 @@ import {ContextualHelp} from '@react-spectrum/contextualhelp'; import {DateRangePicker} from '../'; import {Heading} from '@react-spectrum/text'; import React from 'react'; +import {userEvent, within} from '@storybook/testing-library'; export default { title: 'DateRangePicker', parameters: { chromaticProvider: { - locales: ['en-US'/* , 'ar-EG', 'ja-JP' */] + locales: ['en-US', 'ar-EG', 'ja-JP', 'he-IL'] } } }; @@ -37,6 +38,17 @@ const focusParams = { const openParams = { chromaticProvider: { + locales: ['en-US'], + colorSchemes: ['light'], + scales: ['medium'], + disableAnimations: true, + express: false + } +}; + +const openParamsRTL = { + chromaticProvider: { + locales: ['ar-EG'], colorSchemes: ['light'], scales: ['medium'], disableAnimations: true, @@ -65,6 +77,16 @@ export const Placeholder = () => ; PlaceholderFocus.parameters = focusParams; +export const PlaceholderFocusRTL = () => ; +PlaceholderFocusRTL.parameters = { + chromaticProvider: { + locales: ['he-IL'], + scales: ['medium'], + colorSchemes: ['light'], + express: false + } +}; + export const PlaceholderFocusExpress = () => ; PlaceholderFocusExpress.parameters = { chromaticProvider: { @@ -121,10 +143,18 @@ export const OpenPlaceholder = () => ; +OpenPlaceholderRTL.parameters = openParamsRTL; +OpenPlaceholderRTL.decorators = openDecorators; + export const OpenValue = () => ; OpenValue.parameters = openParams; OpenValue.decorators = openDecorators; +export const OpenValueRTL = () => ; +OpenValueRTL.parameters = openParamsRTL; +OpenValueRTL.decorators = openDecorators; + export const OpenTime = () => ; OpenTime.parameters = openParams; OpenTime.decorators = openDecorators; @@ -158,6 +188,54 @@ OpenExpress.parameters = { }; OpenExpress.decorators = openDecorators; +export const OpenFocusLTRInteractions = () => ; +OpenFocusLTRInteractions.parameters = { + chromaticProvider: { + locales: ['en-US'], + scales: ['medium'], + colorSchemes: ['light'], + express: false + } +}; +OpenFocusLTRInteractions.decorators = openDecorators; + +OpenFocusLTRInteractions.play = async ({canvasElement}) => { + await userEvent.tab(); + await userEvent.keyboard('[ArrowRight]'); + await userEvent.keyboard('[ArrowRight]'); + await userEvent.keyboard('[ArrowRight]'); + await userEvent.keyboard('[ArrowRight]'); + await userEvent.keyboard('[ArrowRight]'); + await userEvent.keyboard('[ArrowRight]'); + await userEvent.keyboard('[Enter]'); + let body = canvasElement.ownerDocument.body; + await within(body).findByRole('dialog'); + await userEvent.keyboard('[ArrowLeft]'); +}; + +export const OpenFocusRTLInteractions = () => ; +OpenFocusRTLInteractions.parameters = { + chromaticProvider: { + locales: ['he-IL'], + scales: ['medium'], + colorSchemes: ['light'], + express: false + } +}; +OpenFocusRTLInteractions.decorators = openDecorators; + +OpenFocusRTLInteractions.play = async ({canvasElement}) => { + await userEvent.tab(); + await userEvent.keyboard('[ArrowLeft]'); + await userEvent.keyboard('[ArrowLeft]'); + await userEvent.keyboard('[ArrowLeft]'); + await userEvent.keyboard('[ArrowLeft]'); + await userEvent.keyboard('[Enter]'); + let body = canvasElement.ownerDocument.body; + await within(body).findByRole('dialog'); + await userEvent.keyboard('[ArrowRight]'); +}; + export const MultipleMonths = () => ; MultipleMonths.parameters = openParams; MultipleMonths.decorators = [Story =>
]; diff --git a/packages/@react-spectrum/datepicker/chromatic/TimeField.stories.tsx b/packages/@react-spectrum/datepicker/chromatic/TimeField.stories.tsx index 8294c546115..1c63080e598 100644 --- a/packages/@react-spectrum/datepicker/chromatic/TimeField.stories.tsx +++ b/packages/@react-spectrum/datepicker/chromatic/TimeField.stories.tsx @@ -21,7 +21,7 @@ export default { title: 'TimeField', parameters: { chromaticProvider: { - locales: ['en-US'/* , 'ar-EG', 'ja-JP' */] + locales: ['en-US', 'ar-EG', 'ja-JP'] } } }; @@ -41,6 +41,16 @@ PlaceholderFocus.parameters = { } }; +export const PlaceholderFocusRTL = () => ; +PlaceholderFocusRTL.parameters = { + chromaticProvider: { + locales: ['ar-EG'], + scales: ['medium'], + colorSchemes: ['light'], + express: false + } +}; + export const PlaceholderFocusExpress = () => ; PlaceholderFocusExpress.parameters = { chromaticProvider: { diff --git a/packages/@react-spectrum/datepicker/src/DatePickerField.tsx b/packages/@react-spectrum/datepicker/src/DatePickerField.tsx index 210bfd21324..275bd477ad7 100644 --- a/packages/@react-spectrum/datepicker/src/DatePickerField.tsx +++ b/packages/@react-spectrum/datepicker/src/DatePickerField.tsx @@ -45,7 +45,7 @@ export function DatePickerField(props: DatePickerFieldProps let {fieldProps, inputProps} = useDateField({...props, inputRef}, state, ref); return ( -
+ {state.segments.map((segment, i) => ((props: DatePickerFieldProps isRequired={isRequired} />) )} -
+ ); } diff --git a/packages/@react-spectrum/datepicker/src/DatePickerSegment.tsx b/packages/@react-spectrum/datepicker/src/DatePickerSegment.tsx index 82f43c90e0f..acddf30c569 100644 --- a/packages/@react-spectrum/datepicker/src/DatePickerSegment.tsx +++ b/packages/@react-spectrum/datepicker/src/DatePickerSegment.tsx @@ -54,7 +54,7 @@ function EditableSegment({segment, state}: DatePickerSegmentProps) { let {segmentProps} = useDateSegment(segment, state, ref); return ( -
{segment.isPlaceholder ? : segment.text} -
+ ); } diff --git a/packages/@react-spectrum/datepicker/src/DateRangePicker.tsx b/packages/@react-spectrum/datepicker/src/DateRangePicker.tsx index 359df856ae2..9a7026a2459 100644 --- a/packages/@react-spectrum/datepicker/src/DateRangePicker.tsx +++ b/packages/@react-spectrum/datepicker/src/DateRangePicker.tsx @@ -138,33 +138,35 @@ export const DateRangePicker = React.forwardRef(function DateRangePicker - - - - - +
+ + + + + +