feat(ModalWindow): improve imperative API by returning promise from o…

…pen method example-app is also refactored to fix the regression in delete file(s) action. The issue started to happen after upgrading to the latest version of @react-aria/focus, but it's because the deletion happens in the confirmation dialog, and before its closed. So the node to restore the focus to is removed when the focus restoration logic runs. Note: upgrade of @react-aria/overlays didn't have anything to do with the fix, but was kept anyway.
alirezamirian · Aug 4, 2024 · d3dd6fd · d3dd6fd
1 parent 351c426
commit d3dd6fd
Show file tree

Hide file tree

Showing 5 changed files with 96 additions and 65 deletions.
diff --git a/packages/example-app/src/ProjectView/actions/deleteAction.tsx b/packages/example-app/src/ProjectView/actions/deleteAction.tsx
@@ -1,5 +1,5 @@
 import path from "path";
-import { selector, useRecoilCallback } from "recoil";
+import { selector } from "recoil";
 import React from "react";
 
 import {
@@ -22,7 +22,6 @@ import { selectedNodesState } from "../ProjectView.state";
 import { findRootPaths } from "../../path-utils";
 import {
   deleteDirCallback,
-  deleteFileCallback,
   deleteFilesCallback,
 } from "../../Project/fs-operations";
 import { IntlMessageFormat } from "intl-messageformat";
@@ -52,7 +51,7 @@ export const deleteActionState = selector({
     isDisabled: !get(activePathExistsState),
     actionPerformed: getCallback(({ snapshot }) => async () => {
       const deleteDir = getCallback(deleteDirCallback);
-      const deleteFile = getCallback(deleteFileCallback);
+      const deleteFiles = getCallback(deleteFilesCallback);
       const selectedNodes = snapshot.getLoadable(selectedNodesState).getValue();
       const windowManager = snapshot
         .getLoadable(windowManagerRefState)
@@ -105,12 +104,21 @@ export const deleteActionState = selector({
         });
         if (confirmed) {
           directories.forEach((pathname) => deleteDir(pathname));
-          filePaths.forEach((pathname) => deleteFile(pathname));
+          deleteFiles(filePaths);
         }
       } else {
-        windowManager?.open(({ close }) => (
-          <DeleteFilesConfirmationDialog filePaths={filePaths} close={close} />
-        ));
+        windowManager
+          ?.open<boolean>(({ close }) => (
+            <DeleteFilesConfirmationDialog
+              filePaths={filePaths}
+              close={close}
+            />
+          ))
+          .then((confirmed) => {
+            if (confirmed) {
+              deleteFiles(filePaths);
+            }
+          });
       }
     }),
   }),
