Skip to content

feat: Emit native change events in form components #8426

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

Draft
wants to merge 6 commits into
base: main
Choose a base branch
from
Draft
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
4 changes: 3 additions & 1 deletion packages/@react-aria/combobox/src/useComboBox.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ import {AriaComboBoxProps} from '@react-types/combobox';
import {ariaHideOutside} from '@react-aria/overlays';
import {AriaListBoxOptions, getItemId, listData} from '@react-aria/listbox';
import {BaseEvent, DOMAttributes, KeyboardDelegate, LayoutDelegate, PressEvent, RefObject, RouterOptions, ValidationResult} from '@react-types/shared';
import {chain, getActiveElement, getOwnerDocument, isAppleDevice, mergeProps, useLabels, useRouter, useUpdateEffect} from '@react-aria/utils';
import {chain, getActiveElement, getOwnerDocument, isAppleDevice, mergeProps, useInputEvent, useLabels, useRouter, useUpdateEffect} from '@react-aria/utils';
import {ComboBoxState} from '@react-stately/combobox';
import {dispatchVirtualFocus} from '@react-aria/focus';
import {FocusEvent, InputHTMLAttributes, KeyboardEvent, TouchEvent, useEffect, useMemo, useRef} from 'react';
Expand Down Expand Up @@ -351,6 +351,8 @@ export function useComboBox<T>(props: AriaComboBoxOptions<T>, state: ComboBoxSta
}
}, [focusedItem]);

useInputEvent(inputRef, state.onInputChange);

