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: Remove iOS HiddenSelect workaround #7200

Merged
merged 7 commits into from
Nov 1, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 0 additions & 1 deletion packages/@react-aria/select/docs/useSelect.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,6 @@ select components that can be styled as needed without compromising on high qual
the first or last item accordingly
* Typeahead to allow selecting options by typing text, even without opening the listbox
* Browser autofill integration via a hidden native `<select>` element
* Support for mobile form navigation via software keyboard
* Mobile screen reader listbox dismissal support

## Anatomy
Expand Down
23 changes: 2 additions & 21 deletions packages/@react-aria/select/src/HiddenSelect.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ import {selectData} from './useSelect';
import {SelectState} from '@react-stately/select';
import {useFormReset} from '@react-aria/utils';
import {useFormValidation} from '@react-aria/form';
import {useInteractionModality} from '@react-aria/interactions';
import {useVisuallyHidden} from '@react-aria/visually-hidden';

export interface AriaHiddenSelectProps {
Expand Down Expand Up @@ -68,7 +67,6 @@ export function useHiddenSelect<T>(props: AriaHiddenSelectOptions, state: Select
let data = selectData.get(state) || {};
let {autoComplete, name = data.name, isDisabled = data.isDisabled} = props;
let {validationBehavior, isRequired} = data;
let modality = useInteractionModality();
let {visuallyHiddenProps} = useVisuallyHidden();

useFormReset(props.selectRef, state.selectedKey, state.setSelectedKey);
Expand All @@ -83,18 +81,6 @@ export function useHiddenSelect<T>(props: AriaHiddenSelectOptions, state: Select
// The solution is to use <VisuallyHidden> to hide the elements, which clips the elements to a
// 1px rectangle. In addition, we hide from screen readers with aria-hidden, and make the <select>
// non tabbable with tabIndex={-1}.
//
// In mobile browsers, there are next/previous buttons above the software keyboard for navigating
// between fields in a form. These only support native form inputs that are tabbable. In order to
// support those, an additional hidden input is used to marshall focus to the button. It is tabbable
// except when the button is focused, so that shift tab works properly to go to the actual previous
// input in the form. Using the <select> for this also works, but Safari on iOS briefly flashes
// the native menu on focus, so this isn't ideal. A font-size of 16px or greater is required to
// prevent Safari from zooming in on the input when it is focused.
//
// If the current interaction modality is null, then the user hasn't interacted with the page yet.
// In this case, we set the tabIndex to -1 on the input element so that automated accessibility
// checkers don't throw false-positives about focusable elements inside an aria-hidden parent.
return {
containerProps: {
...visuallyHiddenProps,
Expand All @@ -105,11 +91,7 @@ export function useHiddenSelect<T>(props: AriaHiddenSelectOptions, state: Select
['data-a11y-ignore']: 'aria-hidden-focus'
},
inputProps: {
type: 'text',
tabIndex: modality == null || state.isFocused || state.isOpen ? -1 : 0,
style: {fontSize: 16},
onFocus: () => triggerRef.current.focus(),
disabled: isDisabled
style: {display: 'none'}
},
selectProps: {
tabIndex: -1,
Expand All @@ -130,15 +112,14 @@ export function useHiddenSelect<T>(props: AriaHiddenSelectOptions, state: Select
export function HiddenSelect<T>(props: HiddenSelectProps<T>) {
let {state, triggerRef, label, name, isDisabled} = props;
let selectRef = useRef(null);
let {containerProps, inputProps, selectProps} = useHiddenSelect({...props, selectRef}, state, triggerRef);
let {containerProps, selectProps} = useHiddenSelect({...props, selectRef}, state, triggerRef);

// If used in a <form>, use a hidden input so the value can be submitted to a server.
// If the collection isn't too big, use a hidden <select> element for this so that browser
// autofill will work. Otherwise, use an <input type="hidden">.
if (state.collection.size <= 300) {
return (
<div {...containerProps} data-testid="hidden-select-container">
<input {...inputProps} />
<label>
{label}
<select {...selectProps} ref={selectRef}>
Expand Down
46 changes: 0 additions & 46 deletions packages/@react-spectrum/picker/test/Picker.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -64,9 +64,6 @@ describe('Picker', function () {
</Provider>
);

let select = getByRole('textbox', {hidden: true});
expect(select).not.toBeDisabled();

let picker = getByRole('button');
expect(picker).toHaveAttribute('aria-haspopup', 'listbox');
expect(picker).toHaveAttribute('data-testid', 'test');
Expand Down Expand Up @@ -1721,33 +1718,6 @@ describe('Picker', function () {
expect(onSelectionChange).toHaveBeenLastCalledWith('CA');
expect(picker).toHaveTextContent('California');
});

it('should have a hidden input to marshall focus to the button', function () {
let {getByRole} = render(
<Provider theme={theme}>
<Picker label="Test" onSelectionChange={onSelectionChange}>
<Item>One</Item>
<Item>Two</Item>
<Item>Three</Item>
</Picker>
</Provider>
);

let hiddenInput = getByRole('textbox', {hidden: true}); // get the hidden ones
expect(hiddenInput).toHaveAttribute('tabIndex', '0');
expect(hiddenInput).toHaveAttribute('style', 'font-size: 16px;');
expect(hiddenInput.parentElement).toHaveAttribute('aria-hidden', 'true');

act(() => hiddenInput.focus());

let button = getByRole('button');
expect(document.activeElement).toBe(button);
expect(hiddenInput).toHaveAttribute('tabIndex', '-1');

act(() => button.blur());

expect(hiddenInput).toHaveAttribute('tabIndex', '0');
});
});

describe('async loading', function () {
Expand Down Expand Up @@ -1838,22 +1808,6 @@ describe('Picker', function () {
});

describe('disabled', function () {
it('disables the hidden select when isDisabled is true', function () {
let {getByRole} = render(
<Provider theme={theme}>
<Picker isDisabled label="Test" onSelectionChange={onSelectionChange}>
<Item key="one">One</Item>
<Item key="two">Two</Item>
<Item key="three">Three</Item>
</Picker>
</Provider>
);

let select = getByRole('textbox', {hidden: true});

expect(select).toBeDisabled();
});

it('does not open on mouse down when isDisabled is true', async function () {
let onOpenChange = jest.fn();
let {getByRole, queryByRole} = render(
Expand Down
6 changes: 0 additions & 6 deletions packages/@react-spectrum/picker/test/TempUtilTest.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -50,9 +50,6 @@ describe('Picker/Select ', function () {
</Provider>
);

let select = getByRole('textbox', {hidden: true});
expect(select).not.toBeDisabled();

let picker = getByRole('button');
expect(picker).toHaveAttribute('aria-haspopup', 'listbox');
expect(picker).toHaveAttribute('data-testid', 'test');
Expand Down Expand Up @@ -157,9 +154,6 @@ describe('Picker/Select ', function () {
</Provider>
);

let select = getByRole('textbox', {hidden: true});
expect(select).not.toBeDisabled();

let picker = getByRole('button');
expect(picker).toHaveAttribute('aria-haspopup', 'listbox');
expect(picker).toHaveAttribute('data-testid', 'test');
Expand Down
2 changes: 1 addition & 1 deletion packages/react-aria-components/docs/Select.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -178,7 +178,7 @@ select components that can be styled as needed without compromising on high qual
* **Flexible** – Support for controlled and uncontrolled state, async loading, disabled items, validation, sections, complex items, and more.
* **Keyboard navigation** – Select can be opened and navigated using the arrow keys, along with page up/down, home/end, etc. Auto scrolling, and typeahead both in the listbox and on the button, are supported as well.
* **Accessible** – Follows the [ARIA listbox pattern](https://www.w3.org/WAI/ARIA/apg/patterns/listbox/), with support for items and sections, and slots for label and description elements within each item for improved screen reader announcement.
* **HTML form integration** – A visually hidden `<select>` element is included to enable HTML form integration, autofill, and mobile form navigation via the software keyboard.
* **HTML form integration** – A visually hidden `<select>` element is included to enable HTML form integration and autofill.
* **Validation** – Support for native HTML constraint validation with customizable UI, custom validation functions, and server-side validation errors.
* **Styleable** – Items include builtin states for styling, such as hover, press, focus, selected, and disabled.

Expand Down
77 changes: 76 additions & 1 deletion packages/react-aria-components/test/Select.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
*/

import {act, pointerMap, render, within} from '@react-spectrum/test-utils-internal';
import {Button, FieldError, Label, ListBox, ListBoxItem, Popover, Select, SelectContext, SelectValue, Text} from '../';
import {Button, FieldError, Label, ListBox, ListBoxItem, Popover, Select, SelectContext, SelectStateContext, SelectValue, Text} from '../';
import React from 'react';
import {User} from '@react-aria/test-utils';
import userEvent from '@testing-library/user-event';
Expand Down Expand Up @@ -278,4 +278,79 @@ describe('Select', () => {

expect(button).toHaveTextContent('0');
});

it('should support extra children for use with the state', async () => {
let onChangeSpy = jest.fn();

function SelectClearButton() {
let state = React.useContext(SelectStateContext);
return (
<Button
data-testid="clear"
// Don't inherit behavior from Select.
slot={null}
style={{fontSize: 'small', marginTop: 6, padding: 4}}
onPress={() => state?.setSelectedKey(null)}>
Clear
</Button>
);
}

let {getByTestId} = render(
<>
<input data-testid="before" />
<Select onSelectionChange={onChangeSpy}>
<Label>Favorite Animal</Label>
<Button data-testid="select">
<SelectValue />
<span aria-hidden="true">▼</span>
</Button>
<SelectClearButton />
<Popover>
<ListBox>
<ListBoxItem id="cat">Cat</ListBoxItem>
<ListBoxItem id="dog">Dog</ListBoxItem>
<ListBoxItem id="kangaroo">Kangaroo</ListBoxItem>
</ListBox>
</Popover>
</Select>
<input data-testid="after" />
</>
);

let beforeInput = getByTestId('before');
let afterInput = getByTestId('after');
let wrapper = getByTestId('select');
let clearButton = getByTestId('clear');
let selectTester = testUtilUser.createTester('Select', {root: wrapper});

await user.tab();
await user.tab();
expect(document.activeElement).toBe(selectTester.trigger);

await user.tab();
expect(document.activeElement).toBe(clearButton);

await user.tab();
expect(document.activeElement).toBe(afterInput);

await user.tab({shift: true});
expect(document.activeElement).toBe(clearButton);

await user.tab({shift: true});
expect(document.activeElement).toBe(selectTester.trigger);

await user.tab({shift: true});
expect(document.activeElement).toBe(beforeInput);

await user.tab();
await selectTester.selectOption({optionText: 'Dog'});

expect(onChangeSpy).toHaveBeenCalledTimes(1);
expect(onChangeSpy).toHaveBeenLastCalledWith('dog');

await user.click(clearButton);
expect(onChangeSpy).toHaveBeenCalledTimes(2);
expect(onChangeSpy).toHaveBeenLastCalledWith(null);
});
});