@@ -120,11 +128,9 @@ function DeleteFilesConfirmationDialog({
   close,
   filePaths,
 }: {
-  close: () => void;
+  close: (confirmed?: true) => void;
   filePaths: string[];
 }) {
-  const deleteFiles = useRecoilCallback(deleteFilesCallback, []);
-
   return (
     <ModalWindow minWidth="content" minHeight="content">
       <WindowLayout
@@ -165,15 +171,14 @@ function DeleteFilesConfirmationDialog({
             left={<HelpButton onPress={() => notImplemented()}></HelpButton>}
             right={
               <>
-                <Button onPress={close}>Cancel</Button>
+                <Button onPress={() => close()}>Cancel</Button>
                 <Button
                   autoFocus
                   type="submit"
                   form="delete_dialog_form" // Using form in absence of built-in support for default button
                   variant="default"
                   onPress={() => {
-                    deleteFiles(filePaths);
-                    close();
+                    close(true);
                   }}
                 >
                   Ok

diff --git a/packages/jui/cypress/e2e/file-actions.cy.ts b/packages/jui/cypress/e2e/file-actions.cy.ts
@@ -44,8 +44,7 @@ describe("files actions", () => {
     });
   });
 
-  // FIXME
-  it.skip("can create, delete and recreate a file without vcs", () => {
+  it("can create, delete and recreate a file without vcs", () => {
     cy.initialization();
 
     createFileWithoutVcs("test.ts");

diff --git a/packages/jui/package.json b/packages/jui/package.json
@@ -51,7 +51,7 @@
     "@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/overlays": "~3.23.1",
     "@react-aria/progress": "~3.1.8",
     "@react-aria/select": "~3.14.7",
     "@react-aria/selection": "~3.18.1",

diff --git a/packages/jui/src/ModalWindow/WindowManager.tsx b/packages/jui/src/ModalWindow/WindowManager.tsx
@@ -8,22 +8,57 @@ import React, {
 } from "react";
 import {
   ModalWindow,
-  WindowControllerContext,
   ModalWindowProps,
+  WindowControllerContext,
 } from "./ModalWindow";
 
 export interface WindowManagerAPI {
   /**
    * Shows a modal window within the stack of windows managed by {@link WindowManager}.
    * The opened windows will be closed when `onClose` interactions happen.
+   * Returns a Promise which is resolved when the modal window is closed.
+   *
+   * Modal window element, or a function that returns the modal window element can
+   * be passed.
+   * If a function is passed, it will receive a close function in its object argument,
+   * which can be used to resolve the returned promise with.
+   *
+   * @example
+   *
+   * ```
+   * const windowManager = useWindowManager();
+   *
+   * const showWindow = () => {
+   *   windowManager
+   *     .open<boolean>(({ close }) => (
+   *       <ModalWindow>
+   *         <WindowLayout
+   *           header="..."
+   *           content={<>...</>}
+   *           footer={
+   *             <WindowLayout.Footer
+   *               right={
+   *                 <>
+   *                   <Button onPress={close}>Cancel</Button>
+   *                   <Button onPress={() => close(true)}>Ok</Button>
+   *                 </>
+   *               }
+   *             />
+   *           }
+   *         />
+   *       </ModalWindow>
+   *     ))
+   *     .then((confirmed) => alert(`confirmed: ${confirmed ?? false}`));
+   * };
+   * ```
    */
-  open(
+  open<T>(
     props:
       | React.ReactElement<ModalWindowProps, typeof ModalWindow>
       | ((args: {
-          close: () => void;
+          close: (result?: T) => void;
         }) => React.ReactElement<ModalWindowProps, typeof ModalWindow>)
-  ): void;
+  ): Promise<T | undefined>;
 }
 
 const NotImplementedFn = () => {
@@ -59,25 +94,39 @@ export const WindowManager: React.FC<WindowManagerProps> = ({ children }) => {
   const newKeyRef = useRef<number>(0);
 
   const api = useMemo<WindowManagerAPI>(() => {
-    const openModalWindow: WindowManagerAPI["open"] = (content) => {
-      newKeyRef.current++;
-      const close = () => {
-        setWindows((currentWindows) =>
-          currentWindows.filter((aWindow) => aWindow !== window)
-        );
-      };
-      const window = (
-        <WindowControllerContext.Provider
-          value={{ onClose: close }}
-          key={newKeyRef.current}
-        >
-          {typeof content === "function" ? content({ close }) : content}
-        </WindowControllerContext.Provider>
-      );
-      setWindows((currentWindows) => currentWindows.concat(window));
-    };
     return {
-      open: openModalWindow,
+      open: function <T>(content: Parameters<WindowManagerAPI["open"]>[0]) {
+        return new Promise<T | undefined>((resolve) => {
+          newKeyRef.current++;
+          const close = (result?: T) => {
+            setWindows((currentWindows) =>
+              currentWindows.filter((aWindow) => aWindow !== window)
+            );
+            // Make sure (?) to resolve the promise after the dialog is closed,
+            // for the focus to be restored to the previous element before the
+            // potential further actions take place.
+            requestAnimationFrame(() => {
+              resolve(result);
+            });
+          };
+          const window = (
+            <WindowControllerContext.Provider
+              value={{
+                onClose: () => {
+                  close();
+                },
+              }}
+              key={newKeyRef.current}
+            >
+              {typeof content === "function"
+                ? // @ts-expect-error close signature is not correctly inferred
+                  content({ close })
+                : content}
+            </WindowControllerContext.Provider>
+          );
+          setWindows((currentWindows) => currentWindows.concat(window));
+        });
+      },
     };
   }, []);
 

diff --git a/yarn.lock b/yarn.lock
@@ -2909,7 +2909,7 @@ __metadata:
     "@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/overlays": ~3.23.1
     "@react-aria/progress": ~3.1.8
     "@react-aria/select": ~3.14.7
     "@react-aria/selection": ~3.18.1
@@ -5920,7 +5920,7 @@ __metadata:
   languageName: node
   linkType: hard
 
-"@react-aria/overlays@npm:^3.22.1, @react-aria/overlays@npm:^3.23.1":
+"@react-aria/overlays@npm:^3.22.1, @react-aria/overlays@npm:^3.23.1, @react-aria/overlays@npm:~3.23.1":
   version: 3.23.1
   resolution: "@react-aria/overlays@npm:3.23.1"
   dependencies:
@@ -5942,28 +5942,6 @@ __metadata:
   languageName: node
   linkType: hard
 
-"@react-aria/overlays@npm:~3.22.1":
-  version: 3.22.1
-  resolution: "@react-aria/overlays@npm:3.22.1"
-  dependencies:
-    "@react-aria/focus": ^3.17.1
-    "@react-aria/i18n": ^3.11.1
-    "@react-aria/interactions": ^3.21.3
-    "@react-aria/ssr": ^3.9.4
-    "@react-aria/utils": ^3.24.1
-    "@react-aria/visually-hidden": ^3.8.12
-    "@react-stately/overlays": ^3.6.7
-    "@react-types/button": ^3.9.4
-    "@react-types/overlays": ^3.8.7
-    "@react-types/shared": ^3.23.1
-    "@swc/helpers": ^0.5.0
-  peerDependencies:
-    react: ^16.8.0 || ^17.0.0-rc.1 || ^18.0.0
-    react-dom: ^16.8.0 || ^17.0.0-rc.1 || ^18.0.0
-  checksum: 14a81edee5d2d2af05d21f209c712e4838753aaec7ad7bea4d362a2c354c70cb7f5b2e70cf549ce644f5849af1be7721550cfa577e973d4241605f55ab916115
-  languageName: node
-  linkType: hard
-
 "@react-aria/progress@npm:~3.1.8":
   version: 3.1.8
   resolution: "@react-aria/progress@npm:3.1.8"
@@ -6054,7 +6032,7 @@ __metadata:
   languageName: node
   linkType: hard
 
-"@react-aria/ssr@npm:^3.9.4, @react-aria/ssr@npm:^3.9.5":
+"@react-aria/ssr@npm:^3.9.5":
   version: 3.9.5
   resolution: "@react-aria/ssr@npm:3.9.5"
   dependencies:
@@ -6150,7 +6128,7 @@ __metadata:
   languageName: node
   linkType: hard
 
-"@react-aria/visually-hidden@npm:^3.8.12, @react-aria/visually-hidden@npm:^3.8.14, @react-aria/visually-hidden@npm:~3.8.14":
+"@react-aria/visually-hidden@npm:^3.8.14, @react-aria/visually-hidden@npm:~3.8.14":
   version: 3.8.14
   resolution: "@react-aria/visually-hidden@npm:3.8.14"
   dependencies:
@@ -6502,7 +6480,7 @@ __metadata:
   languageName: node
   linkType: hard
 
-"@react-types/overlays@npm:^3.6.1, @react-types/overlays@npm:^3.8.7, @react-types/overlays@npm:^3.8.9":
+"@react-types/overlays@npm:^3.6.1, @react-types/overlays@npm:^3.8.9":
   version: 3.8.9
   resolution: "@react-types/overlays@npm:3.8.9"
   dependencies: