Skip to content

Commit

Permalink
refactor(collections): simplify the API for connecting an input to a …
Browse files Browse the repository at this point in the history
…collection component

useCollectionSearchInput had this advantage of allowing for connecting an input to ANY collection without requiring support from the collection component. However the collection component needed to support selectionManagerRef, so there was still a need to make the collection component compatible with useCollectionSearchInput. Plus, the usage API isn't simple enough, as it requires:
- creating a ref to hold selection manager. Passing that ref to selectionManagerRef, and importing the right type for the ref value.
- importing and invoking useCollectionSearchInput, and applying the returned props on the connected input.

That's a lot of not-so-intuitive work.
Considering any supported collection component still needs to support `CollectionRefProps` and utilize `useCollectionRef` to support connected input, the implementation is changed to have the event listeners baked into the collections (via a new useCollectionFocusProxy hook). This way at least the usage API is as simple as passing a ref to the connected input to `focusProxyRef` prop of the collection component.
  • Loading branch information
alirezamirian committed Dec 8, 2024
1 parent f1342d2 commit 7fc3a51
Show file tree
Hide file tree
Showing 14 changed files with 189 additions and 171 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ import {
Tabs,
Tooltip,
TooltipTrigger,
useCollectionSearchInput,
useGetActionShortcut,
} from "@intellij-platform/core";
import { useTips } from "./useTips";
Expand Down Expand Up @@ -276,7 +275,7 @@ export function SearchEverywherePopup() {

const close = () => setOpen(false);

const collectionRef = useRef<HTMLDivElement>(null);
const searchInputRef = useRef<HTMLInputElement>(null);
const selectionManagerRef = useRef<SelectionManager>(null);
const onAction = (key: React.Key) => {
if (key === LOAD_MORE_ITEM_KEY) {
Expand Down Expand Up @@ -317,12 +316,6 @@ export function SearchEverywherePopup() {
}
};

const { collectionSearchInputProps } = useCollectionSearchInput({
collectionRef,
onAction,
selectionManagerRef,
});

const tips = useTips();
return (
<ContentAwarePopup
Expand Down Expand Up @@ -398,7 +391,7 @@ export function SearchEverywherePopup() {
<PlatformIcon icon="actions/search" />
<FocusScope contain>
<Input
{...collectionSearchInputProps}
ref={searchInputRef}
autoFocus
onFocus={(event) => {
event.target.select();
Expand All @@ -421,7 +414,7 @@ export function SearchEverywherePopup() {
{pattern && (
<StyledSearchResultsContainer>
<List
ref={collectionRef}
focusProxyRef={searchInputRef}
selectionManagerRef={selectionManagerRef}
items={visibleSearchResult}
selectionMode="single"
Expand Down
Original file line number Diff line number Diff line change
@@ -1,11 +1,6 @@
import React, { useRef, useState } from "react";
import { useRecoilState, useRecoilValue, useSetRecoilState } from "recoil";
import {
PlatformIcon,
SpeedSearchTree,
styled,
useCollectionSearchInput,
} from "@intellij-platform/core";
import { PlatformIcon, SpeedSearchTree, styled } from "@intellij-platform/core";

import {
branchTreeNodeRenderers,
Expand All @@ -20,8 +15,6 @@ import {
} from "./BranchesTree.state";
import { StyledHeader } from "../styled-components";
import { vcsLogFilterCurrentTab } from "../vcs-logs.state";
import { TreeSelectionManager } from "@intellij-platform/core/Tree/TreeSelectionManager";
import { mergeProps } from "@react-aria/utils";
import { useLatestRecoilValue } from "../../../recoil-utils";

const StyledSearchInput = styled.input`
Expand Down Expand Up @@ -55,7 +48,6 @@ export function BranchesTree({ tabKey }: { tabKey: string }) {
const treeRef = useRecoilValue(branchesTreeRefState(tabKey));
const ref = useRef<HTMLDivElement>(null);
const searchInputRef = useRef<HTMLInputElement>(null);
const selectionManagerRef = useRef<TreeSelectionManager>(null);
const [isInputFocused, setInputFocused] = useState(false);
const setBranchFilter = useSetRecoilState(vcsLogFilterCurrentTab.branch);
const onAction = (key: React.Key) => {
Expand All @@ -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
Expand Down Expand Up @@ -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);
}}
/>
</StyledHeader>
<SpeedSearchTree
ref={ref}
treeRef={treeRef}
selectionManagerRef={selectionManagerRef}
focusProxyRef={searchInputRef}
items={branchesTreeNodes ?? []}
selectionMode="multiple"
selectedKeys={selectedKeys}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,11 +15,9 @@ import {
PlatformIcon,
styled,
TooltipTrigger,
useCollectionSearchInput,
WINDOW_SHADOW,
} from "@intellij-platform/core";
import { MENU_VERTICAL_PADDING } from "@intellij-platform/core/Menu/StyledMenu";
import { TreeSelectionManager } from "@intellij-platform/core/Tree/TreeSelectionManager";
import { usePrevious } from "@intellij-platform/core/utils/usePrevious";
import { notImplemented } from "../../Project/notImplemented";
import { DIR_ICON } from "../../file-utils";
Expand Down Expand Up @@ -76,15 +74,9 @@ export function PathInputField({
const inputRef = useRef<HTMLInputElement>(null);

const collectionRef = useRef<HTMLDivElement>(null);
const selectionManagerRef = useRef<TreeSelectionManager>(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("");
Expand Down Expand Up @@ -175,7 +167,6 @@ export function PathInputField({
inputProps={mergeProps(
props.inputProps ?? {},
shortcutHandlerProps,
collectionSearchInputProps,
{
onKeyDown: (e: React.KeyboardEvent) => {
if (e.key === "Escape") {
Expand Down Expand Up @@ -218,7 +209,7 @@ export function PathInputField({
<StyledPopover>
<ListBoxStyledAsMenu
ref={collectionRef}
selectionManagerRef={selectionManagerRef}
focusProxyRef={inputRef}
aria-label="Directories"
shouldFocusOnHover
shouldUseVirtualFocus
Expand Down
2 changes: 1 addition & 1 deletion packages/jui/src/Collections/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ export { Item } from "./Item";
export * from "./Divider";
export * from "./ItemStateContext";
export * from "./ItemLayout";
export * from "./useCollectionSearchInput";
export * from "./useCollectionFocusProxy";

// NOTE: some stuff like `useCollectionCacheInvalidation` are not exported from the index, since the index is re-exported
// from other modules like Menu, Tabs, etc., but that part of the API is not considered a public API at the moment.
Expand Down
114 changes: 114 additions & 0 deletions packages/jui/src/Collections/useCollectionFocusProxy.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,114 @@
import { Key, RefObject, useEffect } from "react";
import { SelectionManager } from "@react-stately/selection";
import { Collection, Node } from "@react-types/shared";

/**
* interface to be extended by the props of collection components that support focus proxy.
*/
export interface CollectionFocusProxyProps {
/**
* ref to an element (typically HTMLInputElement) that should act as a focus
* proxy that handles ArrowUp, ArrowDown, and Enter keys to allow for
* navigating the collection and selecting items.
* Useful for implementing
* autocompletion or search input connected to a collection element.
*/
focusProxyRef?: RefObject<HTMLElement>;
}

/**
* 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 = <T>({
state,
focusProxyRef,
collectionRef,
onAction,
}: {
focusProxyRef: RefObject<HTMLElement> | undefined;
collectionRef: RefObject<HTMLElement>;
state: {
/** A collection of items in the list. */
collection: Collection<Node<T>>;
/** 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) */
);
};
92 changes: 0 additions & 92 deletions packages/jui/src/Collections/useCollectionSearchInput.ts

This file was deleted.

Loading

0 comments on commit 7fc3a51

Please sign in to comment.