return {
labelProps,
buttonProps: {
Expand Down
3 changes: 1 addition & 2 deletions packages/@react-aria/radio/src/useRadio.ts
Original file line number Diff line number Diff line change
Expand Up @@ -63,8 +63,7 @@ export function useRadio(props: AriaRadioProps, state: RadioGroupState, ref: Ref

let checked = state.selectedValue === value;

let onChange = (e) => {
e.stopPropagation();
let onChange = () => {
state.setSelectedValue(value);
};

Expand Down
12 changes: 9 additions & 3 deletions packages/@react-aria/select/src/HiddenSelect.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import {FocusableElement, RefObject} from '@react-types/shared';
import React, {InputHTMLAttributes, JSX, ReactNode, useCallback, useRef} from 'react';
import {selectData} from './useSelect';
import {SelectState} from '@react-stately/select';
import {useFormReset} from '@react-aria/utils';
import {useFormReset, useInputEvent} from '@react-aria/utils';
import {useFormValidation} from '@react-aria/form';
import {useVisuallyHidden} from '@react-aria/visually-hidden';

Expand Down Expand Up @@ -82,8 +82,14 @@ export function useHiddenSelect<T>(props: AriaHiddenSelectOptions, state: Select
focus: () => triggerRef.current?.focus()
}, state, props.selectRef);

// eslint-disable-next-line react-hooks/exhaustive-deps
let onChange = useCallback((e: React.ChangeEvent<HTMLSelectElement> | React.FormEvent<HTMLSelectElement>) => state.setSelectedKey(e.currentTarget.value), [state.setSelectedKey]);
useInputEvent(props.selectRef!, state.onSelectionChange);

let setSelectedKey = state.setSelectedKey;
let onChange = useCallback((e: React.ChangeEvent<HTMLSelectElement> | React.FormEvent<HTMLSelectElement>) => {
if (!e.nativeEvent['__reactAriaIgnore']) {
setSelectedKey(e.currentTarget.value);
}
}, [setSelectedKey]);

// In Safari, the <select> cannot have `display: none` or `hidden` for autofill to work.
// In Firefox, there must be a <label> to identify the <select> whereas other browsers
Expand Down
22 changes: 20 additions & 2 deletions packages/@react-aria/slider/src/useSliderThumb.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import {AriaSliderThumbProps} from '@react-types/slider';
import {clamp, focusWithoutScrolling, mergeProps, useFormReset, useGlobalListeners} from '@react-aria/utils';
import {clamp, focusWithoutScrolling, mergeProps, useFormReset, useGlobalListeners, useInputEvent} from '@react-aria/utils';
import {DOMAttributes, RefObject} from '@react-types/shared';
import {getSliderThumbId, sliderData} from './utils';
import React, {ChangeEvent, InputHTMLAttributes, LabelHTMLAttributes, useCallback, useEffect, useRef} from 'react';
Expand Down Expand Up @@ -232,6 +232,22 @@ export function useSliderThumb(
state.setThumbValue(index, v);
});

let {onChange, onChangeEnd} = state;
useInputEvent<number>(inputRef, useCallback(fn => {
return onChange((changedIndex, value) => {
if (index === changedIndex) {
fn(value);
}
});
}, [onChange, index]), 'input');
useInputEvent<number>(inputRef, useCallback(fn => {
return onChangeEnd((changedIndex, value) => {
if (index === changedIndex) {
fn(value);
}
});
}, [onChangeEnd, index]), 'change');

// We install mouse handlers for the drag motion on the thumb div, but
// not the key handler for moving the thumb with the slider. Instead,
// we focus the range input, and let the browser handle the keyboard
Expand All @@ -255,7 +271,9 @@ export function useSliderThumb(
'aria-describedby': [data['aria-describedby'], opts['aria-describedby']].filter(Boolean).join(' '),
'aria-details': [data['aria-details'], opts['aria-details']].filter(Boolean).join(' '),
onChange: (e: ChangeEvent<HTMLInputElement>) => {
state.setThumbValue(index, parseFloat(e.target.value));
if (!e.nativeEvent['__reactAriaIgnore']) {
state.setThumbValue(index, parseFloat(e.target.value));
}
}
}),
thumbProps: {
Expand Down
6 changes: 5 additions & 1 deletion packages/@react-aria/textfield/src/useTextField.ts
Original file line number Diff line number Diff line change
Expand Up @@ -206,7 +206,11 @@ export function useTextField<T extends TextFieldIntrinsicElements = DefaultEleme
'aria-haspopup': props['aria-haspopup'],
'aria-controls': props['aria-controls'],
value,
onChange: (e: ChangeEvent<HTMLInputElement>) => setValue(e.target.value),
onChange: (e: ChangeEvent<HTMLInputElement>) => {
if (!e.nativeEvent['__reactAriaIgnore']) {
setValue(e.target.value);
}
},
autoComplete: props.autoComplete,
autoCapitalize: props.autoCapitalize,
maxLength: props.maxLength,
Expand Down
1 change: 1 addition & 0 deletions packages/@react-aria/textfield/test/useTextField.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,7 @@ describe('useTextField hook', () => {
let onChange = jest.fn();
let props = renderTextFieldHook({onChange, 'aria-label': 'mandatory label'});
let mockEvent = {
nativeEvent: {},
target: {
value: 1
}
Expand Down
3 changes: 0 additions & 3 deletions packages/@react-aria/toggle/src/useToggle.ts
Original file line number Diff line number Diff line change
Expand Up @@ -58,9 +58,6 @@ export function useToggle(props: AriaToggleProps, state: ToggleState, ref: RefOb
} = props;

let onChange = (e) => {
// since we spread props on label, onChange will end up there as well as in here.
// so we have to stop propagation at the lowest level that we care about
e.stopPropagation();
state.setSelected(e.target.checked);
};

Expand Down
1 change: 1 addition & 0 deletions packages/@react-aria/utils/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -51,5 +51,6 @@ export {CLEAR_FOCUS_EVENT, FOCUS_EVENT} from './constants';
export {isCtrlKeyPressed} from './keyboard';
export {useEnterAnimation, useExitAnimation} from './animation';
export {isFocusable, isTabbable} from './isFocusable';
export {useInputEvent} from './useInputEvent';

export type {LoadMoreSentinelProps} from './useLoadMoreSentinel';
34 changes: 34 additions & 0 deletions packages/@react-aria/utils/src/useInputEvent.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
import {RefObject, useEffect} from 'react';

export function useInputEvent<T>(
ref: RefObject<HTMLSelectElement | HTMLInputElement | HTMLTextAreaElement | null>,
subscribe: (fn: (value: T) => void) => () => void,
type?: 'input' | 'change'
): void {
useEffect(() => {
return subscribe(value => {
let el = ref.current;
if (el && window.event?.type !== 'reset' && (type === 'change' || el.value !== String(value ?? ''))) {
// Use native input element value property setter from the element's prototype.
// React overrides the setter directly on the element and ignores the input event.
// This matches more closely what the browser does natively (setter is not triggered).
// See https://stackoverflow.com/questions/23892547/what-is-the-best-way-to-trigger-change-or-input-event-in-react-js
let proto = Object.getPrototypeOf(el);
let setValue = Object.getOwnPropertyDescriptor(proto, 'value')!.set!;
setValue.call(el, String(value ?? ''));

if (!type || type === 'input') {
let inputEvent = new Event('input', {bubbles: true});
inputEvent['__reactAriaIgnore'] = true;
el.dispatchEvent(inputEvent);
Copy link
Member

Choose a reason for hiding this comment

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

don't love that this starts a new capture phase for the event

Copy link
Member Author

Choose a reason for hiding this comment

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

Hmm? It's not re-dispatching an existing event, it's totally new. So isn't that expected?

}

if (!type || type === 'change') {
let changeEvent = new Event('change', {bubbles: true});
changeEvent['__reactAriaIgnore'] = true;
el.dispatchEvent(changeEvent);
}
}
});
}, [ref, subscribe, type]);
}
Original file line number Diff line number Diff line change
Expand Up @@ -2512,16 +2512,15 @@ describe('SearchAutocomplete', function () {
jest.runAllTimers();
});

expect(input).toHaveAttribute('aria-describedby');

let listbox = getByRole('listbox');
let items = within(listbox).getAllByRole('option');
await user.click(items[0]);
act(() => {
jest.runAllTimers();
});

expect(input).toHaveAttribute('aria-describedby');

await user.tab();
expect(input).not.toHaveAttribute('aria-describedby');
});

Expand Down Expand Up @@ -2550,17 +2549,15 @@ describe('SearchAutocomplete', function () {
jest.runAllTimers();
});

expect(input).toHaveAttribute('aria-describedby');

let listbox = getByRole('listbox');
let items = within(listbox).getAllByRole('option');
await user.click(items[0]);
act(() => {
jest.runAllTimers();
});

expect(input).toHaveAttribute('aria-describedby');

await user.tab();

expect(input).not.toHaveAttribute('aria-describedby');
expect(input.validity.valid).toBe(true);
});
Expand Down Expand Up @@ -2603,16 +2600,15 @@ describe('SearchAutocomplete', function () {
jest.runAllTimers();
});

expect(input).toHaveAttribute('aria-describedby');

let listbox = getByRole('listbox');
let items = within(listbox).getAllByRole('option');
await user.click(items[0]);
act(() => {
jest.runAllTimers();
});

expect(input).toHaveAttribute('aria-describedby');
await user.tab();

expect(input).not.toHaveAttribute('aria-describedby');
expect(input.validity.valid).toBe(true);
});
Expand Down
16 changes: 6 additions & 10 deletions packages/@react-spectrum/combobox/test/ComboBox.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -5335,16 +5335,15 @@ describe('ComboBox', function () {
jest.runAllTimers();
});

