From fee3c1fd544fce9bf951870c30bd9d601d5b2126 Mon Sep 17 00:00:00 2001
From: Nicholas VanCise <40526638+thenick775@users.noreply.github.com>
Date: Mon, 18 Nov 2024 20:50:55 -0800
Subject: [PATCH] feat: standardize modal behavior (#215)

---
 .../components/modals/upload-cheats.spec.tsx  | 41 +++++++++++--------
 .../src/components/modals/upload-cheats.tsx   | 29 +++----------
 .../upload-public-external-roms.spec.tsx      |  2 -
 .../modals/upload-public-external-roms.tsx    | 19 ++-------
 .../components/modals/upload-saves.spec.tsx   | 41 +++++++++++--------
 gbajs3/src/components/modals/upload-saves.tsx | 29 +++----------
 gbajs3/src/components/shared/styled.tsx       |  4 --
 7 files changed, 64 insertions(+), 101 deletions(-)

diff --git a/gbajs3/src/components/modals/upload-cheats.spec.tsx b/gbajs3/src/components/modals/upload-cheats.spec.tsx
index 4019ac9f..63040509 100644
--- a/gbajs3/src/components/modals/upload-cheats.spec.tsx
+++ b/gbajs3/src/components/modals/upload-cheats.spec.tsx
@@ -11,15 +11,17 @@ import { productTourLocalStorageKey } from '../product-tour/consts.tsx';
 import type { GBAEmulator } from '../../emulator/mgba/mgba-emulator.tsx';
 
 describe('<UploadCheatsModal />', () => {
-  it('uploads file', async () => {
+  it('uploads file and closes modal', async () => {
     const uploadCheatsSpy: (file: File, cb?: () => void) => void = vi.fn(
       (_file, cb) => cb && cb()
     );
     const syncActionIfEnabledSpy = vi.fn();
+    const setIsModalOpenSpy = vi.fn();
 
-    const { useEmulatorContext: originalEmulator } = await vi.importActual<
-      typeof contextHooks
-    >('../../hooks/context.tsx');
+    const {
+      useEmulatorContext: originalEmulator,
+      useModalContext: originalModal
+    } = await vi.importActual<typeof contextHooks>('../../hooks/context.tsx');
     const { useAddCallbacks: originalCallbacks } = await vi.importActual<
       typeof addCallbackHooks
     >('../../hooks/emulator/use-add-callbacks.tsx');
@@ -36,6 +38,11 @@ describe('<UploadCheatsModal />', () => {
       syncActionIfEnabled: syncActionIfEnabledSpy
     }));
 
+    vi.spyOn(contextHooks, 'useModalContext').mockImplementation(() => ({
+      ...originalModal(),
+      setIsModalOpen: setIsModalOpenSpy
+    }));
+
     const testCheatFile = new File(['Some cheat file contents'], 'rom1.cheats');
 
     renderWithContext(<UploadCheatsModal />);
@@ -57,21 +64,20 @@ describe('<UploadCheatsModal />', () => {
       expect.anything()
     );
     expect(syncActionIfEnabledSpy).toHaveBeenCalledOnce();
-
-    expect(screen.getByText('Upload complete!')).toBeVisible();
-    expect(screen.queryByText('File to upload:')).not.toBeInTheDocument();
-    expect(screen.queryByText('rom1.cheats')).not.toBeInTheDocument();
+    expect(setIsModalOpenSpy).toHaveBeenCalledWith(false);
   });
 
-  it('uploads multiple files', async () => {
+  it('uploads multiple files and closes modal', async () => {
     const uploadCheatsSpy: (file: File, cb?: () => void) => void = vi.fn(
       (_file, cb) => cb && cb()
     );
     const syncActionIfEnabledSpy = vi.fn();
+    const setIsModalOpenSpy = vi.fn();
 
-    const { useEmulatorContext: originalEmulator } = await vi.importActual<
-      typeof contextHooks
-    >('../../hooks/context.tsx');
+    const {
+      useEmulatorContext: originalEmulator,
+      useModalContext: originalModal
+    } = await vi.importActual<typeof contextHooks>('../../hooks/context.tsx');
     const { useAddCallbacks: originalCallbacks } = await vi.importActual<
       typeof addCallbackHooks
     >('../../hooks/emulator/use-add-callbacks.tsx');
@@ -88,6 +94,11 @@ describe('<UploadCheatsModal />', () => {
       syncActionIfEnabled: syncActionIfEnabledSpy
     }));
 
+    vi.spyOn(contextHooks, 'useModalContext').mockImplementation(() => ({
+      ...originalModal(),
+      setIsModalOpen: setIsModalOpenSpy
+    }));
+
     const testCheatFiles = [
       new File(['Some cheat file contents 1'], 'rom1.cheats'),
       new File(['Some cheat file contents 2'], 'rom2.cheats')
@@ -117,11 +128,7 @@ describe('<UploadCheatsModal />', () => {
       expect.anything()
     );
     expect(syncActionIfEnabledSpy).toHaveBeenCalledOnce();
-
-    expect(screen.getByText('Upload complete!')).toBeVisible();
-    expect(screen.queryByText('Files to upload:')).not.toBeInTheDocument();
-    expect(screen.queryByText('rom1.cheats')).not.toBeInTheDocument();
-    expect(screen.queryByText('rom2.cheats')).not.toBeInTheDocument();
+    expect(setIsModalOpenSpy).toHaveBeenCalledWith(false);
   });
 
   it('renders form validation error', async () => {
diff --git a/gbajs3/src/components/modals/upload-cheats.tsx b/gbajs3/src/components/modals/upload-cheats.tsx
index 8cd9ff07..4550b442 100644
--- a/gbajs3/src/components/modals/upload-cheats.tsx
+++ b/gbajs3/src/components/modals/upload-cheats.tsx
@@ -11,9 +11,7 @@ import {
   EmbeddedProductTour,
   type TourSteps
 } from '../product-tour/embedded-product-tour.tsx';
-import { CircleCheckButton } from '../shared/circle-check-button.tsx';
 import { DragAndDropInput } from '../shared/drag-and-drop-input.tsx';
-import { CenteredTextContainer } from '../shared/styled.tsx';
 
 type InputProps = {
   cheatFiles: File[];
@@ -24,13 +22,7 @@ const validFileExtensions = ['.cheats'];
 export const UploadCheatsModal = () => {
   const { setIsModalOpen } = useModalContext();
   const { emulator } = useEmulatorContext();
-  const {
-    reset,
-    handleSubmit,
-    setValue,
-    formState: { isSubmitSuccessful },
-    control
-  } = useForm<InputProps>();
+  const { reset, handleSubmit, setValue, control } = useForm<InputProps>();
   const { syncActionIfEnabled } = useAddCallbacks();
   const cheatsFormId = useId();
 
@@ -52,8 +44,8 @@ export const UploadCheatsModal = () => {
       )
     );
 
-    reset();
-    syncActionIfEnabled();
+    await syncActionIfEnabled();
+    setIsModalOpen(false);
   };
 
   const tourSteps: TourSteps = [
@@ -101,7 +93,6 @@ export const UploadCheatsModal = () => {
                 validFileExtensions={validFileExtensions}
                 error={error?.message}
                 hideAcceptedFiles={!value?.length}
-                hideErrors={isSubmitSuccessful}
                 multiple
               >
                 <p>
@@ -112,20 +103,12 @@ export const UploadCheatsModal = () => {
               </DragAndDropInput>
             )}
           />
-          {isSubmitSuccessful && (
-            <CenteredTextContainer>
-              <p>Upload complete!</p>
-            </CenteredTextContainer>
-          )}
         </form>
       </ModalBody>
       <ModalFooter>
-        <CircleCheckButton
-          copy="Upload"
-          form={cheatsFormId}
-          showSuccess={isSubmitSuccessful}
-          type="submit"
-        />
+        <Button form={cheatsFormId} type="submit" variant="contained">
+          Upload
+        </Button>
         <Button variant="outlined" onClick={() => setIsModalOpen(false)}>
           Close
         </Button>
diff --git a/gbajs3/src/components/modals/upload-public-external-roms.spec.tsx b/gbajs3/src/components/modals/upload-public-external-roms.spec.tsx
index 8d18abd2..1132bdda 100644
--- a/gbajs3/src/components/modals/upload-public-external-roms.spec.tsx
+++ b/gbajs3/src/components/modals/upload-public-external-roms.spec.tsx
@@ -84,8 +84,6 @@ describe('<UploadPublicExternalRomsModal />', () => {
     expect(onLoadOrDismissSpy).toHaveBeenCalledOnce();
     expect(onLoadOrDismissSpy).toHaveBeenCalledWith('loaded');
     expect(setIsModalOpenSpy).toHaveBeenCalledWith(false);
-
-    expect(await screen.findByText('Upload complete!')).toBeVisible();
   });
 
   it('renders external rom error', async () => {
diff --git a/gbajs3/src/components/modals/upload-public-external-roms.tsx b/gbajs3/src/components/modals/upload-public-external-roms.tsx
index ee911745..9249897d 100644
--- a/gbajs3/src/components/modals/upload-public-external-roms.tsx
+++ b/gbajs3/src/components/modals/upload-public-external-roms.tsx
@@ -21,7 +21,6 @@ import {
   LoadingIndicator,
   PacmanIndicator
 } from '../shared/loading-indicator.tsx';
-import { CenteredTextContainer } from '../shared/styled.tsx';
 
 import type { PublicRomUploadStatus } from '../../hooks/use-show-load-public-roms.tsx';
 
@@ -60,7 +59,6 @@ export const UploadPublicExternalRomsModal = ({
   const theme = useTheme();
   const { setIsModalOpen } = useModalContext();
   const { emulator } = useEmulatorContext();
-  const [hasCompletedUpload, setHasCompletedUpload] = useState(false);
   const [currentRomURL, setCurrentRomURL] = useState<string | null>(null);
   const uploadRomButtonId = useId();
   const runGame = useRunGame();
@@ -87,7 +85,6 @@ export const UploadPublicExternalRomsModal = ({
       };
       emulator?.uploadRom(externalRomFile, runCallback);
       setCurrentRomURL(null);
-      setHasCompletedUpload(true);
     }
   }, [
     onLoadOrDismiss,
@@ -113,13 +110,9 @@ export const UploadPublicExternalRomsModal = ({
           indicator={<PacmanIndicator />}
           loadingCopy="Loading rom from url:"
         >
-          {!hasCompletedUpload && (
-            <>
-              <p>A public rom URL has been shared with you.</p>
-              <p>You can load it using the upload button!</p>
-              <p>Make sure you trust the provider before uploading:</p>
-            </>
-          )}
+          <p>A public rom URL has been shared with you.</p>
+          <p>You can load it using the upload button!</p>
+          <p>Make sure you trust the provider before uploading:</p>
           <URLDisplay url={url} />
           {!!externalRomLoadError && (
             <ErrorWithIcon
@@ -127,16 +120,12 @@ export const UploadPublicExternalRomsModal = ({
               text="Loading rom from URL has failed"
             />
           )}
-          {hasCompletedUpload && (
-            <CenteredTextContainer>
-              <p>Upload complete!</p>
-            </CenteredTextContainer>
-          )}
         </LoadingIndicator>
       </ModalBody>
       <ModalFooter>
         <Button
           id={uploadRomButtonId}
+          disabled={isExternalRomLoading}
           onClick={() => {
             setCurrentRomURL(url.href);
             executeLoadExternalRom({ url: url });
diff --git a/gbajs3/src/components/modals/upload-saves.spec.tsx b/gbajs3/src/components/modals/upload-saves.spec.tsx
index d90d6414..4160579a 100644
--- a/gbajs3/src/components/modals/upload-saves.spec.tsx
+++ b/gbajs3/src/components/modals/upload-saves.spec.tsx
@@ -11,14 +11,16 @@ import { productTourLocalStorageKey } from '../product-tour/consts.tsx';
 import type { GBAEmulator } from '../../emulator/mgba/mgba-emulator.tsx';
 
 describe('<UploadSavesModal />', () => {
-  it('uploads file', async () => {
+  it('uploads file and closes modal', async () => {
     const uploadSaveOrSaveStateSpy: (file: File, cb?: () => void) => void =
       vi.fn((_file, cb) => cb && cb());
     const syncActionIfEnabledSpy = vi.fn();
+    const setIsModalOpenSpy = vi.fn();
 
-    const { useEmulatorContext: originalEmulator } = await vi.importActual<
-      typeof contextHooks
-    >('../../hooks/context.tsx');
+    const {
+      useEmulatorContext: originalEmulator,
+      useModalContext: originalModal
+    } = await vi.importActual<typeof contextHooks>('../../hooks/context.tsx');
     const { useAddCallbacks: originalCallbacks } = await vi.importActual<
       typeof addCallbackHooks
     >('../../hooks/emulator/use-add-callbacks.tsx');
@@ -35,6 +37,11 @@ describe('<UploadSavesModal />', () => {
       syncActionIfEnabled: syncActionIfEnabledSpy
     }));
 
+    vi.spyOn(contextHooks, 'useModalContext').mockImplementation(() => ({
+      ...originalModal(),
+      setIsModalOpen: setIsModalOpenSpy
+    }));
+
     const testSaveFile = new File(['Some save file contents'], 'rom1.sav');
 
     renderWithContext(<UploadSavesModal />);
@@ -56,21 +63,20 @@ describe('<UploadSavesModal />', () => {
       expect.anything()
     );
     expect(syncActionIfEnabledSpy).toHaveBeenCalledOnce();
-
-    expect(screen.getByText('Upload complete!')).toBeVisible();
-    expect(screen.queryByText('File to upload:')).not.toBeInTheDocument();
-    expect(screen.queryByText('rom1.sav')).not.toBeInTheDocument();
+    expect(setIsModalOpenSpy).toHaveBeenCalledWith(false);
   });
 
-  it('uploads multiple files', async () => {
+  it('uploads multiple files and closes modal', async () => {
     const uploadSaveSpy: (file: File, cb?: () => void) => void = vi.fn(
       (_file, cb) => cb && cb()
     );
     const syncActionIfEnabledSpy = vi.fn();
+    const setIsModalOpenSpy = vi.fn();
 
-    const { useEmulatorContext: originalEmulator } = await vi.importActual<
-      typeof contextHooks
-    >('../../hooks/context.tsx');
+    const {
+      useEmulatorContext: originalEmulator,
+      useModalContext: originalModal
+    } = await vi.importActual<typeof contextHooks>('../../hooks/context.tsx');
     const { useAddCallbacks: originalCallbacks } = await vi.importActual<
       typeof addCallbackHooks
     >('../../hooks/emulator/use-add-callbacks.tsx');
@@ -87,6 +93,11 @@ describe('<UploadSavesModal />', () => {
       syncActionIfEnabled: syncActionIfEnabledSpy
     }));
 
+    vi.spyOn(contextHooks, 'useModalContext').mockImplementation(() => ({
+      ...originalModal(),
+      setIsModalOpen: setIsModalOpenSpy
+    }));
+
     const testSaveFiles = [
       new File(['Some save file contents 1'], 'rom1.sav'),
       new File(['Some save file contents 2'], 'rom2.sav')
@@ -116,11 +127,7 @@ describe('<UploadSavesModal />', () => {
       expect.anything()
     );
     expect(syncActionIfEnabledSpy).toHaveBeenCalledOnce();
-
-    expect(screen.getByText('Upload complete!')).toBeVisible();
-    expect(screen.queryByText('Files to upload:')).not.toBeInTheDocument();
-    expect(screen.queryByText('rom1.sav')).not.toBeInTheDocument();
-    expect(screen.queryByText('rom2.sav')).not.toBeInTheDocument();
+    expect(setIsModalOpenSpy).toHaveBeenCalledWith(false);
   });
 
   it('renders form validation error', async () => {
diff --git a/gbajs3/src/components/modals/upload-saves.tsx b/gbajs3/src/components/modals/upload-saves.tsx
index a0ca300e..b1d4e822 100644
--- a/gbajs3/src/components/modals/upload-saves.tsx
+++ b/gbajs3/src/components/modals/upload-saves.tsx
@@ -11,9 +11,7 @@ import {
   EmbeddedProductTour,
   type TourSteps
 } from '../product-tour/embedded-product-tour.tsx';
-import { CircleCheckButton } from '../shared/circle-check-button.tsx';
 import { DragAndDropInput } from '../shared/drag-and-drop-input.tsx';
-import { CenteredTextContainer } from '../shared/styled.tsx';
 
 type InputProps = {
   saveFiles: File[];
@@ -28,13 +26,7 @@ export const UploadSavesModal = () => {
   const { setIsModalOpen } = useModalContext();
   const { emulator } = useEmulatorContext();
   const { syncActionIfEnabled } = useAddCallbacks();
-  const {
-    reset,
-    handleSubmit,
-    setValue,
-    formState: { isSubmitSuccessful },
-    control
-  } = useForm<InputProps>();
+  const { reset, handleSubmit, setValue, control } = useForm<InputProps>();
   const uploadSavesFormId = useId();
 
   const onDrop = useCallback(
@@ -55,8 +47,8 @@ export const UploadSavesModal = () => {
       )
     );
 
-    reset();
-    syncActionIfEnabled();
+    await syncActionIfEnabled();
+    setIsModalOpen(false);
   };
 
   const tourSteps: TourSteps = [
@@ -105,7 +97,6 @@ export const UploadSavesModal = () => {
                 validFileExtensions={validFileExtensions}
                 error={error?.message}
                 hideAcceptedFiles={!value?.length}
-                hideErrors={isSubmitSuccessful}
                 multiple
               >
                 <p>
@@ -115,20 +106,12 @@ export const UploadSavesModal = () => {
               </DragAndDropInput>
             )}
           />
-          {isSubmitSuccessful && (
-            <CenteredTextContainer>
-              <p>Upload complete!</p>
-            </CenteredTextContainer>
-          )}
         </form>
       </ModalBody>
       <ModalFooter>
-        <CircleCheckButton
-          copy="Upload"
-          form={uploadSavesFormId}
-          showSuccess={isSubmitSuccessful}
-          type="submit"
-        />
+        <Button form={uploadSavesFormId} type="submit" variant="contained">
+          Upload
+        </Button>
         <Button variant="outlined" onClick={() => setIsModalOpen(false)}>
           Close
         </Button>
diff --git a/gbajs3/src/components/shared/styled.tsx b/gbajs3/src/components/shared/styled.tsx
index d23ef9a4..f4f2f6f8 100644
--- a/gbajs3/src/components/shared/styled.tsx
+++ b/gbajs3/src/components/shared/styled.tsx
@@ -1,10 +1,6 @@
 import { BiPlus } from 'react-icons/bi';
 import { styled } from 'styled-components';
 
-export const CenteredTextContainer = styled.div`
-  text-align: center;
-`;
-
 export const CenteredText = styled.p`
   text-align: center;
   margin: 0;