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: Properly clear Autocomplete wrapped SearchField when "x" button is pressed #7606

Merged
merged 6 commits into from
Jan 14, 2025
Merged
Show file tree
Hide file tree
Changes from 4 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: 1 addition & 0 deletions packages/@react-aria/autocomplete/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
"@react-aria/interactions": "^3.22.5",
"@react-aria/listbox": "^3.13.6",
"@react-aria/searchfield": "^3.7.11",
"@react-aria/textfield": "^3.15.0",
"@react-aria/utils": "^3.26.0",
"@react-stately/autocomplete": "3.0.0-alpha.1",
"@react-stately/combobox": "^3.10.1",
Expand Down
25 changes: 9 additions & 16 deletions packages/@react-aria/autocomplete/src/useAutocomplete.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,12 +11,12 @@
*/

import {AriaLabelingProps, BaseEvent, DOMProps, RefObject} from '@react-types/shared';
import {AriaTextFieldProps} from '@react-aria/textfield';
import {AutocompleteProps, AutocompleteState} from '@react-stately/autocomplete';
import {ChangeEvent, InputHTMLAttributes, KeyboardEvent as ReactKeyboardEvent, useCallback, useEffect, useMemo, useRef} from 'react';
import {CLEAR_FOCUS_EVENT, FOCUS_EVENT, isCtrlKeyPressed, mergeProps, mergeRefs, UPDATE_ACTIVEDESCENDANT, useEffectEvent, useId, useLabels, useObjectRef} from '@react-aria/utils';
// @ts-ignore
import intlMessages from '../intl/*.json';
import {useKeyboard} from '@react-aria/interactions';
import {KeyboardEvent as ReactKeyboardEvent, useCallback, useEffect, useMemo, useRef} from 'react';
import {useLocalizedStringFormatter} from '@react-aria/i18n';

export interface CollectionOptions extends DOMProps, AriaLabelingProps {
Expand All @@ -41,8 +41,8 @@ export interface AriaAutocompleteOptions extends Omit<AriaAutocompleteProps, 'ch
}

export interface AutocompleteAria {
/** Props for the autocomplete input element. */
inputProps: InputHTMLAttributes<HTMLInputElement>,
/** Props for the autocomplete textfield/searchfield element. These should be passed to the textfield/searchfield aria hooks respectively. */
textFieldProps: AriaTextFieldProps,
/** Props for the collection, to be passed to collection's respective aria hook (e.g. useMenu). */
collectionProps: CollectionOptions,
/** Ref to attach to the wrapped collection. */
Expand Down Expand Up @@ -138,16 +138,16 @@ export function UNSTABLE_useAutocomplete(props: AriaAutocompleteOptions, state:
});

