-
Notifications
You must be signed in to change notification settings - Fork 1.3k
fix: Autocomplete context refactor #8695
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
Changes from 11 commits
c111744
24dd7cb
d8969a7
ae3cc84
3a81f31
db91b3a
7177106
dfb80f3
ec8066f
76107ff
cde0977
82cb364
18d3dbc
7cbf8a1
8bfe6d2
5a72a98
05aca0b
6cfdb09
213b1f4
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -10,7 +10,7 @@ | |
| * governing permissions and limitations under the License. | ||
| */ | ||
|
|
||
| import {AriaLabelingProps, BaseEvent, DOMProps, Node, RefObject} from '@react-types/shared'; | ||
| import {AriaLabelingProps, BaseEvent, DOMProps, FocusableElement, Node, RefObject} from '@react-types/shared'; | ||
| import {AriaTextFieldProps} from '@react-aria/textfield'; | ||
| import {AutocompleteProps, AutocompleteState} from '@react-stately/autocomplete'; | ||
| import {CLEAR_FOCUS_EVENT, FOCUS_EVENT, getActiveElement, getOwnerDocument, isCtrlKeyPressed, mergeProps, mergeRefs, useEffectEvent, useEvent, useLabels, useObjectRef, useSlotId} from '@react-aria/utils'; | ||
|
|
@@ -40,7 +40,12 @@ export interface AriaAutocompleteProps<T> extends AutocompleteProps { | |
| * Whether or not to focus the first item in the collection after a filter is performed. | ||
| * @default false | ||
| */ | ||
| disableAutoFocusFirst?: boolean | ||
| disableAutoFocusFirst?: boolean, | ||
|
|
||
| /** | ||
| * Whether the autocomplete should disable virtual focus, instead making the wrapped collection directly tabbable. | ||
| */ | ||
| disallowVirtualFocus?: boolean | ||
LFDanLu marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| } | ||
|
|
||
| export interface AriaAutocompleteOptions<T> extends Omit<AriaAutocompleteProps<T>, 'children'> { | ||
|
|
@@ -52,7 +57,7 @@ export interface AriaAutocompleteOptions<T> extends Omit<AriaAutocompleteProps<T | |
|
|
||
| export interface AutocompleteAria<T> { | ||
| /** Props for the autocomplete textfield/searchfield element. These should be passed to the textfield/searchfield aria hooks respectively. */ | ||
| textFieldProps: AriaTextFieldProps, | ||
| textFieldProps: AriaTextFieldProps<FocusableElement>, | ||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. maybe more appropriate to make these types more broad |
||
| /** 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. */ | ||
|
|
@@ -72,7 +77,8 @@ export function useAutocomplete<T>(props: AriaAutocompleteOptions<T>, state: Aut | |
| inputRef, | ||
| collectionRef, | ||
| filter, | ||
| disableAutoFocusFirst = false | ||
| disableAutoFocusFirst = false, | ||
| disallowVirtualFocus = false | ||
| } = props; | ||
|
|
||
| let collectionId = useSlotId(); | ||
|
|
@@ -83,7 +89,7 @@ export function useAutocomplete<T>(props: AriaAutocompleteOptions<T>, state: Aut | |
|
|
||
| // For mobile screen readers, we don't want virtual focus, instead opting to disable FocusScope's restoreFocus and manually | ||
| // moving focus back to the subtriggers | ||
| let shouldUseVirtualFocus = getInteractionModality() !== 'virtual'; | ||
| let shouldUseVirtualFocus = getInteractionModality() !== 'virtual' && !disallowVirtualFocus; | ||
|
|
||
| useEffect(() => { | ||
| return () => clearTimeout(timeout.current); | ||
|
|
@@ -254,15 +260,17 @@ export function useAutocomplete<T>(props: AriaAutocompleteOptions<T>, state: Aut | |
| } | ||
|
|
||
| let shouldPerformDefaultAction = true; | ||
| if (focusedNodeId == null) { | ||
| shouldPerformDefaultAction = collectionRef.current?.dispatchEvent( | ||
| new KeyboardEvent(e.nativeEvent.type, e.nativeEvent) | ||
| ) || false; | ||
| } else { | ||
| let item = document.getElementById(focusedNodeId); | ||
| shouldPerformDefaultAction = item?.dispatchEvent( | ||
| new KeyboardEvent(e.nativeEvent.type, e.nativeEvent) | ||
| ) || false; | ||
| if (collectionRef.current !== null) { | ||
| if (focusedNodeId == null) { | ||
| shouldPerformDefaultAction = collectionRef.current?.dispatchEvent( | ||
| new KeyboardEvent(e.nativeEvent.type, e.nativeEvent) | ||
| ) || false; | ||
| } else { | ||
| let item = document.getElementById(focusedNodeId); | ||
| shouldPerformDefaultAction = item?.dispatchEvent( | ||
| new KeyboardEvent(e.nativeEvent.type, e.nativeEvent) | ||
| ) || false; | ||
| } | ||
| } | ||
|
|
||
| if (shouldPerformDefaultAction) { | ||
|
|
@@ -282,6 +290,9 @@ export function useAutocomplete<T>(props: AriaAutocompleteOptions<T>, state: Aut | |
| } | ||
| break; | ||
| } | ||
| } else { | ||
| // TODO: check if we can do this, want to stop textArea from using its default Enter behavior so items are properly triggered | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. todo?
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. it seems to work, but I'm realizing with the grid virtual focus work that there are a ton of configurations to test haha |
||
| e.preventDefault(); | ||
| } | ||
| }; | ||
|
|
||
|
|
@@ -359,25 +370,28 @@ export function useAutocomplete<T>(props: AriaAutocompleteOptions<T>, state: Aut | |
| let textFieldProps = { | ||
| value: state.inputValue, | ||
| onChange | ||
| } as AriaTextFieldProps<HTMLInputElement>; | ||
| } as AriaTextFieldProps<FocusableElement>; | ||
|
|
||
| let virtualFocusProps = { | ||
| onKeyDown, | ||
| 'aria-activedescendant': state.focusedNodeId ?? undefined, | ||
| onBlur, | ||
| onFocus | ||
| }; | ||
|
|
||
| if (collectionId) { | ||
| textFieldProps = { | ||
| ...textFieldProps, | ||
| onKeyDown, | ||
| autoComplete: 'off', | ||
| 'aria-haspopup': collectionId ? 'listbox' : undefined, | ||
| ...(shouldUseVirtualFocus && virtualFocusProps), | ||
| enterKeyHint: 'go', | ||
| 'aria-controls': collectionId, | ||
| // TODO: readd proper logic for completionMode = complete (aria-autocomplete: both) | ||
| 'aria-autocomplete': 'list', | ||
| 'aria-activedescendant': state.focusedNodeId ?? undefined, | ||
| // This disable's iOS's autocorrect suggestions, since the autocomplete provides its own suggestions. | ||
| autoCorrect: 'off', | ||
| // This disable's the macOS Safari spell check auto corrections. | ||
| spellCheck: 'false', | ||
| enterKeyHint: 'go', | ||
| onBlur, | ||
| onFocus | ||
| autoComplete: 'off' | ||
| }; | ||
| } | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -10,29 +10,44 @@ | |
| * governing permissions and limitations under the License. | ||
| */ | ||
|
|
||
| import {AriaAutocompleteProps, CollectionOptions, useAutocomplete} from '@react-aria/autocomplete'; | ||
| import {AriaAutocompleteProps, useAutocomplete} from '@react-aria/autocomplete'; | ||
| import {AriaLabelingProps, DOMProps, FocusableElement, FocusEvents, KeyboardEvents, Node, ValueBase} from '@react-types/shared'; | ||
| import {AriaTextFieldProps} from '@react-aria/textfield'; | ||
| import {AutocompleteState, useAutocompleteState} from '@react-stately/autocomplete'; | ||
| import {InputContext} from './Input'; | ||
| import {ContextValue, Provider, removeDataAttributes, SlotProps, SlottedContextValue, useSlottedContext} from './utils'; | ||
| import {mergeProps} from '@react-aria/utils'; | ||
| import {Node} from '@react-types/shared'; | ||
| import {Provider, removeDataAttributes, SlotProps, SlottedContextValue, useSlottedContext} from './utils'; | ||
| import React, {createContext, JSX, RefObject, useRef} from 'react'; | ||
| import {SearchFieldContext} from './SearchField'; | ||
| import {TextFieldContext} from './TextField'; | ||
| import React, {createContext, JSX, useRef} from 'react'; | ||
|
|
||
| export interface AutocompleteProps<T> extends AriaAutocompleteProps<T>, SlotProps {} | ||
|
|
||
| interface InternalAutocompleteContextValue<T> { | ||
| // TODO: naming | ||
| // IMO I think this could also contain the props that useSelectableCollection takes (minus the selection options?) | ||
| interface CollectionContextValue<T> extends DOMProps, AriaLabelingProps { | ||
LFDanLu marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| filter?: (nodeTextValue: string, node: Node<T>) => boolean, | ||
| collectionProps: CollectionOptions, | ||
| collectionRef: RefObject<HTMLElement | null> | ||
| /** Whether the collection items should use virtual focus instead of being focused directly. */ | ||
| shouldUseVirtualFocus?: boolean, | ||
| /** Whether typeahead is disabled. */ | ||
| disallowTypeAhead?: boolean | ||
| } | ||
|
|
||
| // TODO: naming | ||
| interface FieldInputContextValue<T = FocusableElement> extends | ||
| DOMProps, | ||
| FocusEvents<T>, | ||
| KeyboardEvents, | ||
| Pick<ValueBase<string>, 'onChange' | 'value'>, | ||
| Pick<AriaTextFieldProps, 'enterKeyHint' | 'aria-controls' | 'aria-autocomplete' | 'aria-activedescendant' | 'spellCheck' | 'autoCorrect' | 'autoComplete'> {} | ||
|
|
||
| export const AutocompleteContext = createContext<SlottedContextValue<Partial<AutocompleteProps<any>>>>(null); | ||
| export const AutocompleteStateContext = createContext<AutocompleteState | null>(null); | ||
| // This context is to pass the register and filter down to whatever collection component is wrapped by the Autocomplete | ||
| // TODO: export from RAC, but rename to something more appropriate | ||
| export const UNSTABLE_InternalAutocompleteContext = createContext<InternalAutocompleteContextValue<any> | null>(null); | ||
|
|
||
| // TODO export from RAC, maybe move up and out of Autocomplete | ||
| // also can't make this use ContextValue (so that we can call useContextProps) like FieldInput for a similar reason. The HTMLElement type for the ref | ||
| // makes useContextProps complain since it doesn't mesh up with HTMLDivElement | ||
|
||
| export const CollectionContext = createContext<ContextValue<CollectionContextValue<any>, HTMLElement>>(null); | ||
| // TODO: too restrictive to type this as a HTMLInputElement? Needed for the ref merging that happens in TextField/SearchField | ||
| // Attempted to use FocusableElement but as mentioned above, SearchField and TextField complain since they expect HTMLInputElement for their hooks and stuff | ||
LFDanLu marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| export const FieldInputContext = createContext<ContextValue<FieldInputContextValue, FocusableElement>>(null); | ||
|
|
||
| /** | ||
| * An autocomplete combines a TextField or SearchField with a Menu or ListBox, allowing users to search or filter a list of suggestions. | ||
|
|
@@ -61,13 +76,14 @@ export function Autocomplete<T extends object>(props: AutocompleteProps<T>): JSX | |
| <Provider | ||
| values={[ | ||
| [AutocompleteStateContext, state], | ||
| [SearchFieldContext, textFieldProps], | ||
| [TextFieldContext, textFieldProps], | ||
| [InputContext, {ref: inputRef}], | ||
| [UNSTABLE_InternalAutocompleteContext, { | ||
| filter: filterFn as (nodeTextValue: string, node: Node<T>) => boolean, | ||
| collectionProps, | ||
| collectionRef: mergedCollectionRef | ||
| [FieldInputContext, { | ||
| ...textFieldProps, | ||
| ref: inputRef | ||
| }], | ||
| [CollectionContext, { | ||
| ...collectionProps, | ||
| filter: filterFn, | ||
| ref: mergedCollectionRef | ||
| }] | ||
| ]}> | ||
| {props.children} | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -13,6 +13,7 @@ import {AriaGridListProps, DraggableItemResult, DragPreviewRenderer, DropIndicat | |
| import {ButtonContext} from './Button'; | ||
| import {CheckboxContext} from './RSPContexts'; | ||
| import {Collection, CollectionBuilder, createLeafComponent, FilterLessNode, ItemNode} from '@react-aria/collections'; | ||
| import {CollectionContext} from './Autocomplete'; | ||
| import {CollectionProps, CollectionRendererContext, DefaultCollectionRenderer, ItemRenderProps} from './Collection'; | ||
| import {ContextValue, DEFAULT_SLOT, Provider, RenderProps, SlotProps, StyleProps, StyleRenderProps, useContextProps, useRenderProps} from './utils'; | ||
| import {DragAndDropContext, DropIndicatorContext, DropIndicatorProps, useDndPersistedKeys, useRenderDropIndicator} from './DragAndDrop'; | ||
|
|
@@ -23,7 +24,6 @@ import {forwardRefType, GlobalDOMAttributes, HoverEvents, Key, LinkDOMProps, Pre | |
| import {ListStateContext} from './ListBox'; | ||
| import React, {createContext, ForwardedRef, forwardRef, HTMLAttributes, JSX, ReactNode, useContext, useEffect, useMemo, useRef} from 'react'; | ||
| import {TextContext} from './Text'; | ||
| import {UNSTABLE_InternalAutocompleteContext} from './Autocomplete'; | ||
|
|
||
| export interface GridListRenderProps { | ||
| /** | ||
|
|
@@ -106,7 +106,9 @@ interface GridListInnerProps<T extends object> { | |
| function GridListInner<T extends object>({props, collection, gridListRef: ref}: GridListInnerProps<T>) { | ||
| // TODO: for now, don't grab collection ref and collectionProps from the autocomplete, rely on the user tabbing to the gridlist | ||
| // figure out if we want to support virtual focus for grids when wrapped in an autocomplete | ||
| let {filter, collectionProps} = useContext(UNSTABLE_InternalAutocompleteContext) || {}; | ||
| let contextProps; | ||
| [contextProps] = useContextProps({}, null, CollectionContext); | ||
|
||
| let {filter, ...collectionProps} = contextProps; | ||
| // eslint-disable-next-line @typescript-eslint/no-unused-vars | ||
| let {shouldUseVirtualFocus, disallowTypeAhead, ...DOMCollectionProps} = collectionProps || {}; | ||
| let {dragAndDropHooks, keyboardNavigationBehavior = 'arrow', layout = 'stack'} = props; | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
disableVirtualFocus i think is a better name, it's not really about allowing or not, it's just whether it is or not
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I debated about this myself a bit haha. I went with the above since it means the default is false as per our API guidelines, but I certainly do like
disablebetter (as can be seen fromdisableAutoFocusFirst)EDIT: I also just realized I somehow thought the default for disableVirtualFocus would be true lol, will make the change
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
another thing to discuss perhaps: is it a problem that virtual focus doesn't work with grid collection just yet but will be enabled by default later?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
that's an excellent question, does seem problematic