Skip to content

Commit

Permalink
fix: Remove iOS HiddenSelect workaround (#7200)
Browse files Browse the repository at this point in the history
* Remove iOS HiddenSelect workaround

* fix lint

* remove extraneous comment

* update tests

* fix lint

* remove reference
  • Loading branch information
snowystinger authored Nov 1, 2024
1 parent f554c29 commit f256f0e
Show file tree
Hide file tree
Showing 6 changed files with 79 additions and 76 deletions.
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);
});
});

1 comment on commit f256f0e

@rspbot
Copy link

@rspbot rspbot commented on f256f0e Nov 1, 2024

Choose a reason for hiding this comment

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

Please sign in to comment.