expect(input).toHaveAttribute('aria-describedby');

let listbox = getByRole('listbox');
let items = within(listbox).getAllByRole('option');
await user.click(items[0]);
act(() => {
jest.runAllTimers();
});

expect(input).toHaveAttribute('aria-describedby');

await user.tab();
expect(input).not.toHaveAttribute('aria-describedby');
});

Expand Down Expand Up @@ -5372,17 +5371,15 @@ describe('ComboBox', function () {
jest.runAllTimers();
});

expect(input).toHaveAttribute('aria-describedby');

let listbox = getByRole('listbox');
let items = within(listbox).getAllByRole('option');
await user.click(items[0]);
act(() => {
jest.runAllTimers();
});

expect(input).toHaveAttribute('aria-describedby');

await user.tab();

expect(input).not.toHaveAttribute('aria-describedby');
expect(input.validity.valid).toBe(true);
});
Expand Down Expand Up @@ -5425,16 +5422,15 @@ describe('ComboBox', function () {
jest.runAllTimers();
});

expect(input).toHaveAttribute('aria-describedby');

let listbox = getByRole('listbox');
let items = within(listbox).getAllByRole('option');
await user.click(items[0]);
act(() => {
jest.runAllTimers();
});

expect(input).toHaveAttribute('aria-describedby');
await user.tab();

expect(input).not.toHaveAttribute('aria-describedby');
expect(input.validity.valid).toBe(true);
});
Expand Down
31 changes: 29 additions & 2 deletions packages/@react-spectrum/picker/test/Picker.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -1210,7 +1210,7 @@ describe('Picker', function () {
it('supports controlled selection', async function () {
let {getByRole} = render(
<Provider theme={theme}>
<Picker label="Test" selectedKey="two" onSelectionChange={onSelectionChange}>
<Picker label="Test" selectedKey="two" name="test" onSelectionChange={onSelectionChange}>
<Item key="one">One</Item>
<Item key="two">Two</Item>
<Item key="three">Three</Item>
Expand All @@ -1219,7 +1219,9 @@ describe('Picker', function () {
);

let picker = getByRole('button');
let input = document.querySelector('select[name=test]');
expect(picker).toHaveTextContent('Two');
expect(input).toHaveValue('two');
await user.click(picker);
act(() => jest.runAllTimers());

Expand Down Expand Up @@ -1249,6 +1251,7 @@ describe('Picker', function () {
act(() => jest.runAllTimers());
expect(document.activeElement).toBe(picker);
expect(picker).toHaveTextContent('Two');
expect(input).toHaveValue('two');
});

it('supports default selection', async function () {
Expand Down Expand Up @@ -2109,7 +2112,6 @@ describe('Picker', function () {
expect(input).toHaveValue('one');
});


it('should support form prop', () => {
render(
<Provider theme={theme}>
Expand Down Expand Up @@ -2157,6 +2159,31 @@ describe('Picker', function () {
});
}

it('onChange event bubbles to form', async function () {
let onChange = jest.fn();
let {getByTestId, getByRole} = render(
<Provider theme={theme}>
<form onChange={onChange}>
<Picker name="picker" data-testid="picker" label="Test">
<Item key="one">One</Item>
<Item key="two">Two</Item>
<Item key="three">Three</Item>
</Picker>
</form>
</Provider>
);
let picker = getByTestId('picker');
await user.click(picker);
act(() => jest.runAllTimers());

let listbox = getByRole('listbox');
let items = within(listbox).getAllByRole('option');
expect(items.length).toBe(3);
await user.click(items[1]);
expect(onChange).toHaveBeenCalledTimes(1);
expect(onChange.mock.lastCall[0].target.value).toBe('two');
});

describe('validation', () => {
describe('validationBehavior=native', () => {
it('supports isRequired', async () => {
Expand Down
Loading