// TODO: update to see if we can tell what kind of event (paste vs backspace vs typing) is happening instead
let onChange = (e: ChangeEvent<HTMLInputElement>) => {
let onChange = (value: string) => {
// Tell wrapped collection to focus the first element in the list when typing forward and to clear focused key when deleting text
// for screen reader announcements
if (state.inputValue !== e.target.value && state.inputValue.length <= e.target.value.length) {
if (state.inputValue !== value && state.inputValue.length <= value.length) {
focusFirstItem();
} else {
clearVirtualFocus();
}

state.setInputValue(e.target.value);
state.setInputValue(value);
};

// For textfield specific keydown operations
Expand All @@ -166,12 +166,7 @@ export function UNSTABLE_useAutocomplete(props: AriaAutocompleteOptions, state:
// Early return for Escape here so it doesn't leak the Escape event from the simulated collection event below and
// close the dialog prematurely. Ideally that should be up to the discretion of the input element hence the check
// for isPropagationStopped
// Also set the inputValue to '' to cover Firefox case where Esc doesn't actually clear searchfields. Normally we already
// handle this in useSearchField, but we are directly setting the inputValue on the input element in RAC Autocomplete instead of
// passing it to the SearchField via props. This means that a controlled value set on the Autocomplete isn't synced up with the
// SearchField until the user makes a change to the field's value via typing
if (e.isPropagationStopped()) {
Copy link
Member

Choose a reason for hiding this comment

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

Is this still needed?

state.setInputValue('');
return;
}
break;
Expand Down Expand Up @@ -249,8 +244,6 @@ export function UNSTABLE_useAutocomplete(props: AriaAutocompleteOptions, state:
};
}, [inputRef, onKeyUpCapture]);

let {keyboardProps} = useKeyboard({onKeyDown});

let stringFormatter = useLocalizedStringFormatter(intlMessages, '@react-aria/autocomplete');
let collectionProps = useLabels({
id: collectionId,
Expand All @@ -266,10 +259,10 @@ export function UNSTABLE_useAutocomplete(props: AriaAutocompleteOptions, state:
}, [state.inputValue, filter]);

return {
inputProps: {
textFieldProps: {
value: state.inputValue,
onChange,
...keyboardProps,
onKeyDown,
autoComplete: 'off',
'aria-haspopup': 'listbox',
'aria-controls': collectionId,
Expand Down
3 changes: 3 additions & 0 deletions packages/@react-aria/textfield/src/useTextField.ts
Original file line number Diff line number Diff line change
Expand Up @@ -174,6 +174,7 @@ export function useTextField<T extends TextFieldIntrinsicElements = DefaultEleme
'aria-activedescendant': props['aria-activedescendant'],
'aria-autocomplete': props['aria-autocomplete'],
'aria-haspopup': props['aria-haspopup'],
'aria-controls': props['aria-controls'],
value,
onChange: (e: ChangeEvent<HTMLInputElement>) => setValue(e.target.value),
autoComplete: props.autoComplete,
Expand All @@ -183,6 +184,8 @@ export function useTextField<T extends TextFieldIntrinsicElements = DefaultEleme
name: props.name,
placeholder: props.placeholder,
inputMode: props.inputMode,
autoCorrect: props.autoCorrect,
spellCheck: props.spellCheck,

// Clipboard events
onCopy: props.onCopy,
Expand Down
12 changes: 11 additions & 1 deletion packages/@react-types/shared/src/dom.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -166,7 +166,17 @@ export interface TextInputDOMProps extends DOMProps, InputDOMProps, TextInputDOM
/**
* Hints at the type of data that might be entered by the user while editing the element or its contents. See [MDN](https://html.spec.whatwg.org/multipage/interaction.html#input-modalities:-the-inputmode-attribute).
*/
inputMode?: 'none' | 'text' | 'tel' | 'url' | 'email' | 'numeric' | 'decimal' | 'search'
inputMode?: 'none' | 'text' | 'tel' | 'url' | 'email' | 'numeric' | 'decimal' | 'search',

/**
* An attribute that takes as its value a space-separated string that describes what, if any, type of autocomplete functionality the input should provide. See [MDN](https://developer.mozilla.org/en-US/docs/Web/HTML/Element/input#autocomplete).
*/
autoCorrect?: string,

/**
* An enumerated attribute that defines whether the element may be checked for spelling errors. See [MDN](https://developer.mozilla.org/en-US/docs/Web/HTML/Global_attributes/spellcheck).
*/
spellCheck?: string
}

/**
Expand Down
4 changes: 3 additions & 1 deletion packages/@react-types/textfield/src/index.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,9 @@ export interface AriaTextFieldProps<T = HTMLInputElement> extends TextFieldProps
*/
'aria-autocomplete'?: 'none' | 'inline' | 'list' | 'both',
/** Indicates the availability and type of interactive popup element, such as menu or dialog, that can be triggered by an element. */
'aria-haspopup'?: boolean | 'false' | 'true' | 'menu' | 'listbox' | 'tree' | 'grid' | 'dialog'
'aria-haspopup'?: boolean | 'false' | 'true' | 'menu' | 'listbox' | 'tree' | 'grid' | 'dialog',
/** Identifies the element (or elements) whose contents or presence are controlled by the current element. */
'aria-controls'?: string
}

interface SpectrumTextFieldBaseProps {
Expand Down
8 changes: 6 additions & 2 deletions packages/react-aria-components/src/Autocomplete.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@ import {InputContext} from './Input';
import {mergeProps} from '@react-aria/utils';
import {Provider, removeDataAttributes, SlotProps, SlottedContextValue, useSlottedContext} from './utils';
import React, {createContext, RefObject, useRef} from 'react';
import {SearchFieldContext} from './SearchField';
import {TextFieldContext} from './TextField';

export interface AutocompleteProps extends AriaAutocompleteProps, SlotProps {}

Expand Down Expand Up @@ -43,7 +45,7 @@ export function UNSTABLE_Autocomplete(props: AutocompleteProps) {
let inputRef = useRef<HTMLInputElement>(null);

let {
inputProps,
textFieldProps,
collectionProps,
collectionRef: mergedCollectionRef,
filterFn
Expand All @@ -58,7 +60,9 @@ export function UNSTABLE_Autocomplete(props: AutocompleteProps) {
<Provider
values={[
[UNSTABLE_AutocompleteStateContext, state],
[InputContext, {...inputProps, ref: inputRef}],
[SearchFieldContext, textFieldProps],
[TextFieldContext, textFieldProps],
[InputContext, {ref: inputRef}],
Copy link
Member

Choose a reason for hiding this comment

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

can't we pass the inputRef to SearchField or TextField Context now?

Copy link
Member

Choose a reason for hiding this comment

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

if so can we remove the InputContext merging?

Copy link
Member Author

Choose a reason for hiding this comment

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

if I pass the ref down through context, it will be attached to the wrapping div rather than the input itself so I still need the InputContext. If I can get rid of the checks against inputRef in useAutocomplete then I can get rid of it but digging for alternatives right now

Copy link
Member

Choose a reason for hiding this comment

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

remind me why we need a document-level capturing listener?

Copy link
Member Author

Choose a reason for hiding this comment

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

We need to dispatch a keyup event onto the ListBox link item so that we can properly simulate a Enter press and trigger a the Listbox's link navigation

[UNSTABLE_InternalAutocompleteContext, {
filterFn,
collectionProps,
Expand Down
23 changes: 22 additions & 1 deletion packages/react-aria-components/test/Autocomplete.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
*/

import {AriaAutocompleteTests} from './AriaAutocomplete.test-util';
import {Header, Input, Label, ListBox, ListBoxItem, ListBoxSection, Menu, MenuItem, MenuSection, SearchField, Separator, Text, UNSTABLE_Autocomplete} from '..';
import {Button, Header, Input, Label, ListBox, ListBoxItem, ListBoxSection, Menu, MenuItem, MenuSection, SearchField, Separator, Text, UNSTABLE_Autocomplete} from '..';
import {pointerMap, render} from '@react-spectrum/test-utils-internal';
import React, {ReactNode} from 'react';
import {useAsyncList} from 'react-stately';
Expand Down Expand Up @@ -110,6 +110,7 @@ let AutocompleteWrapper = ({autocompleteProps = {}, inputProps = {}, children}:
<SearchField {...inputProps}>
<Label style={{display: 'block'}}>Test</Label>
<Input />
<Button>✕</Button>
<Text style={{display: 'block'}} slot="description">Please select an option below.</Text>
</SearchField>
{children}
Expand Down Expand Up @@ -199,6 +200,26 @@ describe('Autocomplete', () => {
expect(onKeyDown).not.toHaveBeenCalled();
onKeyDown.mockReset();
});

it('should clear the input field when clicking on the clear button', async () => {
let {getByRole} = render(
<AutocompleteWrapper>
<StaticMenu />
</AutocompleteWrapper>
);

let input = getByRole('searchbox');
await user.tab();
expect(document.activeElement).toBe(input);
await user.keyboard('Foo');
expect(input).toHaveValue('Foo');

let button = getByRole('button');
expect(button).toHaveAttribute('aria-label', 'Clear search');
await user.click(button);

expect(input).toHaveValue('');
});
});

AriaAutocompleteTests({
Expand Down
1 change: 1 addition & 0 deletions yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -5916,6 +5916,7 @@ __metadata:
"@react-aria/interactions": "npm:^3.22.5"
"@react-aria/listbox": "npm:^3.13.6"
"@react-aria/searchfield": "npm:^3.7.11"
"@react-aria/textfield": "npm:^3.15.0"
"@react-aria/utils": "npm:^3.26.0"
"@react-stately/autocomplete": "npm:3.0.0-alpha.1"
"@react-stately/combobox": "npm:^3.10.1"
Expand Down
Loading