fix some regressions after upgrading dependencies

The issue with button not receiving PressResponder props was due to .mjs entry being loaded through @react-aria/selection and .main.js entry being loaded when imported from source, because imports are transpiled to require and `require` entry of the package.json exports being picked.
alirezamirian · Jul 31, 2024 · 7fcac2f · 7fcac2f
1 parent 0418756
commit 7fcac2f
Show file tree

Hide file tree

Showing 14 changed files with 664 additions and 536 deletions.
diff --git a/packages/example-app/src/SearchEverywhere/SearchEverywherePopup.tsx b/packages/example-app/src/SearchEverywhere/SearchEverywherePopup.tsx
@@ -278,13 +278,52 @@ export function SearchEverywherePopup() {
 
   const collectionRef = useRef<HTMLDivElement>(null);
   const selectionManagerRef = useRef<SelectionManager>(null);
+  const onAction = (key: React.Key) => {
+    if (key === LOAD_MORE_ITEM_KEY) {
+      setSearchResultLimit((limit) => limit + SEARCH_RESULT_LIMIT);
+      const nextItem = searchResult[visibleSearchResult.length + 1];
+      if (nextItem) {
+        // nextItem is expected to always have value
+
+        // Timeout needed to let the item get rendered first. Could be done in an effect instead,
+        // if we want to avoid setTimeout
+        setTimeout(() => {
+          setSelectedKeys(new Set([nextItem.key]));
+          selectionManagerRef.current?.setFocusedKey(nextItem.key);
+        });
+      }
+    } else {
+      close();
+      // Making sure the popup is fully closed before the new action is performed. One edge case that can
+      // make a difference is actions like FindAction that open the same popup. By performing an action
+      // async, we make sure the popup is closed and reopened, which is good, because otherwise, the user
+      // won't get any feedback when choosing such actions.
+      setTimeout(() => {
+        const itemWrapper = searchResult.find((item) => item.key === key);
+        itemWrapper?.contributor.processSelectedItem(itemWrapper.item);
+        /**
+         * The 50ms timeout is a workaround for an issue in FocusScope:
+         * restoreFocus only works if the previously focused element is in the dom, when the focus
+         * scope is unmounted. In case of SearchEveryWhere, actions like "Rollback" open a modal
+         * window, which has a focus scope, when the window is opened, the currently focused
+         * element (which will be the one to restore focus to), is search everywhere dialog, which
+         * is immediately closed. So when the modal window is closed, it tries to move focus back
+         * to search everywhere dialog, which is long gone! It would be nice if FocusScope could
+         * track a chain of nodes to restore focus to.
+         * With this 50ms timeout, focus is first restored to where it was, after SearchEveryWhere
+         * is closed, and then the actions is performed, for focus restoration to work.
+         */
+      }, 50);
+    }
+  };
+
   const { collectionSearchInputProps } = useCollectionSearchInput({
     collectionRef,
+    onAction,
     selectionManager: selectionManagerRef.current,
   });
 
   const tips = useTips();
-
   return (
     <ContentAwarePopup
       persistedBoundsState={searchEverywhereState.bounds}
@@ -391,53 +430,7 @@ export function SearchEverywherePopup() {
                         fillAvailableSpace
                         selectedKeys={selectedKeys}
                         onSelectionChange={setSelectedKeys}
-                        onAction={(key) => {
-                          if (key === LOAD_MORE_ITEM_KEY) {
-                            setSearchResultLimit(
-                              (limit) => limit + SEARCH_RESULT_LIMIT
-                            );
-                            const nextItem =
-                              searchResult[visibleSearchResult.length + 1];
-                            if (nextItem) {
-                              // nextItem is expected to always have value
-
-                              // Timeout needed to let the item get rendered first. Could be done in an effect instead,
-                              // if we want to avoid setTimeout
-                              setTimeout(() => {
-                                setSelectedKeys(new Set([nextItem.key]));
-                                selectionManagerRef.current?.setFocusedKey(
-                                  nextItem.key
-                                );
-                              });
-                            }
-                          } else {
-                            close();
-                            // Making sure the popup is fully closed before the new action is performed. One edge case that can
-                            // make a difference is actions like FindAction that open the same popup. By performing an action
-                            // async, we make sure the popup is closed and reopened, which is good, because otherwise, the user
-                            // won't get any feedback when choosing such actions.
-                            setTimeout(() => {
-                              const itemWrapper = searchResult.find(
-                                (item) => item.key === key
-                              );
-                              itemWrapper?.contributor.processSelectedItem(
-                                itemWrapper.item
-                              );
-                              /**
-                               * The 50ms timeout is a workaround for an issue in FocusScope:
-                               * restoreFocus only works if the previously focused element is in the dom, when the focus
-                               * scope is unmounted. In case of SearchEveryWhere, actions like "Rollback" open a modal
-                               * window, which has a focus scope, when the window is opened, the currently focused
-                               * element (which will be the one to restore focus to), is search everywhere dialog, which
-                               * is immediately closed. So when the modal window is closed, it tries to move focus back
-                               * to search everywhere dialog, which is long gone! It would be nice if FocusScope could
-                               * track a chain of nodes to restore focus to.
-                               * With this 50ms timeout, focus is first restored to where it was, after SearchEveryWhere
-                               * is closed, and then the actions is performed, for focus restoration to work.
-                               */
-                            }, 50);
-                          }
-                        }}
+                        onAction={onAction}
                       >
                         {({ key, item, contributor }) => {
                           if (key === LOAD_MORE_ITEM_KEY) {

diff --git a/...ges/example-app/src/VersionControl/VersionControlToolWindow/BranchesView/BranchesTree.tsx b/...ges/example-app/src/VersionControl/VersionControlToolWindow/BranchesView/BranchesTree.tsx
@@ -58,9 +58,17 @@ export function BranchesTree({ tabKey }: { tabKey: string }) {
   const selectionManagerRef = useRef<TreeSelectionManager>(null);
   const [isInputFocused, setInputFocused] = useState(false);
   const setBranchFilter = useSetRecoilState(vcsLogFilterCurrentTab.branch);
+  const onAction = (key: React.Key) => {
+    const branch = keyToBranch(`${key}`);
+    if (branch) {
+      setBranchFilter([branch]);
+    }
+  };
+
   const { collectionSearchInputProps } = useCollectionSearchInput({
     collectionRef: ref,
     selectionManager: selectionManagerRef.current,
+    onAction,
   });
   /**
    * TODO: remaining from search:
@@ -114,12 +122,7 @@ export function BranchesTree({ tabKey }: { tabKey: string }) {
         onSelectionChange={setSelectedKeys}
         expandedKeys={expandedKeys}
         onExpandedChange={setExpandedKeys}
-        onAction={(key) => {
-          const branch = keyToBranch(`${key}`);
-          if (branch) {
-            setBranchFilter([branch]);
-          }
-        }}
+        onAction={onAction}
         fillAvailableSpace
         // speed search related props
         showAsFocused={isInputFocused}

diff --git a/packages/jui/package.json b/packages/jui/package.json
@@ -41,43 +41,43 @@
     "api-extractor": "../../node_modules/.bin/api-extractor"
   },
   "dependencies": {
-    "@react-aria/button": "^3.4.1",
-    "@react-aria/checkbox": "^3.3.1",
-    "@react-aria/dialog": "^3.2.1",
-    "@react-aria/focus": "^3.5.2",
-    "@react-aria/i18n": "^3.3.7",
-    "@react-aria/interactions": "^3.8.1",
-    "@react-aria/label": "^3.5.2",
-    "@react-aria/link": "^3.2.2",
-    "@react-aria/listbox": "^3.4.2",
-    "@react-aria/menu": "^3.7.1",
-    "@react-aria/overlays": "^3.12.1",
-    "@react-aria/progress": "^3.1.6",
-    "@react-aria/select": "^3.14.5",
-    "@react-aria/selection": "^3.7.3",
-    "@react-aria/separator": "^3.1.5",
-    "@react-aria/tabs": "^3.1.2",
-    "@react-aria/tooltip": "^3.3.0",
-    "@react-aria/utils": "^3.11.2",
-    "@react-aria/virtualizer": "^3.3.7",
-    "@react-aria/visually-hidden": "^3.2.5",
-    "@react-stately/collections": "^3.3.6",
+    "@react-aria/button": "~3.9.7",
+    "@react-aria/checkbox": "~3.3.4",
+    "@react-aria/dialog": "~3.2.1",
+    "@react-aria/focus": "~3.18.1",
+    "@react-aria/i18n": "^3.12.1",
+    "@react-aria/interactions": "~3.22.1",
+    "@react-aria/label": "~3.7.10",
+    "@react-aria/link": "~3.2.5",
+    "@react-aria/listbox": "~3.12.1",
+    "@react-aria/menu": "~3.14.1",
+    "@react-aria/overlays": "~3.22.1",
+    "@react-aria/progress": "~3.1.8",
+    "@react-aria/select": "~3.14.7",
+    "@react-aria/selection": "~3.18.1",
+    "@react-aria/separator": "~3.1.7",
+    "@react-aria/tabs": "~3.1.5",
+    "@react-aria/tooltip": "~3.3.4",
+    "@react-aria/utils": "~3.25.1",
+    "@react-aria/virtualizer": "3.3.7",
+    "@react-aria/visually-hidden": "~3.8.14",
+    "@react-stately/collections": "~3.10.9",
     "@react-stately/layout": "3.4.4",
-    "@react-stately/list": "^3.4.3",
-    "@react-stately/menu": "^3.2.5",
-    "@react-stately/overlays": "^3.4.4",
-    "@react-stately/select": "^3.6.4",
-    "@react-stately/selection": "^3.9.2",
-    "@react-stately/tabs": "^3.0.3",
-    "@react-stately/toggle": "^3.2.5",
-    "@react-stately/tooltip": "^3.2.0",
-    "@react-stately/tree": "^3.2.2",
-    "@react-stately/utils": "^3.4.1",
-    "@react-stately/virtualizer": "^3.1.7",
-    "@react-types/menu": "^3.5.1",
-    "@react-types/shared": "^3.11.1",
-    "@swc/helpers": "^0.3.17",
-    "ramda": "^0.27.1"
+    "@react-stately/list": "~3.10.7",
+    "@react-stately/menu": "~3.7.1",
+    "@react-stately/overlays": "~3.6.9",
+    "@react-stately/select": "~3.6.6",
+    "@react-stately/selection": "~3.15.1",
+    "@react-stately/tabs": "~3.6.8",
+    "@react-stately/toggle": "~3.7.6",
+    "@react-stately/tooltip": "~3.2.4",
+    "@react-stately/tree": "~3.8.3",
+    "@react-stately/utils": "~3.10.2",
+    "@react-stately/virtualizer": "~3.1.9",
+    "@react-types/menu": "~3.9.11",
+    "@react-types/shared": "~3.24.1",
+    "@swc/helpers": "~0.3.17",
+    "ramda": "~0.27.2"
   },
   "devDependencies": {
     "@babel/core": "^7.13.15",
@@ -86,10 +86,10 @@
     "@oreillymedia/cypress-playback": "^3.0.8",
     "@percy/cli": "^1.27.1",
     "@percy/cypress": "^3.1.2",
-    "@react-stately/data": "^3.4.2",
-    "@react-types/button": "^3.4.1",
-    "@react-types/link": "^3.2.2",
-    "@react-types/listbox": "3.1.1",
+    "@react-stately/data": "^3.11.6",
+    "@react-types/button": "^3.9.6",
+    "@react-types/link": "^3.5.7",
+    "@react-types/listbox": "3.5.1",
     "@storybook/addon-actions": "^7.3.2",
     "@storybook/addon-essentials": "^7.3.2",
     "@storybook/addon-links": "^7.3.2",

diff --git a/packages/jui/src/Collections/useCollectionSearchInput.ts b/packages/jui/src/Collections/useCollectionSearchInput.ts
@@ -1,4 +1,4 @@
-import React, { RefObject } from "react";
+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";
@@ -28,6 +28,7 @@ import { DOMAttributes } from "@react-types/shared";
 export const useCollectionSearchInput = ({
   collectionRef,
   selectionManager,
+  onAction,
 }: {
   /**
    * ref to the html element of the collection component
@@ -39,9 +40,17 @@ export const useCollectionSearchInput = ({
    * `useCollectionRef`, to get a hold of selection manager, from outside.
    */
   selectionManager: SelectionManager | 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<HTMLInputElement> } => {
   const relayEventsToCollection = useEventCallback(
-    (event: React.KeyboardEvent) => {
+    (event: React.KeyboardEvent<HTMLInputElement>) => {
       // Relay ArrowUp and ArrowDown to the container
       if (
         event.type === "keydown" &&
@@ -52,21 +61,27 @@ export const useCollectionSearchInput = ({
         collectionRef.current?.dispatchEvent(
           new KeyboardEvent(event.type, event.nativeEvent)
         );
-      }
-      // Relay Enter to the focused item
-      else if (event.key === "Enter" && selectionManager?.focusedKey) {
-        collectionRef.current
-          ?.querySelector(`[data-key="${selectionManager?.focusedKey}"]`)
-          ?.dispatchEvent(new KeyboardEvent(event.type, event.nativeEvent));
-        event.preventDefault();
+      } else if (
+        event.type === "keydown" &&
+        event.key === "Enter" &&
+        selectionManager?.focusedKey != null
+      ) {
+        event.currentTarget.addEventListener(
+          "keyup",
+          (event: KeyboardEvent) => {
+            if (event.key === "Enter" && selectionManager?.focusedKey != null) {
+              onAction?.(selectionManager?.focusedKey);
+            }
+          },
+          { once: true, capture: true }
+        );
       }
     }
   );
 
   return {
     collectionSearchInputProps: {
       onKeyDown: relayEventsToCollection,
-      onKeyUp: relayEventsToCollection,
       onKeyPress: relayEventsToCollection,
     },
   };

diff --git a/packages/jui/src/List/ListItem.tsx b/packages/jui/src/List/ListItem.tsx
@@ -23,11 +23,6 @@ export function ListItem<T>({ item, children }: ListItemProps<T>) {
     onAction: () => onAction?.(item.key),
     selectionManager: state.selectionManager,
   });
-  let { pressProps } = usePress({
-    ...itemProps,
-    isDisabled,
-    preventFocusOnPress: false,
-  });
 
   return (
     <StyledListItem
@@ -37,7 +32,7 @@ export function ListItem<T>({ item, children }: ListItemProps<T>) {
       aria-disabled={isDisabled}
       aria-selected={isSelected}
       aria-label={item["aria-label"]}
-      {...pressProps}
+      {...itemProps}
       ref={ref}
     >
       <ItemStateContext.Provider

diff --git a/packages/jui/src/List/story-helpers.tsx b/packages/jui/src/List/story-helpers.tsx
@@ -60,6 +60,7 @@ export const commonListStories = {
       const selectionManagerRef = React.useRef<SelectionManager>(null);
       const { collectionSearchInputProps } = useCollectionSearchInput({
         collectionRef: listRef,
+        onAction: props.onAction,
         selectionManager: selectionManagerRef.current,
       });
       return (

diff --git a/packages/jui/src/Menu/SpeedSearchMenu.cy.tsx b/packages/jui/src/Menu/SpeedSearchMenu.cy.tsx
@@ -29,7 +29,7 @@ describe("SpeedSearchMenu", () => {
     matchImageSnapshot("SpeedSearchMenu top level items filtered");
   });
 
-  it("lets user filter menu items in submenus", () => {
+  it.only("lets user filter menu items in submenus", () => {
     cy.mount(<Default />);
     cy.findByRole("menuitem", { name: "View Mode" }).click();
     cy.findByRole("menuitem", { name: "Float" }).should("exist");

diff --git a/packages/jui/src/Menu/SpeedSearchMenu.tsx b/packages/jui/src/Menu/SpeedSearchMenu.tsx
@@ -97,7 +97,7 @@ function useSpeedSearchMenu<T>(
       () =>
         new ListCollection(
           rootKey
-            ? state.collection.getChildren?.(rootKey) ?? []
+            ? state.collection.getItem(rootKey)?.childNodes ?? []
             : state.collection
         ),
       [rootKey, state.collection]

diff --git a/packages/jui/src/Overlay/useResizableMovableOverlay.tsx b/packages/jui/src/Overlay/useResizableMovableOverlay.tsx
@@ -1,4 +1,4 @@
-import React, { RefObject, useState } from "react";
+import { RefObject, useState } from "react";
 import { useControlledState } from "@react-stately/utils";
 import {
   Bounds,
@@ -127,7 +127,7 @@ export function useResizableMovableOverlay(
   UNSAFE_measureContentSize: () => void;
 } {
   const [bounds, setBounds] = useControlledState<Partial<Bounds> | undefined>(
-    inputBounds!,
+    inputBounds,
     defaultBounds!,
     // onBoundsChange is called with Bounds object. Not Partial<Bounds>, and not undefined.
     onBoundsChange as (
@@ -180,7 +180,8 @@ export function useResizableMovableOverlay(
     },
     finishInteraction: () => {
       if (currentInteraction && overlayRef.current) {
-        setBounds(getBounds(overlayRef.current));
+        // @ts-expect-error https://github.com/adobe/react-spectrum/issues/6784
+        setBounds(getBounds(overlayRef.current), currentInteraction.type);
       }
       setCurrentInteraction(null);
     },

diff --git a/packages/jui/src/Popup/Popup.tsx b/packages/jui/src/Popup/Popup.tsx
@@ -119,7 +119,6 @@ export const _Popup = (
     ref,
     onInteractOutsideStart: (e) => {
       if (
-        !ref.current?.contains(document.activeElement) && // If we are focused, shouldCloseOnBlur will call onClose.
         !nonDismissable &&
         shouldCloseOnInteractOutside(e.target as Element)
       ) {