diff --git a/packages/example-app/src/SearchEverywhere/SearchEverywherePopup.tsx b/packages/example-app/src/SearchEverywhere/SearchEverywherePopup.tsx index ae29d3f3..e1c1ffa1 100644 --- a/packages/example-app/src/SearchEverywhere/SearchEverywherePopup.tsx +++ b/packages/example-app/src/SearchEverywhere/SearchEverywherePopup.tsx @@ -17,7 +17,6 @@ import { Tabs, Tooltip, TooltipTrigger, - useCollectionSearchInput, useGetActionShortcut, } from "@intellij-platform/core"; import { useTips } from "./useTips"; @@ -276,7 +275,7 @@ export function SearchEverywherePopup() { const close = () => setOpen(false); - const collectionRef = useRef(null); + const searchInputRef = useRef(null); const selectionManagerRef = useRef(null); const onAction = (key: React.Key) => { if (key === LOAD_MORE_ITEM_KEY) { @@ -317,12 +316,6 @@ export function SearchEverywherePopup() { } }; - const { collectionSearchInputProps } = useCollectionSearchInput({ - collectionRef, - onAction, - selectionManagerRef, - }); - const tips = useTips(); return ( { event.target.select(); @@ -421,7 +414,7 @@ export function SearchEverywherePopup() { {pattern && ( (null); const searchInputRef = useRef(null); - const selectionManagerRef = useRef(null); const [isInputFocused, setInputFocused] = useState(false); const setBranchFilter = useSetRecoilState(vcsLogFilterCurrentTab.branch); const onAction = (key: React.Key) => { @@ -65,11 +57,6 @@ export function BranchesTree({ tabKey }: { tabKey: string }) { } }; - const { collectionSearchInputProps } = useCollectionSearchInput({ - collectionRef: ref, - selectionManagerRef, - onAction, - }); /** * TODO: remaining from search: * - Make the search input red when there is no match @@ -102,20 +89,18 @@ export function BranchesTree({ tabKey }: { tabKey: string }) { tabIndex={-1} value={searchTerm} onChange={(e) => setSearchTerm(e.target.value)} - {...mergeProps(collectionSearchInputProps, { - onFocus: () => { - setInputFocused(true); - }, - onBlur: () => { - setInputFocused(false); - }, - })} + onFocus={() => { + setInputFocused(true); + }} + onBlur={() => { + setInputFocused(false); + }} /> (null); const collectionRef = useRef(null); - const selectionManagerRef = useRef(null); const onAutocompleteSuggestionSelected = (key: React.Key) => { setValue(path.join(`${key}`, path.sep)); // TODO: handle completion with tab }; - const { collectionSearchInputProps } = useCollectionSearchInput({ - collectionRef, - onAction: onAutocompleteSuggestionSelected, - selectionManagerRef, - }); // Path search query is kept in a separate state because it needs to be updated when autocomplete action triggers. // Otherwise, it could be a computed value based on `isAutocompleteVisible` and `directory` const [pathSearchQuery, setPathSearchQuery] = useState(""); @@ -175,7 +167,6 @@ export function PathInputField({ inputProps={mergeProps( props.inputProps ?? {}, shortcutHandlerProps, - collectionSearchInputProps, { onKeyDown: (e: React.KeyboardEvent) => { if (e.key === "Escape") { @@ -218,7 +209,7 @@ export function PathInputField({ ; +} + +/** + * A solution for connecting a collection to a search input so that the collection can still be navigated by keyboard + * while the input is focused. It works by replaying certain keyboard events on the collection container and focused + * item. An alternative approach (which is used in react-aria's useCombobox) is to use useSelectableCollection + * separately for the input, but the biggest issue with that approach is that it's limiting in the following ways: + * - Rendering input should be a part of the same component that renders the collection. Having specific components + * for use cases that require a search input is not flexible enough. For example, one may want to use SpeedSearchList + * or List connected to an input. Also, the input and the collection may need to be in different layouts in different + * use cases. Decoupling the rendering of the input and collection is a more flexible solution. + * - The same options used for collection should be passed to the input field for behavior consistency, and that is + * prone to error. + * Some of these options, like `keyboardDelegate` can even have a default value in hooks like + * `useSelectableList`. + * It means making sure the same value is passed to the useSelectableCollection for input + * would require not using the default value, since the same value can't be accessed. + * + * With this event-forwarding approach, it's an arrow up or down event would behave exactly like it was triggered on + * the collection itself, leaving no room for behavior discrepancies. But it has a few drawbacks: + * - Although small, there is still some coupling between this code and implementation of the collection component. + * More specifically, this implementation assumes the following things: + * - "Enter" keys (selection or action) are handled on items, but arrow keys are handled on the collection element. + * - "[data-key] attribute is set on items. That is used to find the element for the focused item (which, of course, + * is not actually focused while the input is). + * + * Note: there has been some addition to react-aria useSelectableCollection and useSelectableItem hooks + * based on CustomEvent and a similar event reply mechanism in useAutocomplete. + * It may be possible to replace this hook with built-in functionality in react-aria at some point. + * But at the moment, it seems like that implementation is too coupled with the autocompletion use case, while + * what is supported here is more generic and allows for the connected search input use case too. + */ + +export const useCollectionFocusProxy = ({ + state, + focusProxyRef, + collectionRef, + onAction, +}: { + focusProxyRef: RefObject | undefined; + collectionRef: RefObject; + state: { + /** A collection of items in the list. */ + collection: Collection>; + /** A selection manager to read and update multiple selection state. */ + selectionManager: SelectionManager; + }; + onAction: ((key: Key) => void) | undefined; +}) => { + // TODO: focus/blur events should probably be handled as well, to keep the + // isFocused state of the collection in sync. + useEffect( + () => { + const proxy = focusProxyRef?.current; + if (proxy) { + const onKeyDown = (event: KeyboardEvent) => { + if (event.key === "ArrowUp" || event.key === "ArrowDown") { + event.preventDefault(); + event.stopPropagation(); + + collectionRef.current?.dispatchEvent( + new KeyboardEvent(event.type, event) + ); + } else if ( + event.key === "Enter" && + state.selectionManager?.focusedKey != null + ) { + event.preventDefault(); // in forms, pressing Enter on input submits the form + (event.currentTarget as HTMLElement)?.addEventListener( + "keyup", + (event: KeyboardEvent) => { + console.log( + "Keyup", + event.key, + state.selectionManager.focusedKey, + "onAction", + onAction + ); + if ( + event.key === "Enter" && + state.selectionManager.focusedKey != null + ) { + onAction?.(state.selectionManager.focusedKey); + } + }, + { once: true, capture: true } + ); + } + }; + proxy.addEventListener("keydown", onKeyDown); + return () => { + proxy.removeEventListener("keydown", onKeyDown); + }; + } + } /* with no dependency here, event listeners are reattached on each render, but that's the case when unmemoized + event handlers are passed to elements too (e.g., when using any react-aria hook) */ + ); +}; diff --git a/packages/jui/src/Collections/useCollectionSearchInput.ts b/packages/jui/src/Collections/useCollectionSearchInput.ts deleted file mode 100644 index 1f0e4597..00000000 --- a/packages/jui/src/Collections/useCollectionSearchInput.ts +++ /dev/null @@ -1,92 +0,0 @@ -import React, { Key, RefObject } from "react"; -import { SelectionManager } from "@react-stately/selection"; -import { useEventCallback } from "@intellij-platform/core/utils/useEventCallback"; -import { DOMAttributes } from "@react-types/shared"; - -/** - * A solution for connecting a collection to a search input, so that collection can still be navigated by keyboard - * while the input is focused. It works by replaying certain keyboard events on the collection container and focused - * item. An alternative approach (which is used in react-aria's useCombobox) is to use useSelectableCollection - * separately for the input, but the biggest issue with that approach is that it's limiting in the following ways: - * - Rendering input should be a part of the same component that renders the collection. Having specific components - * for use cases that requires a search input is not flexible enough. For example one may want to use SpeedSearchList - * or List connected to an input. Also, the input and the collection may need to be in different layouts in different - * use cases. Decoupling the rendering of the input and collection is a more flexible solution. - * - The same options used for collection should be passed to the input field for behavior consistency, and that can be - * prone to error. Some of these options, like `keyboardDelegate` can even have a default value in hooks like - * `useSelectableList`, which means for making sure the same value is passed to the useSelectableCollection for input, - * would require to not use the default value, since the same value can't be accessed. - * - * With this event forwarding approach, it's an arrow up or down event would behave exactly like it was triggered on - * the collection itself, leaving no room for behavior discrepancies. But it has a few drawbacks: - * - Although small, there is still some coupling between this code and implementation of the collection component. - * More specifically, the following things are assumed by this implementation: - * - "Enter" keys (selection or action) are handled on items, but arrow keys are handled on the collection element. - * - "[data-key] attribute is set on items. That is used to find the element for the focused item (which of course is - * not actually focused while the input is). - */ -export const useCollectionSearchInput = ({ - collectionRef, - selectionManagerRef, - onAction, -}: { - /** - * ref to the html element of the collection component - */ - collectionRef: RefObject; - /** - * SelectionManager instance, returned from the state management hook for the collection component. - * {@link CollectionRefProps.selectionManagerRef} can be used on collection components that implement - * `useCollectionRef`, to get a hold of selection manager, from outside. - */ - selectionManagerRef: RefObject | null | undefined; - /** - * onAction callback passed to the collection component. It's needed since some upgrade of @react-aria/interactions, - * since a check is added to not have keyup events on outside elements trigger onPress. That's to prevent scenarios - * where focus is moved between keydown and keyup, but is also breaking the previous solution of just replying - * input keyboard events on the list item. - * @param key - */ - onAction?: (key: Key) => void; -}): { collectionSearchInputProps: DOMAttributes } => { - const relayEventsToCollection = useEventCallback( - (event: React.KeyboardEvent) => { - // Relay ArrowUp and ArrowDown to the container - if ( - event.type === "keydown" && - (event.key === "ArrowUp" || event.key === "ArrowDown") - ) { - event.preventDefault(); - event.stopPropagation(); - collectionRef.current?.dispatchEvent( - new KeyboardEvent(event.type, event.nativeEvent) - ); - } else if ( - event.type === "keydown" && - event.key === "Enter" && - selectionManagerRef?.current?.focusedKey != null - ) { - event.preventDefault(); // in forms, pressing Enter on input submits the form - event.currentTarget.addEventListener( - "keyup", - (event: KeyboardEvent) => { - if ( - event.key === "Enter" && - selectionManagerRef?.current?.focusedKey != null - ) { - onAction?.(selectionManagerRef?.current?.focusedKey); - } - }, - { once: true, capture: true } - ); - } - } - ); - - return { - collectionSearchInputProps: { - onKeyDown: relayEventsToCollection, - onKeyPress: relayEventsToCollection, - }, - }; -}; diff --git a/packages/jui/src/Dropdown/ListBox.tsx b/packages/jui/src/Dropdown/ListBox.tsx index d4a1caeb..62f5d1c3 100644 --- a/packages/jui/src/Dropdown/ListBox.tsx +++ b/packages/jui/src/Dropdown/ListBox.tsx @@ -2,14 +2,11 @@ import React, { ForwardedRef } from "react"; import { AriaListBoxOptions, AriaListBoxProps } from "@react-aria/listbox"; import { useListState } from "@react-stately/list"; import { StatelessListBox } from "@intellij-platform/core/Dropdown/StatelessListBox"; -import { - CollectionRefProps, - useCollectionRef, -} from "@intellij-platform/core/Collections/useCollectionRef"; +import { CollectionFocusProxyProps } from "@intellij-platform/core/Collections"; export interface ListBoxProps extends AriaListBoxProps, - CollectionRefProps, + CollectionFocusProxyProps, Pick< AriaListBoxOptions, "shouldFocusOnHover" | "shouldUseVirtualFocus" @@ -26,7 +23,6 @@ export const ListBox = React.forwardRef(function ListBox( selectionMode: "single", ...props, }); - useCollectionRef(props, state); return ; }); diff --git a/packages/jui/src/Dropdown/StatelessListBox.tsx b/packages/jui/src/Dropdown/StatelessListBox.tsx index 3b260b08..fd11ba83 100644 --- a/packages/jui/src/Dropdown/StatelessListBox.tsx +++ b/packages/jui/src/Dropdown/StatelessListBox.tsx @@ -9,12 +9,24 @@ import { useObjectRef } from "@react-aria/utils"; import { ListState } from "@react-stately/list"; import { Node } from "@react-types/shared"; -import { ItemStateContext } from "@intellij-platform/core/Collections"; +import { + CollectionFocusProxyProps, + ItemStateContext, + useCollectionFocusProxy, +} from "@intellij-platform/core/Collections"; import { StyledListItem } from "@intellij-platform/core/List/StyledListItem"; import { StyledList } from "@intellij-platform/core/List/StyledList"; import { StyledVerticalSeparator } from "@intellij-platform/core/StyledSeparator"; import { styled } from "@intellij-platform/core/styled"; +interface StatelessListBoxProps + extends AriaListBoxOptions, + CollectionFocusProxyProps { + state: ListState; + style?: CSSProperties; + className?: string; +} + export const StatelessListBox = React.forwardRef(function StatelessListBox< T extends object >( @@ -22,17 +34,21 @@ export const StatelessListBox = React.forwardRef(function StatelessListBox< state, style, className, + focusProxyRef, ...props - }: AriaListBoxOptions & { - state: ListState; - style?: CSSProperties; - className?: string; - }, + }: StatelessListBoxProps, forwardedRef: ForwardedRef ) { const ref = useObjectRef(forwardedRef); const { listBoxProps, labelProps } = useListBox(props, state, ref); + useCollectionFocusProxy({ + state, + collectionRef: ref, + focusProxyRef, + onAction: props.onAction, + }); + return ( <>
{props.label}
diff --git a/packages/jui/src/List/List.tsx b/packages/jui/src/List/List.tsx index 411de626..0b97bb1c 100644 --- a/packages/jui/src/List/List.tsx +++ b/packages/jui/src/List/List.tsx @@ -11,12 +11,14 @@ import { CollectionRefProps } from "@intellij-platform/core/Collections/useColle import { Virtualizer } from "@react-aria/virtualizer"; import { useListVirtualizer } from "@intellij-platform/core/List/useListVirtualizer"; import { ListContext } from "@intellij-platform/core/List/ListContext"; +import { CollectionFocusProxyProps } from "@intellij-platform/core/Collections"; export type ListProps = Omit< Omit, "disallowEmptySelection">, keyof AsyncLoadable > & - CollectionRefProps & { + CollectionRefProps & + CollectionFocusProxyProps & { /** * fills the available horizontal or vertical space, when rendered in a flex container. */ diff --git a/packages/jui/src/List/story-helpers.tsx b/packages/jui/src/List/story-helpers.tsx index f7d5217e..ae7cd087 100644 --- a/packages/jui/src/List/story-helpers.tsx +++ b/packages/jui/src/List/story-helpers.tsx @@ -1,7 +1,6 @@ import { Legend, legends } from "../../test-data"; -import React, { ReactNode } from "react"; +import React, { ReactNode, useRef } from "react"; import { Story } from "@storybook/react"; -import { SelectionManager } from "@react-stately/selection"; import { Divider, DividerItem, @@ -10,7 +9,6 @@ import { List, ListProps, Section, - useCollectionSearchInput, } from "@intellij-platform/core"; import { Pane } from "../story-components"; @@ -55,24 +53,20 @@ export const renderItemTextWithHighlights = (item: Legend) => ( export const commonListStories = { withConnectedInput: (ListCmp: typeof List) => { const WithConnectedInput: Story> = (props) => { + const inputRef = useRef(null); const [isFocused, setIsFocused] = React.useState(false); const listRef = React.useRef(null); - const selectionManagerRef = React.useRef(null); - const { collectionSearchInputProps } = useCollectionSearchInput({ - collectionRef: listRef, - onAction: props.onAction, - selectionManagerRef, - }); + return ( setIsFocused(true)} onBlur={() => setIsFocused(false)} /> , + CollectionFocusProxyProps, CollectionRefProps { allowEmptySelection?: boolean; /** @@ -31,7 +36,7 @@ export interface ListProps // import { useSelectableList } from "@react-aria/selection"; export function useList( - { onAction, showAsFocused, ...props }: ListProps, + { onAction, showAsFocused, focusProxyRef, ...props }: ListProps, state: ListState, ref: React.RefObject ) { @@ -47,6 +52,14 @@ export function useList( // if selectOnFocus is going to be an option (which is not in intellij UI), we should also conditionally show outline on items selectOnFocus: true, }); + + useCollectionFocusProxy({ + focusProxyRef, + onAction, + state, + collectionRef: ref, + }); + const [focused, setFocused] = useState(false); const { focusWithinProps } = useFocusWithin({ diff --git a/packages/jui/src/Tree/SpeedSearchTree/SpeedSearchTree.tsx b/packages/jui/src/Tree/SpeedSearchTree/SpeedSearchTree.tsx index 25fd0ae4..65082ce1 100644 --- a/packages/jui/src/Tree/SpeedSearchTree/SpeedSearchTree.tsx +++ b/packages/jui/src/Tree/SpeedSearchTree/SpeedSearchTree.tsx @@ -6,7 +6,6 @@ import { SpeedSearchProps, SpeedSearchStateProps, } from "@intellij-platform/core/SpeedSearch"; -import { useCollectionRef } from "@intellij-platform/core/Collections/useCollectionRef"; import useForwardedRef from "@intellij-platform/core/utils/useForwardedRef"; import { StyledTree } from "../StyledTree"; import { SpeedSearchPopup } from "../../SpeedSearch/SpeedSearchPopup"; @@ -44,7 +43,6 @@ export const SpeedSearchTree = React.forwardRef( { ...props, disallowEmptySelection: !props.allowEmptySelection }, treeRef ); - useCollectionRef(props, state); const ref = useForwardedRef(forwardedRef); const { treeProps, diff --git a/packages/jui/src/Tree/Tree.tsx b/packages/jui/src/Tree/Tree.tsx index d44ef2a2..e28fa679 100644 --- a/packages/jui/src/Tree/Tree.tsx +++ b/packages/jui/src/Tree/Tree.tsx @@ -5,20 +5,15 @@ import { StyledTree } from "./StyledTree"; import { TreeRefValue } from "./useTreeRef"; import { TreeNode } from "./TreeNode"; import { TreeContext } from "./TreeContext"; -import { useTreeState, TreeProps as StatelyTreeProps } from "./useTreeState"; +import { TreeProps as StatelyTreeProps, useTreeState } from "./useTreeState"; import { SelectableTreeProps, useSelectableTree } from "./useSelectableTree"; import { useTreeVirtualizer } from "./useTreeVirtualizer"; import { CollectionCacheInvalidationProps } from "@intellij-platform/core/Collections/useCollectionCacheInvalidation"; -import { - CollectionRefProps, - useCollectionRef, -} from "@intellij-platform/core/Collections/useCollectionRef"; import { filterDOMProps, useObjectRef } from "@react-aria/utils"; export interface TreeProps extends Omit, "disallowEmptySelection">, CollectionCacheInvalidationProps, - CollectionRefProps, Omit, "keyboardDelegate" | "isVirtualized"> { fillAvailableSpace?: boolean; /** @@ -52,7 +47,6 @@ export const Tree = React.forwardRef( forwardedRef: ForwardedRef ) => { const state = useTreeState(props, treeRef); - useCollectionRef(props, state); const ref = useObjectRef(forwardedRef); const { treeProps, treeContext } = useSelectableTree( diff --git a/packages/jui/src/Tree/useSelectableTree.tsx b/packages/jui/src/Tree/useSelectableTree.tsx index 7b08c292..3272ac9e 100644 --- a/packages/jui/src/Tree/useSelectableTree.tsx +++ b/packages/jui/src/Tree/useSelectableTree.tsx @@ -18,10 +18,15 @@ import { hasAnyModifier } from "@intellij-platform/core/utils/keyboard-utils"; import { FocusEvents } from "@react-types/shared/src/events"; import { FocusStrategy } from "@react-types/shared/src/selection"; import { groupBy } from "ramda"; +import { + CollectionFocusProxyProps, + useCollectionFocusProxy, +} from "@intellij-platform/core/Collections"; export interface SelectableTreeProps extends DOMProps, - Omit { + Omit, + CollectionFocusProxyProps { isVirtualized?: boolean; keyboardDelegate?: KeyboardDelegate; /** @@ -49,6 +54,7 @@ export function useSelectableTree( onBlur, autoFocus, showAsFocused, + focusProxyRef, ...props }: SelectableTreeProps, state: TreeState, @@ -84,6 +90,14 @@ export function useSelectableTree( [state.collection, state.disabledKeys, props.keyboardDelegate] ), }); + + useCollectionFocusProxy({ + collectionRef: ref, + state, + onAction, + focusProxyRef, + }); + const { focusWithinProps } = useFocusWithin({ onFocusWithinChange: setFocused, });