Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: ensure date/time segments are ordered correctly in RTL #7423

Open
wants to merge 43 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 32 commits
Commits
Show all changes
43 commits
Select commit Hold shift + click to select a range
bbe6191
bdo on timefield, reverse segments on timefield in datefield
yihuiliao Nov 21, 2024
0716ad5
fix lint
yihuiliao Nov 21, 2024
76b8d0e
make things inline
yihuiliao Nov 26, 2024
47e2af5
use unicode character to wrap segments
yihuiliao Dec 4, 2024
ef0b637
fix test
yihuiliao Dec 4, 2024
b3e2f70
append unicode to text in hooks, update rac
yihuiliao Dec 6, 2024
7b77f1e
Merge branch 'main' into bidi-override
yihuiliao Dec 6, 2024
fd5a902
add comment
yihuiliao Dec 17, 2024
d9b69a8
Merge branch 'main' into bidi-override
yihuiliao Dec 18, 2024
a9f732e
skip failing test for now
yihuiliao Dec 20, 2024
8df5ad6
update keyboard nav
yihuiliao Dec 20, 2024
679e355
update logic of how unicode is applied
yihuiliao Dec 20, 2024
bedf713
fix spacing
yihuiliao Dec 20, 2024
c2df442
add comments
yihuiliao Jan 7, 2025
9080188
update tests
yihuiliao Jan 8, 2025
2b5fb13
undo some previous changes
yihuiliao Jan 8, 2025
91173c3
wrap time segments in lri, wrap fields in unicode isolate
yihuiliao Jan 8, 2025
79a8da3
fix ssr test
yihuiliao Jan 8, 2025
b923904
fix spacing
yihuiliao Jan 10, 2025
ae03f3b
fix css logic
yihuiliao Jan 10, 2025
2bbde19
fix lint
yihuiliao Jan 10, 2025
81560bd
fix keyboard nav in rac datepicker popover
yihuiliao Jan 14, 2025
dad9ceb
fix lint
yihuiliao Jan 14, 2025
ec8fe1d
prevent overflow in date range picker
yihuiliao Jan 14, 2025
a863e18
Merge branch 'main' into bidi-override
yihuiliao Jan 14, 2025
891dce9
move overflow hidden to separate new div to fix weird focus ring arou…
yihuiliao Jan 14, 2025
4b56394
this time actually fix the overflow and focus ring issue
yihuiliao Jan 14, 2025
1a9eaa9
update var names to be nicer
yihuiliao Jan 14, 2025
f50b4b1
fix japanese placeholder for extra space
yihuiliao Jan 14, 2025
e39f1ea
fix css positioning
yihuiliao Jan 14, 2025
05ed94d
Merge branch 'main' into bidi-override
yihuiliao Jan 14, 2025
ccf7b85
fix custom width
yihuiliao Jan 15, 2025
b1915c2
small css changes so that rtl will format properly
yihuiliao Jan 15, 2025
af3ab18
memo ordering of segments for keyboard navigation
yihuiliao Jan 15, 2025
50bb4df
add chromatic tests
yihuiliao Jan 16, 2025
1bd410e
fix lint
yihuiliao Jan 16, 2025
623219d
add tests to rsp date components
yihuiliao Jan 17, 2025
ccf7def
add tests to rac
yihuiliao Jan 17, 2025
ccce891
fix tests
yihuiliao Jan 17, 2025
2e682e2
Merge branch 'main' into bidi-override
yihuiliao Jan 17, 2025
6efe67f
remove comment
yihuiliao Jan 17, 2025
0fdb98d
fix chromatic stories
yihuiliao Jan 17, 2025
ab2a67a
add chromatic story
yihuiliao Jan 17, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions packages/@react-aria/datepicker/src/useDateField.ts
Original file line number Diff line number Diff line change
Expand Up @@ -181,6 +181,9 @@ export function useDateField<T extends DateValue>(props: AriaDateFieldOptions<T>
if (props.onKeyUp) {
props.onKeyUp(e);
}
},
style: {
unicodeBidi: 'isolate'
Copy link
Member Author

@yihuiliao yihuiliao Jan 15, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

added for DateRangePicker because things were looking a little funky without it. wraps around each datefield. see codepen for reproduction

i could update this so that it only applies in rtl locales but it also doesn't seem to have any affect for ltr locales.

}
}),
inputProps,
Expand Down
49 changes: 47 additions & 2 deletions packages/@react-aria/datepicker/src/useDatePickerGroup.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,32 @@ export function useDatePickerGroup(state: DatePickerState | DateRangePickerState
e.preventDefault();
e.stopPropagation();
if (direction === 'rtl') {
focusManager.focusNext();
let editableSegments: NodeListOf<Element> | undefined = ref.current?.querySelectorAll('span[role="spinbutton"], span[role="textbox"]');
if (editableSegments) {
let segments = Array.from(editableSegments);
let button = ref.current?.querySelector('button');
let target = e.target as FocusableElement;

let segmentArr = segments.map(node => {
return {
element: node as FocusableElement,
rectX: node?.getBoundingClientRect().left
};
});

let arr = segmentArr.sort((a, b) => a.rectX - b.rectX).map((item => item.element));
let index = arr.indexOf(target);

if (index === 0) {
target = button || target;
} else {
target = arr[index - 1] || target;
}

if (target) {
target.focus();
}
}
} else {
focusManager.focusPrevious();
}
Expand All @@ -40,7 +65,27 @@ export function useDatePickerGroup(state: DatePickerState | DateRangePickerState
e.preventDefault();
e.stopPropagation();
if (direction === 'rtl') {
focusManager.focusPrevious();
let editableSegments: NodeListOf<Element> | undefined = ref.current?.querySelectorAll('span[role="spinbutton"], span[role="textbox"]');
if (editableSegments) {
let segments = Array.from(editableSegments);
let target = e.target as FocusableElement;

let segmentArr = segments.map(node => {
return {
element: node as FocusableElement,
rectX: node?.getBoundingClientRect().left
};
});

let arr = segmentArr.sort((a, b) => a.rectX - b.rectX).map((item => item.element));
let index = arr.indexOf(target);

target = arr[index + 1] || target;

if (target) {
target.focus();
}
}
} else {
focusManager.focusNext();
}
Expand Down
18 changes: 13 additions & 5 deletions packages/@react-aria/datepicker/src/useDateSegment.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand All @@ -33,7 +33,7 @@ export interface DateSegmentAria {
*/
export function useDateSegment(segment: DateSegment, state: DateFieldState, ref: RefObject<HTMLElement | null>): DateSegmentAria {
let enteredKeys = useRef('');
let {locale} = useLocale();
let {locale, direction} = useLocale();
let displayNames = useDisplayNames();
let {ariaLabel, ariaLabelledBy, ariaDescribedBy, focusManager} = hookData.get(state)!;

Expand Down Expand Up @@ -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,
Expand All @@ -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();
Expand Down
4 changes: 2 additions & 2 deletions packages/@react-spectrum/datepicker/src/DatePickerField.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ export function DatePickerField<T extends DateValue>(props: DatePickerFieldProps
let {fieldProps, inputProps} = useDateField({...props, inputRef}, state, ref);

return (
<div {...fieldProps} data-testid={props['data-testid']} className={classNames(datepickerStyles, 'react-spectrum-Datepicker-segments', inputClassName)} ref={ref}>
<span {...fieldProps} data-testid={props['data-testid']} className={classNames(datepickerStyles, 'react-spectrum-Datepicker-segments', inputClassName)} ref={ref}>
{state.segments.map((segment, i) =>
(<DatePickerSegment
key={i}
Expand All @@ -56,6 +56,6 @@ export function DatePickerField<T extends DateValue>(props: DatePickerFieldProps
isRequired={isRequired} />)
)}
<input {...inputProps} ref={inputRef} />
</div>
</span>
);
}
4 changes: 2 additions & 2 deletions packages/@react-spectrum/datepicker/src/DatePickerSegment.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ function EditableSegment({segment, state}: DatePickerSegmentProps) {
let {segmentProps} = useDateSegment(segment, state, ref);

return (
<div
<span
{...segmentProps}
ref={ref}
className={classNames(styles, 'react-spectrum-DatePicker-cell', {
Expand All @@ -64,6 +64,6 @@ function EditableSegment({segment, state}: DatePickerSegmentProps) {
style={segmentProps.style}
data-testid={segment.type}>
{segment.isPlaceholder ? <span aria-hidden="true" className={classNames(styles, 'react-spectrum-DatePicker-placeholder')}>{segment.placeholder}</span> : segment.text}
</div>
</span>
);
}
58 changes: 30 additions & 28 deletions packages/@react-spectrum/datepicker/src/DateRangePicker.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -138,33 +138,35 @@ export const DateRangePicker = React.forwardRef(function DateRangePicker<T exten
{...mergeProps(groupProps, hoverProps, focusProps)}
className={className}
ref={targetRef}>
<Input
isDisabled={isDisabled}
isQuiet={isQuiet}
validationState={validationState}
className={classNames(styles, 'spectrum-InputGroup-field')}
inputClassName={fieldClassName}
disableFocusRing
minWidth={approximateWidth}>
<DatePickerField
{...startFieldProps}
data-testid="start-date"
isQuiet={props.isQuiet}
inputClassName={classNames(datepickerStyles, 'react-spectrum-Datepicker-startField')} />
<DateRangeDash />
<DatePickerField
{...endFieldProps}
data-testid="end-date"
isQuiet={props.isQuiet}
inputClassName={classNames(
styles,
'spectrum-Datepicker-endField',
classNames(
datepickerStyles,
'react-spectrum-Datepicker-endField'
)
)} />
</Input>
<div style={{overflow: 'hidden', width: '100%'}}>
<Input
isDisabled={isDisabled}
isQuiet={isQuiet}
validationState={validationState}
className={classNames(styles, 'spectrum-InputGroup-field')}
inputClassName={fieldClassName}
disableFocusRing
minWidth={approximateWidth}>
<DatePickerField
{...startFieldProps}
data-testid="start-date"
isQuiet={props.isQuiet}
inputClassName={classNames(datepickerStyles, 'react-spectrum-Datepicker-startField')} />
<DateRangeDash />
<DatePickerField
{...endFieldProps}
data-testid="end-date"
isQuiet={props.isQuiet}
inputClassName={classNames(
styles,
'spectrum-Datepicker-endField',
classNames(
datepickerStyles,
'react-spectrum-Datepicker-endField'
)
)} />
</Input>
</div>
<DialogTrigger
type="popover"
mobileType="tray"
Expand Down Expand Up @@ -227,7 +229,7 @@ export const DateRangePicker = React.forwardRef(function DateRangePicker<T exten

function DateRangeDash() {
return (
<div
<span
aria-hidden="true"
data-testid="date-range-dash"
className={classNames(datepickerStyles, 'react-spectrum-Datepicker-rangeDash')} />
Expand Down
8 changes: 4 additions & 4 deletions packages/@react-spectrum/datepicker/src/styles.css
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@
}

.react-spectrum-Datepicker-field.react-spectrum-Datepicker-field {
width: auto;
width: 100%;
}

.react-spectrum-Datepicker-field .react-spectrum-DateField-Input {
Expand Down Expand Up @@ -77,8 +77,8 @@
}

.react-spectrum-Datepicker-inputSized {
display: flex;
height: 100%;
display: inline;
/* height: 100%; */
align-items: center;
}

Expand All @@ -89,7 +89,7 @@
}

.react-spectrum-Datepicker-segments {
display: flex;
display: inline;
align-items: center;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -277,7 +277,7 @@ const preferences = [
{locale: '', label: 'Default', ordering: 'gregory'},
{label: 'Arabic (Algeria)', locale: 'ar-DZ', territories: 'DJ DZ EH ER IQ JO KM LB LY MA MR OM PS SD SY TD TN YE', ordering: 'gregory islamic islamic-civil islamic-tbla'},
{label: 'Arabic (United Arab Emirates)', locale: 'ar-AE', territories: 'AE BH KW QA', ordering: 'gregory islamic-umalqura islamic islamic-civil islamic-tbla'},
{label: 'Arabic (Egypt)', locale: 'AR-EG', territories: 'EG', ordering: 'gregory coptic islamic islamic-civil islamic-tbla'},
{label: 'Arabic (Egypt)', locale: 'ar-EG', territories: 'EG', ordering: 'gregory coptic islamic islamic-civil islamic-tbla'},
{label: 'Arabic (Saudi Arabia)', locale: 'ar-SA', territories: 'SA', ordering: 'islamic-umalqura gregory islamic islamic-rgsa'},
{label: 'Farsi (Afghanistan)', locale: 'fa-AF', territories: 'AF IR', ordering: 'persian gregory islamic islamic-civil islamic-tbla'},
// {territories: 'CN CX HK MO SG', ordering: 'gregory chinese'},
Expand Down
4 changes: 2 additions & 2 deletions packages/@react-spectrum/datepicker/test/DatePicker.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ function getTextValue(el) {
return '';
}

return el.textContent;
return el.textContent.replace(/[\u2066-\u2069]/g, '');
}

function expectPlaceholder(el, placeholder) {
Expand Down Expand Up @@ -1043,7 +1043,7 @@ describe('DatePicker', function () {
await user.keyboard('{ArrowUp}');

expect(queryByTestId('era')).toBeNull();
expect(document.activeElement).toBe(field.firstChild);
expect(document.activeElement.textContent.replace(/[\u2066-\u2069]/g, '')).toBe('3');
});

it('does not try to shift focus when the entire datepicker is unmounted while focused', function () {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -427,8 +427,8 @@ describe('DatePickerBase', function () {
fireEvent.keyDown(document.activeElement, {key: 'ArrowLeft'});
}
});

it.each`
// TODO: figure out this test. probably remove it since the issues stem from not being able to actually calculate the position of each segments. still would like to find a way to test tho?
it.skip.each`
Name | Component
${'DatePicker'} | ${DatePicker}
${'DateRangePicker'} | ${DateRangePicker}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ function getTextValue(el) {
return '';
}

return [...el.childNodes].map(el => el.nodeType === 3 ? el.textContent : getTextValue(el)).join('');
return [...el.childNodes].map(el => el.nodeType === 3 ? el.textContent.replace(/[\u2066-\u2069]/g, '') : getTextValue(el)).join('');
}

function expectPlaceholder(el, placeholder) {
Expand Down
2 changes: 1 addition & 1 deletion packages/@react-stately/datepicker/src/placeholders.ts
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ const placeholders = new LocalizedStringDictionary({
ia: {year: 'aaaa', month: 'mm', day: 'dd'},
id: {year: 'tttt', month: 'bb', day: 'hh'},
it: {year: 'aaaa', month: 'mm', day: 'gg'},
ja: {year: '', month: '月', day: '日'},
ja: {year: '', month: '月', day: '日'},
ka: {year: 'წწწწ', month: 'თთ', day: 'რრ'},
kk: {year: 'жжжж', month: 'аа', day: 'кк'},
kn: {year: 'ವವವವ', month: 'ಮಿಮೀ', day: 'ದಿದಿ'},
Expand Down
Loading
Loading