From dcf71ccc71f9d7efaa3e286db25ba592e1150ccb Mon Sep 17 00:00:00 2001
From: Jeffrey Phillips <jephilli@redhat.com>
Date: Thu, 10 Oct 2024 13:10:55 -0400
Subject: [PATCH] Require conditional rendering of modals

---
 frontend/.eslintrc                            |  4 +
 frontend/src/app/DevFeatureFlagsBanner.tsx    | 46 ++++-----
 .../EditableLabelsDescriptionListGroup.tsx    | 94 ++++++++++---------
 .../BYONImageModal/ManageBYONImageModal.tsx   |  9 +-
 .../src/pages/BYONImages/BYONImagesTable.tsx  | 21 +++--
 .../BYONImages/ImportBYONImageButton.tsx      | 19 ++--
 .../list/AcceleratorProfileEnableToggle.tsx   | 21 +++--
 .../list/DisableAcceleratorProfileModal.tsx   |  4 +-
 ...geAcceleratorProfileTolerationsSection.tsx | 11 ++-
 .../tolerations/ManageTolerationModal.tsx     |  4 +-
 .../manage/tolerations/TolerationsTable.tsx   | 31 +++---
 .../screens/admin/NotebookAdminControl.tsx    |  4 +-
 .../screens/server/NotebookServer.tsx         | 10 +-
 .../screens/server/SpawnerPage.tsx            | 35 +++----
 .../screens/server/StartServerModal.tsx       | 26 ++---
 .../screens/server/StopServerModal.tsx        | 12 ++-
 .../notebook/NotebookActionsColumn.tsx        |  1 -
 .../projects/notebook/NotebookStateStatus.tsx |  1 -
 .../projects/notebook/StartNotebookModal.tsx  | 18 ++--
 .../notebook/StopNotebookConfirmModal.tsx     |  4 +-
 .../pages/projects/pvc/AddNotebookStorage.tsx | 38 ++++----
 .../data-connections/DataConnectionsList.tsx  | 19 ++--
 .../data-connections/DataConnectionsTable.tsx | 21 +++--
 .../ManageDataConnectionModal.tsx             |  4 +-
 .../detail/notebooks/NotebookTable.tsx        | 20 ++--
 .../detail/storage/ManageStorageModal.tsx     |  8 +-
 .../screens/detail/storage/StorageList.tsx    | 19 ++--
 .../screens/detail/storage/StorageTable.tsx   | 21 +++--
 .../src/utilities/notebookControllerUtils.ts  | 12 +--
 29 files changed, 262 insertions(+), 275 deletions(-)

diff --git a/frontend/.eslintrc b/frontend/.eslintrc
index 8c3142923f..5120c919b5 100755
--- a/frontend/.eslintrc
+++ b/frontend/.eslintrc
@@ -326,6 +326,10 @@
           {
             "selector": "Literal[value=/\\bOpen Data Hub\\b/i],JSXText[value=/\\bOpen Data Hub\\b/i]",
             "message": "Do not hard code product name `Open Data Hub`. Use `~/utilities/const#ODH_PRODUCT_NAME` instead."
+          },
+          {
+            "selector": "JSXElement[openingElement.name.name=/Modal/]:has(> JSXOpeningElement:has(> [name.name=/(isOpen|open)/][value]))",
+            "message": "Do not control modals visibility with 'isOpen|open', use conditional rendering instead."
           }
         ]
       }
diff --git a/frontend/src/app/DevFeatureFlagsBanner.tsx b/frontend/src/app/DevFeatureFlagsBanner.tsx
index 2a4ba5524a..7485c46d6a 100644
--- a/frontend/src/app/DevFeatureFlagsBanner.tsx
+++ b/frontend/src/app/DevFeatureFlagsBanner.tsx
@@ -102,28 +102,30 @@ const DevFeatureFlagsBanner: React.FC<Props> = ({
           </SplitItem>
         </Split>
       </Banner>
-      <Modal
-        data-testid="dev-feature-flags-modal"
-        variant="large"
-        title="Feature flags"
-        isOpen={isModalOpen}
-        onClose={() => {
-          setModalOpen(false);
-          setDevFeatureFlagQueryVisible(false);
-        }}
-        actions={[
-          <Button
-            data-testid="reset-feature-flags-modal-button"
-            key="confirm"
-            variant="link"
-            onClick={() => resetDevFeatureFlags()}
-          >
-            Reset to defaults
-          </Button>,
-        ]}
-      >
-        {renderDevFeatureFlags()}
-      </Modal>
+      {isModalOpen ? (
+        <Modal
+          data-testid="dev-feature-flags-modal"
+          variant="large"
+          title="Feature flags"
+          isOpen
+          onClose={() => {
+            setModalOpen(false);
+            setDevFeatureFlagQueryVisible(false);
+          }}
+          actions={[
+            <Button
+              data-testid="reset-feature-flags-modal-button"
+              key="confirm"
+              variant="link"
+              onClick={() => resetDevFeatureFlags()}
+            >
+              Reset to defaults
+            </Button>,
+          ]}
+        >
+          {renderDevFeatureFlags()}
+        </Modal>
+      ) : null}
     </>
   );
 };
diff --git a/frontend/src/components/EditableLabelsDescriptionListGroup.tsx b/frontend/src/components/EditableLabelsDescriptionListGroup.tsx
index c0e3958017..e405433a36 100644
--- a/frontend/src/components/EditableLabelsDescriptionListGroup.tsx
+++ b/frontend/src/components/EditableLabelsDescriptionListGroup.tsx
@@ -168,52 +168,54 @@ const EditableLabelsDescriptionListGroup: React.FC<EditableTextDescriptionListGr
           ))}
         </LabelGroup>
       </DashboardDescriptionListGroup>
-      <Modal
-        variant="small"
-        title="Add label"
-        isOpen={isAddLabelModalOpen}
-        onClose={toggleAddLabelModal}
-        actions={[
-          <Button
-            key="save"
-            variant="primary"
-            form="add-label-form"
-            onClick={submitAddLabelModal}
-            isDisabled={addLabelModalSubmitDisabled}
-          >
-            Save
-          </Button>,
-          <Button key="cancel" variant="link" onClick={toggleAddLabelModal}>
-            Cancel
-          </Button>,
-        ]}
-      >
-        <Form id="add-label-form" onSubmit={submitAddLabelModal}>
-          <FormGroup label="Label text" fieldId="add-label-form-label-text" isRequired>
-            <TextInput
-              type="text"
-              id="add-label-form-label-text"
-              name="add-label-form-label-text"
-              value={addLabelInputValue}
-              onChange={(_event: React.FormEvent<HTMLInputElement>, value: string) =>
-                setAddLabelInputValue(value)
-              }
-              ref={addLabelInputRef}
-              isRequired
-              validated={addLabelValidationError ? 'error' : 'default'}
-            />
-            {addLabelValidationError && (
-              <FormHelperText>
-                <HelperText>
-                  <HelperTextItem icon={<ExclamationCircleIcon />} variant="error">
-                    {addLabelValidationError}
-                  </HelperTextItem>
-                </HelperText>
-              </FormHelperText>
-            )}
-          </FormGroup>
-        </Form>
-      </Modal>
+      {isAddLabelModalOpen ? (
+        <Modal
+          variant="small"
+          title="Add label"
+          isOpen
+          onClose={toggleAddLabelModal}
+          actions={[
+            <Button
+              key="save"
+              variant="primary"
+              form="add-label-form"
+              onClick={submitAddLabelModal}
+              isDisabled={addLabelModalSubmitDisabled}
+            >
+              Save
+            </Button>,
+            <Button key="cancel" variant="link" onClick={toggleAddLabelModal}>
+              Cancel
+            </Button>,
+          ]}
+        >
+          <Form id="add-label-form" onSubmit={submitAddLabelModal}>
+            <FormGroup label="Label text" fieldId="add-label-form-label-text" isRequired>
+              <TextInput
+                type="text"
+                id="add-label-form-label-text"
+                name="add-label-form-label-text"
+                value={addLabelInputValue}
+                onChange={(_event: React.FormEvent<HTMLInputElement>, value: string) =>
+                  setAddLabelInputValue(value)
+                }
+                ref={addLabelInputRef}
+                isRequired
+                validated={addLabelValidationError ? 'error' : 'default'}
+              />
+              {addLabelValidationError && (
+                <FormHelperText>
+                  <HelperText>
+                    <HelperTextItem icon={<ExclamationCircleIcon />} variant="error">
+                      {addLabelValidationError}
+                    </HelperTextItem>
+                  </HelperText>
+                </FormHelperText>
+              )}
+            </FormGroup>
+          </Form>
+        </Modal>
+      ) : null}
     </>
   );
 };
diff --git a/frontend/src/pages/BYONImages/BYONImageModal/ManageBYONImageModal.tsx b/frontend/src/pages/BYONImages/BYONImageModal/ManageBYONImageModal.tsx
index 0b0cdf09df..c9723b4d6c 100644
--- a/frontend/src/pages/BYONImages/BYONImageModal/ManageBYONImageModal.tsx
+++ b/frontend/src/pages/BYONImages/BYONImageModal/ManageBYONImageModal.tsx
@@ -23,7 +23,6 @@ import DisplayedContentTabContent from './DisplayedContentTabContent';
 
 export type ManageBYONImageModalProps = {
   existingImage?: BYONImage;
-  isOpen: boolean;
   onClose: (submitted: boolean) => void;
 };
 
@@ -32,11 +31,7 @@ export enum DisplayedContentTab {
   PACKAGES,
 }
 
-const ManageBYONImageModal: React.FC<ManageBYONImageModalProps> = ({
-  existingImage,
-  isOpen,
-  onClose,
-}) => {
+const ManageBYONImageModal: React.FC<ManageBYONImageModalProps> = ({ existingImage, onClose }) => {
   const [activeTabKey, setActiveTabKey] = React.useState<string | number>(
     DisplayedContentTab.SOFTWARE,
   );
@@ -123,7 +118,7 @@ const ManageBYONImageModal: React.FC<ManageBYONImageModalProps> = ({
       onEscapePress={(e) => e.preventDefault()}
       variant={ModalVariant.medium}
       title={`${existingImage ? 'Update' : 'Import'} notebook image`}
-      isOpen={isOpen}
+      isOpen
       onClose={() => onBeforeClose(false)}
       showClose={!isEditing}
       footer={
diff --git a/frontend/src/pages/BYONImages/BYONImagesTable.tsx b/frontend/src/pages/BYONImages/BYONImagesTable.tsx
index d869ac342d..d92eb672a9 100644
--- a/frontend/src/pages/BYONImages/BYONImagesTable.tsx
+++ b/frontend/src/pages/BYONImages/BYONImagesTable.tsx
@@ -105,16 +105,17 @@ export const BYONImagesTable: React.FC<BYONImagesTableProps> = ({ images, refres
           }}
         />
       ) : null}
-      <ManageBYONImageModal
-        isOpen={!!editImage}
-        onClose={(updated) => {
-          if (updated) {
-            refresh();
-          }
-          setEditImage(undefined);
-        }}
-        existingImage={editImage}
-      />
+      {editImage ? (
+        <ManageBYONImageModal
+          onClose={(updated) => {
+            if (updated) {
+              refresh();
+            }
+            setEditImage(undefined);
+          }}
+          existingImage={editImage}
+        />
+      ) : null}
     </>
   );
 };
diff --git a/frontend/src/pages/BYONImages/ImportBYONImageButton.tsx b/frontend/src/pages/BYONImages/ImportBYONImageButton.tsx
index 1a90689119..55588980a1 100644
--- a/frontend/src/pages/BYONImages/ImportBYONImageButton.tsx
+++ b/frontend/src/pages/BYONImages/ImportBYONImageButton.tsx
@@ -18,15 +18,16 @@ const ImportBYONImageButton: React.FC<ImportBYONImageButtonProps> = ({ refresh }
       >
         Import new image
       </Button>
-      <ManageBYONImageModal
-        isOpen={isOpen}
-        onClose={(imported) => {
-          if (imported) {
-            refresh();
-          }
-          setOpen(false);
-        }}
-      />
+      {isOpen ? (
+        <ManageBYONImageModal
+          onClose={(imported) => {
+            if (imported) {
+              refresh();
+            }
+            setOpen(false);
+          }}
+        />
+      ) : null}
     </>
   );
 };
diff --git a/frontend/src/pages/acceleratorProfiles/screens/list/AcceleratorProfileEnableToggle.tsx b/frontend/src/pages/acceleratorProfiles/screens/list/AcceleratorProfileEnableToggle.tsx
index 3d84f18989..e53c81308a 100644
--- a/frontend/src/pages/acceleratorProfiles/screens/list/AcceleratorProfileEnableToggle.tsx
+++ b/frontend/src/pages/acceleratorProfiles/screens/list/AcceleratorProfileEnableToggle.tsx
@@ -55,16 +55,17 @@ const AcceleratorProfileEnableToggle: React.FC<AcceleratorProfileEnableTogglePro
           }
         }}
       />
-      <DisableAcceleratorProfileModal
-        data-testid="disable-accelerator-profile-modal"
-        isOpen={isModalOpen}
-        onClose={(confirmStatus) => {
-          if (confirmStatus) {
-            handleChange(false);
-          }
-          setIsModalOpen(false);
-        }}
-      />
+      {isModalOpen ? (
+        <DisableAcceleratorProfileModal
+          data-testid="disable-accelerator-profile-modal"
+          onClose={(confirmStatus) => {
+            if (confirmStatus) {
+              handleChange(false);
+            }
+            setIsModalOpen(false);
+          }}
+        />
+      ) : null}
     </>
   );
 };
diff --git a/frontend/src/pages/acceleratorProfiles/screens/list/DisableAcceleratorProfileModal.tsx b/frontend/src/pages/acceleratorProfiles/screens/list/DisableAcceleratorProfileModal.tsx
index 2986990002..8c09a26e76 100644
--- a/frontend/src/pages/acceleratorProfiles/screens/list/DisableAcceleratorProfileModal.tsx
+++ b/frontend/src/pages/acceleratorProfiles/screens/list/DisableAcceleratorProfileModal.tsx
@@ -2,18 +2,16 @@ import * as React from 'react';
 import { Button, Modal } from '@patternfly/react-core';
 
 type DisableAcceleratorProfileModalType = {
-  isOpen: boolean;
   onClose: (confirmStatus: boolean) => void;
 };
 
 const DisableAcceleratorProfileModal: React.FC<DisableAcceleratorProfileModalType> = ({
-  isOpen,
   onClose,
 }) => (
   <Modal
     variant="small"
     title="Disable accelerator profile"
-    isOpen={isOpen}
+    isOpen
     onClose={() => onClose(false)}
     actions={[
       <Button
diff --git a/frontend/src/pages/acceleratorProfiles/screens/manage/ManageAcceleratorProfileTolerationsSection.tsx b/frontend/src/pages/acceleratorProfiles/screens/manage/ManageAcceleratorProfileTolerationsSection.tsx
index 4a76caa0f1..6e2961bf4a 100644
--- a/frontend/src/pages/acceleratorProfiles/screens/manage/ManageAcceleratorProfileTolerationsSection.tsx
+++ b/frontend/src/pages/acceleratorProfiles/screens/manage/ManageAcceleratorProfileTolerationsSection.tsx
@@ -44,11 +44,12 @@ export const ManageAcceleratorProfileTolerationsSection: React.FC<
           onUpdate={(newTolerations) => setTolerations(newTolerations)}
         />
       </FormSection>
-      <ManageTolerationModal
-        isOpen={manageTolerationModalOpen}
-        onClose={() => setManageTolerationModalOpen(false)}
-        onSave={(toleration) => setTolerations([...tolerations, toleration])}
-      />
+      {manageTolerationModalOpen ? (
+        <ManageTolerationModal
+          onClose={() => setManageTolerationModalOpen(false)}
+          onSave={(toleration) => setTolerations([...tolerations, toleration])}
+        />
+      ) : null}
     </>
   );
 };
diff --git a/frontend/src/pages/acceleratorProfiles/screens/manage/tolerations/ManageTolerationModal.tsx b/frontend/src/pages/acceleratorProfiles/screens/manage/tolerations/ManageTolerationModal.tsx
index a8bba3d3f3..76cfe3bcec 100644
--- a/frontend/src/pages/acceleratorProfiles/screens/manage/tolerations/ManageTolerationModal.tsx
+++ b/frontend/src/pages/acceleratorProfiles/screens/manage/tolerations/ManageTolerationModal.tsx
@@ -6,14 +6,12 @@ import TolerationFields from './TolerationFields';
 import { EMPTY_TOLERATION } from './const';
 
 type ManageTolerationModalProps = {
-  isOpen: boolean;
   onClose: () => void;
   initialToleration?: Toleration;
   onSave: (toleration: Toleration) => void;
 };
 
 const ManageTolerationModal: React.FC<ManageTolerationModalProps> = ({
-  isOpen,
   onClose,
   initialToleration,
   onSave,
@@ -41,7 +39,7 @@ const ManageTolerationModal: React.FC<ManageTolerationModalProps> = ({
     <Modal
       title={initialToleration ? 'Edit toleration' : 'Add toleration'}
       variant="medium"
-      isOpen={isOpen}
+      isOpen
       onClose={() => {
         onBeforeClose();
       }}
diff --git a/frontend/src/pages/acceleratorProfiles/screens/manage/tolerations/TolerationsTable.tsx b/frontend/src/pages/acceleratorProfiles/screens/manage/tolerations/TolerationsTable.tsx
index 258e28df92..31a7fad5a6 100644
--- a/frontend/src/pages/acceleratorProfiles/screens/manage/tolerations/TolerationsTable.tsx
+++ b/frontend/src/pages/acceleratorProfiles/screens/manage/tolerations/TolerationsTable.tsx
@@ -53,21 +53,22 @@ export const TolerationsTable: React.FC<TolerationTableProps> = ({ tolerations,
           />
         )}
       />
-      <ManageTolerationModal
-        isOpen={!!editToleration}
-        initialToleration={editToleration}
-        onClose={() => {
-          setEditToleration(undefined);
-          setCurrentIndex(undefined);
-        }}
-        onSave={(toleration) => {
-          if (currentIndex !== undefined) {
-            const updatedTolerations = [...tolerations];
-            updatedTolerations[currentIndex] = toleration;
-            onUpdate(updatedTolerations);
-          }
-        }}
-      />
+      {editToleration ? (
+        <ManageTolerationModal
+          initialToleration={editToleration}
+          onClose={() => {
+            setEditToleration(undefined);
+            setCurrentIndex(undefined);
+          }}
+          onSave={(toleration) => {
+            if (currentIndex !== undefined) {
+              const updatedTolerations = [...tolerations];
+              updatedTolerations[currentIndex] = toleration;
+              onUpdate(updatedTolerations);
+            }
+          }}
+        />
+      ) : null}
     </>
   );
 };
diff --git a/frontend/src/pages/notebookController/screens/admin/NotebookAdminControl.tsx b/frontend/src/pages/notebookController/screens/admin/NotebookAdminControl.tsx
index f3c66eca05..4411bac456 100644
--- a/frontend/src/pages/notebookController/screens/admin/NotebookAdminControl.tsx
+++ b/frontend/src/pages/notebookController/screens/admin/NotebookAdminControl.tsx
@@ -104,7 +104,9 @@ const NotebookAdminControl: React.FC = () => {
           />
         </StackItem>
       </Stack>
-      <StopServerModal notebooksToStop={notebooksToStop} onNotebooksStop={onNotebooksStop} />
+      {notebooksToStop.length ? (
+        <StopServerModal notebooksToStop={notebooksToStop} onNotebooksStop={onNotebooksStop} />
+      ) : null}
     </ApplicationsPage>
   );
 };
diff --git a/frontend/src/pages/notebookController/screens/server/NotebookServer.tsx b/frontend/src/pages/notebookController/screens/server/NotebookServer.tsx
index 1be1ee9111..6cb5657b43 100644
--- a/frontend/src/pages/notebookController/screens/server/NotebookServer.tsx
+++ b/frontend/src/pages/notebookController/screens/server/NotebookServer.tsx
@@ -51,10 +51,12 @@ const NotebookServer: React.FC = () => {
         {notebook && (
           <Stack hasGutter>
             <StackItem>
-              <StopServerModal
-                notebooksToStop={notebooksToStop}
-                onNotebooksStop={onNotebooksStop}
-              />
+              {notebooksToStop.length ? (
+                <StopServerModal
+                  notebooksToStop={notebooksToStop}
+                  onNotebooksStop={onNotebooksStop}
+                />
+              ) : null}
               <ActionList>
                 <ActionListItem
                   onClick={(e) => {
diff --git a/frontend/src/pages/notebookController/screens/server/SpawnerPage.tsx b/frontend/src/pages/notebookController/screens/server/SpawnerPage.tsx
index dac574eee6..ef05af8730 100644
--- a/frontend/src/pages/notebookController/screens/server/SpawnerPage.tsx
+++ b/frontend/src/pages/notebookController/screens/server/SpawnerPage.tsx
@@ -414,22 +414,25 @@ const SpawnerPage: React.FC = () => {
           </div>
           <BrowserTabPreferenceCheckbox />
         </Form>
-        <StartServerModal
-          spawnInProgress={startShown}
-          open={createInProgress}
-          onClose={() => {
-            if (currentUserNotebook) {
-              const notebookName = currentUserNotebook.metadata.name;
-              stopNotebook(impersonatedUsername || undefined)
-                .then(() => requestNotebookRefresh())
-                .catch((e) => notification.error(`Error stop notebook ${notebookName}`, e.message));
-            } else {
-              // Shouldn't happen, but if we don't have a notebook, there is nothing to stop
-              hideStartShown();
-            }
-            setCreateInProgress(false);
-          }}
-        />
+        {createInProgress ? (
+          <StartServerModal
+            spawnInProgress={startShown}
+            onClose={() => {
+              if (currentUserNotebook) {
+                const notebookName = currentUserNotebook.metadata.name;
+                stopNotebook(impersonatedUsername || undefined)
+                  .then(() => requestNotebookRefresh())
+                  .catch((e) =>
+                    notification.error(`Error stop notebook ${notebookName}`, e.message),
+                  );
+              } else {
+                // Shouldn't happen, but if we don't have a notebook, there is nothing to stop
+                hideStartShown();
+              }
+              setCreateInProgress(false);
+            }}
+          />
+        ) : null}
       </ApplicationsPage>
     </>
   );
diff --git a/frontend/src/pages/notebookController/screens/server/StartServerModal.tsx b/frontend/src/pages/notebookController/screens/server/StartServerModal.tsx
index fb0d04dc94..4fa067d584 100644
--- a/frontend/src/pages/notebookController/screens/server/StartServerModal.tsx
+++ b/frontend/src/pages/notebookController/screens/server/StartServerModal.tsx
@@ -28,7 +28,6 @@ import '~/pages/notebookController/NotebookController.scss';
 
 type StartServerModalProps = {
   spawnInProgress: boolean;
-  open: boolean;
   onClose: () => void;
 };
 
@@ -38,28 +37,19 @@ type SpawnStatus = {
   description: React.ReactNode;
 };
 
-const StartServerModal: React.FC<StartServerModalProps> = ({ open, spawnInProgress, onClose }) => {
+const StartServerModal: React.FC<StartServerModalProps> = ({ spawnInProgress, onClose }) => {
   const { currentUserNotebookIsRunning: isNotebookRunning } =
     React.useContext(NotebookControllerContext);
   const [logsExpanded, setLogsExpanded] = React.useState(false);
   const [spawnPercentile, setSpawnPercentile] = React.useState(0);
   const [spawnStatus, setSpawnStatus] = React.useState<SpawnStatus | null>(null);
-  const [unstableNotebookStatus, events] = useNotebookStatus(spawnInProgress, open);
+  const [unstableNotebookStatus, events] = useNotebookStatus(spawnInProgress);
   const [isUsingCurrentTab] = useBrowserTabPreference();
   const notebookStatus = useDeepCompareMemoize(unstableNotebookStatus);
   const getNotebookLink = useNotebookRedirectLink();
   const navigate = useNavigate();
   const spawnFailed = spawnStatus?.status === AlertVariant.danger;
 
-  React.useEffect(() => {
-    if (!open) {
-      // Reset the modal
-      setLogsExpanded(false);
-      setSpawnPercentile(0);
-      setSpawnStatus(null);
-    }
-  }, [open]);
-
   const navigateToNotebook = React.useCallback(
     (useCurrentTab: boolean): void => {
       getNotebookLink()
@@ -85,7 +75,7 @@ const StartServerModal: React.FC<StartServerModalProps> = ({ open, spawnInProgre
 
   React.useEffect(() => {
     let timer: ReturnType<typeof setTimeout>;
-    if (isNotebookRunning && open) {
+    if (isNotebookRunning) {
       setSpawnPercentile(100);
       setSpawnStatus({
         status: AlertVariant.success,
@@ -101,10 +91,10 @@ const StartServerModal: React.FC<StartServerModalProps> = ({ open, spawnInProgre
     return () => {
       clearTimeout(timer);
     };
-  }, [isNotebookRunning, open, isUsingCurrentTab, navigateToNotebook]);
+  }, [isNotebookRunning, isUsingCurrentTab, navigateToNotebook]);
 
   React.useEffect(() => {
-    if (spawnInProgress && !isNotebookRunning && open) {
+    if (spawnInProgress && !isNotebookRunning) {
       if (!notebookStatus) {
         return;
       }
@@ -131,7 +121,7 @@ const StartServerModal: React.FC<StartServerModalProps> = ({ open, spawnInProgre
         });
       }
     }
-  }, [notebookStatus, spawnInProgress, isNotebookRunning, open]);
+  }, [notebookStatus, spawnInProgress, isNotebookRunning]);
 
   const renderProgress = () => {
     let variant: ProgressVariant | undefined;
@@ -153,7 +143,7 @@ const StartServerModal: React.FC<StartServerModalProps> = ({ open, spawnInProgre
     let title: string;
     if (events.length > 0 && currentEvent) {
       title = currentEvent;
-    } else if (open && !spawnInProgress) {
+    } else if (!spawnInProgress) {
       title = 'Creating resources...';
     } else {
       title = 'Waiting for server request to start...';
@@ -242,7 +232,7 @@ const StartServerModal: React.FC<StartServerModalProps> = ({ open, spawnInProgre
       appendTo={document.body}
       variant={ModalVariant.small}
       title="Starting server"
-      isOpen={open}
+      isOpen
       showClose={spawnFailed}
       onClose={onClose}
     >
diff --git a/frontend/src/pages/notebookController/screens/server/StopServerModal.tsx b/frontend/src/pages/notebookController/screens/server/StopServerModal.tsx
index b8f38d16c9..133924f611 100644
--- a/frontend/src/pages/notebookController/screens/server/StopServerModal.tsx
+++ b/frontend/src/pages/notebookController/screens/server/StopServerModal.tsx
@@ -15,12 +15,14 @@ const StopServerModal: React.FC<StopServerModalProps> = ({ notebooksToStop, onNo
   const notification = useNotification();
   const [isDeleting, setDeleting] = React.useState(false);
 
-  const hasMultipleServers = notebooksToStop.length > 1;
-  const textToShow = hasMultipleServers ? 'all servers' : 'server';
+  const { isAdmin } = useUser();
 
-  const isModalShown = notebooksToStop.length !== 0;
+  if (!notebooksToStop.length) {
+    return null;
+  }
 
-  const { isAdmin } = useUser();
+  const hasMultipleServers = notebooksToStop.length > 1;
+  const textToShow = hasMultipleServers ? 'all servers' : 'server';
 
   const onClose = () => {
     onNotebooksStop(false);
@@ -79,7 +81,7 @@ const StopServerModal: React.FC<StopServerModalProps> = ({ notebooksToStop, onNo
       appendTo={document.body}
       variant={ModalVariant.small}
       title={`Stop ${textToShow}`}
-      isOpen={isModalShown}
+      isOpen
       showClose
       onClose={onClose}
       actions={modalActions}
diff --git a/frontend/src/pages/projects/notebook/NotebookActionsColumn.tsx b/frontend/src/pages/projects/notebook/NotebookActionsColumn.tsx
index c16ac0d886..9e534d68a1 100644
--- a/frontend/src/pages/projects/notebook/NotebookActionsColumn.tsx
+++ b/frontend/src/pages/projects/notebook/NotebookActionsColumn.tsx
@@ -127,7 +127,6 @@ export const useNotebookActionsColumn = (
       />
       {isOpenConfirm ? (
         <StopNotebookConfirmModal
-          isOpen
           notebookState={notebookState}
           onClose={(confirmStatus) => {
             if (confirmStatus) {
diff --git a/frontend/src/pages/projects/notebook/NotebookStateStatus.tsx b/frontend/src/pages/projects/notebook/NotebookStateStatus.tsx
index 4e8559a8ba..c25d4363e5 100644
--- a/frontend/src/pages/projects/notebook/NotebookStateStatus.tsx
+++ b/frontend/src/pages/projects/notebook/NotebookStateStatus.tsx
@@ -100,7 +100,6 @@ const NotebookStateStatus: React.FC<NotebookStateStatusProps> = ({
         </Popover>
         {isStartModalOpen ? (
           <StartNotebookModal
-            isOpen
             notebookState={notebookState}
             notebookStatus={notebookStatus}
             events={events}
diff --git a/frontend/src/pages/projects/notebook/StartNotebookModal.tsx b/frontend/src/pages/projects/notebook/StartNotebookModal.tsx
index 43288cb40c..f40680f8d9 100644
--- a/frontend/src/pages/projects/notebook/StartNotebookModal.tsx
+++ b/frontend/src/pages/projects/notebook/StartNotebookModal.tsx
@@ -22,7 +22,6 @@ import { NotebookState } from './types';
 import { getEventFullMessage, getEventTimestamp } from './utils';
 
 type StartNotebookModalProps = {
-  isOpen: boolean;
   notebookState: NotebookState;
   notebookStatus: NotebookStatus | null;
   events: EventKind[];
@@ -36,7 +35,6 @@ type SpawnStatus = {
 };
 
 const StartNotebookModal: React.FC<StartNotebookModalProps> = ({
-  isOpen,
   notebookStatus,
   events,
   notebookState,
@@ -49,11 +47,7 @@ const StartNotebookModal: React.FC<StartNotebookModalProps> = ({
   const spawnFailed = spawnStatus?.status === AlertVariant.danger;
 
   React.useEffect(() => {
-    if (!isOpen) {
-      // Reset the modal
-      setSpawnPercentile(0);
-      setSpawnStatus(null);
-    } else if (isRunning) {
+    if (isRunning) {
       setSpawnPercentile(100);
       setSpawnStatus({
         status: AlertVariant.success,
@@ -61,10 +55,10 @@ const StartNotebookModal: React.FC<StartNotebookModalProps> = ({
         description: 'The notebook server is up and running.',
       });
     }
-  }, [isOpen, isRunning]);
+  }, [isRunning]);
 
   React.useEffect(() => {
-    if (isStarting && !isRunning && isOpen) {
+    if (isStarting && !isRunning) {
       if (!notebookStatus) {
         return;
       }
@@ -91,7 +85,7 @@ const StartNotebookModal: React.FC<StartNotebookModalProps> = ({
         });
       }
     }
-  }, [notebookStatus, isStarting, isRunning, isOpen]);
+  }, [notebookStatus, isStarting, isRunning]);
 
   const renderProgress = () => {
     let variant: ProgressVariant | undefined;
@@ -113,7 +107,7 @@ const StartNotebookModal: React.FC<StartNotebookModalProps> = ({
     let title: string;
     if (events.length > 0 && currentEvent) {
       title = currentEvent;
-    } else if (isOpen && !isStarting) {
+    } else if (!isStarting) {
       title = 'Creating resources...';
     } else {
       title = 'Waiting for server request to start...';
@@ -189,7 +183,7 @@ const StartNotebookModal: React.FC<StartNotebookModalProps> = ({
       appendTo={document.body}
       variant={ModalVariant.small}
       title="Starting server"
-      isOpen={isOpen}
+      isOpen
       showClose
       onClose={() => onClose(false)}
     >
diff --git a/frontend/src/pages/projects/notebook/StopNotebookConfirmModal.tsx b/frontend/src/pages/projects/notebook/StopNotebookConfirmModal.tsx
index 053489f613..ea31ab7cc2 100644
--- a/frontend/src/pages/projects/notebook/StopNotebookConfirmModal.tsx
+++ b/frontend/src/pages/projects/notebook/StopNotebookConfirmModal.tsx
@@ -5,13 +5,11 @@ import useStopNotebookModalAvailability from './useStopNotebookModalAvailability
 import { NotebookState } from './types';
 
 type StopNotebookConfirmProps = {
-  isOpen: boolean;
   notebookState: NotebookState;
   onClose: (confirmStatus: boolean) => void;
 };
 
 const StopNotebookConfirmModal: React.FC<StopNotebookConfirmProps> = ({
-  isOpen,
   notebookState,
   onClose,
 }) => {
@@ -31,7 +29,7 @@ const StopNotebookConfirmModal: React.FC<StopNotebookConfirmProps> = ({
       variant="small"
       title="Stop workbench?"
       data-testid="stop-workbench-modal"
-      isOpen={isOpen}
+      isOpen
       onClose={() => onBeforeClose(false)}
       actions={[
         <Button
diff --git a/frontend/src/pages/projects/pvc/AddNotebookStorage.tsx b/frontend/src/pages/projects/pvc/AddNotebookStorage.tsx
index 429a8e19cc..97f785c875 100644
--- a/frontend/src/pages/projects/pvc/AddNotebookStorage.tsx
+++ b/frontend/src/pages/projects/pvc/AddNotebookStorage.tsx
@@ -11,7 +11,7 @@ import { getDisplayNameFromK8sResource } from '~/concepts/k8s/utils';
 import MountPathField from './MountPathField';
 
 type AddNotebookStorageProps = {
-  notebook?: NotebookKind;
+  notebook: NotebookKind;
   onClose: (submitted: boolean) => void;
 };
 
@@ -19,9 +19,9 @@ const AddNotebookStorage: React.FC<AddNotebookStorageProps> = ({ notebook, onClo
   const [existingData, setExistingData, resetDefaults] = useExistingStorageDataObjectForNotebook();
   const [isSubmitting, setIsSubmitting] = React.useState(false);
   const [error, setError] = React.useState<Error | undefined>();
-  const notebookDisplayName = notebook ? getDisplayNameFromK8sResource(notebook) : 'this notebook';
+  const notebookDisplayName = getDisplayNameFromK8sResource(notebook);
   const inUseMountPaths = getNotebookMountPaths(notebook);
-  const restartNotebooks = useWillNotebooksRestart([notebook?.metadata.name || '']);
+  const restartNotebooks = useWillNotebooksRestart([notebook.metadata.name]);
 
   const canSubmit =
     !isSubmitting &&
@@ -36,29 +36,27 @@ const AddNotebookStorage: React.FC<AddNotebookStorageProps> = ({ notebook, onClo
   };
 
   const submit = () => {
-    if (notebook) {
-      setIsSubmitting(true);
-      attachNotebookPVC(
-        notebook.metadata.name,
-        notebook.metadata.namespace,
-        existingData.name,
-        existingData.mountPath.value,
-      )
-        .then(() => {
-          beforeClose(true);
-        })
-        .catch((e) => {
-          setError(e);
-          setIsSubmitting(false);
-        });
-    }
+    setIsSubmitting(true);
+    attachNotebookPVC(
+      notebook.metadata.name,
+      notebook.metadata.namespace,
+      existingData.name,
+      existingData.mountPath.value,
+    )
+      .then(() => {
+        beforeClose(true);
+      })
+      .catch((e) => {
+        setError(e);
+        setIsSubmitting(false);
+      });
   };
 
   return (
     <Modal
       variant="small"
       title={`Add storage to ${notebookDisplayName}`}
-      isOpen={!!notebook}
+      isOpen
       onClose={() => beforeClose(false)}
       actions={[
         <Button
diff --git a/frontend/src/pages/projects/screens/detail/data-connections/DataConnectionsList.tsx b/frontend/src/pages/projects/screens/detail/data-connections/DataConnectionsList.tsx
index 8e3c3cff9f..34a8759b1d 100644
--- a/frontend/src/pages/projects/screens/detail/data-connections/DataConnectionsList.tsx
+++ b/frontend/src/pages/projects/screens/detail/data-connections/DataConnectionsList.tsx
@@ -81,15 +81,16 @@ const DataConnectionsList: React.FC = () => {
           <DataConnectionsTable connections={connections} refreshData={refreshAllProjectData} />
         ) : null}
       </DetailsSection>
-      <ManageDataConnectionModal
-        isOpen={open}
-        onClose={(submitted) => {
-          if (submitted) {
-            refreshAllProjectData();
-          }
-          setOpen(false);
-        }}
-      />
+      {open ? (
+        <ManageDataConnectionModal
+          onClose={(submitted) => {
+            if (submitted) {
+              refreshAllProjectData();
+            }
+            setOpen(false);
+          }}
+        />
+      ) : null}
     </>
   );
 };
diff --git a/frontend/src/pages/projects/screens/detail/data-connections/DataConnectionsTable.tsx b/frontend/src/pages/projects/screens/detail/data-connections/DataConnectionsTable.tsx
index 580a21f8b8..b6377b123c 100644
--- a/frontend/src/pages/projects/screens/detail/data-connections/DataConnectionsTable.tsx
+++ b/frontend/src/pages/projects/screens/detail/data-connections/DataConnectionsTable.tsx
@@ -38,16 +38,17 @@ const DataConnectionsTable: React.FC<DataConnectionsTableProps> = ({
           />
         )}
       />
-      <ManageDataConnectionModal
-        isOpen={!!editDataConnection}
-        existingData={editDataConnection}
-        onClose={(updated) => {
-          if (updated) {
-            refreshData();
-          }
-          setEditDataConnection(undefined);
-        }}
-      />
+      {editDataConnection ? (
+        <ManageDataConnectionModal
+          existingData={editDataConnection}
+          onClose={(updated) => {
+            if (updated) {
+              refreshData();
+            }
+            setEditDataConnection(undefined);
+          }}
+        />
+      ) : null}
       {deleteDataConnection ? (
         <DeleteDataConnectionModal
           dataConnection={deleteDataConnection}
diff --git a/frontend/src/pages/projects/screens/detail/data-connections/ManageDataConnectionModal.tsx b/frontend/src/pages/projects/screens/detail/data-connections/ManageDataConnectionModal.tsx
index 558da87ac4..ce0f1fb593 100644
--- a/frontend/src/pages/projects/screens/detail/data-connections/ManageDataConnectionModal.tsx
+++ b/frontend/src/pages/projects/screens/detail/data-connections/ManageDataConnectionModal.tsx
@@ -23,12 +23,10 @@ import { convertAWSSecretData } from './utils';
 
 type ManageDataConnectionModalProps = {
   existingData?: DataConnection;
-  isOpen: boolean;
   onClose: (submitted: boolean) => void;
 };
 
 const ManageDataConnectionModal: React.FC<ManageDataConnectionModalProps> = ({
-  isOpen,
   onClose,
   existingData,
 }) => {
@@ -149,7 +147,7 @@ const ManageDataConnectionModal: React.FC<ManageDataConnectionModalProps> = ({
     <Modal
       title={existingData ? 'Edit data connection' : 'Add data connection'}
       variant="medium"
-      isOpen={isOpen}
+      isOpen
       onClose={() => onBeforeClose(false)}
       showClose
       actions={[
diff --git a/frontend/src/pages/projects/screens/detail/notebooks/NotebookTable.tsx b/frontend/src/pages/projects/screens/detail/notebooks/NotebookTable.tsx
index e386a390f4..3bc5decd87 100644
--- a/frontend/src/pages/projects/screens/detail/notebooks/NotebookTable.tsx
+++ b/frontend/src/pages/projects/screens/detail/notebooks/NotebookTable.tsx
@@ -48,15 +48,17 @@ const NotebookTable: React.FC<NotebookTableProps> = ({ notebookStates, refresh }
           </CanEnableElyraPipelinesCheck>
         )}
       </ElyraInvalidVersionAlerts>
-      <AddNotebookStorage
-        notebook={addNotebookStorage}
-        onClose={(submitted) => {
-          if (submitted) {
-            refresh();
-          }
-          setAddNotebookStorage(undefined);
-        }}
-      />
+      {addNotebookStorage ? (
+        <AddNotebookStorage
+          notebook={addNotebookStorage}
+          onClose={(submitted) => {
+            if (submitted) {
+              refresh();
+            }
+            setAddNotebookStorage(undefined);
+          }}
+        />
+      ) : null}
       {notebookToDelete ? (
         <DeleteNotebookModal
           notebook={notebookToDelete}
diff --git a/frontend/src/pages/projects/screens/detail/storage/ManageStorageModal.tsx b/frontend/src/pages/projects/screens/detail/storage/ManageStorageModal.tsx
index 807fb54607..0a57076722 100644
--- a/frontend/src/pages/projects/screens/detail/storage/ManageStorageModal.tsx
+++ b/frontend/src/pages/projects/screens/detail/storage/ManageStorageModal.tsx
@@ -21,11 +21,10 @@ import ExistingConnectedNotebooks from './ExistingConnectedNotebooks';
 
 type AddStorageModalProps = {
   existingData?: PersistentVolumeClaimKind;
-  isOpen: boolean;
   onClose: (submit: boolean) => void;
 };
 
-const ManageStorageModal: React.FC<AddStorageModalProps> = ({ existingData, isOpen, onClose }) => {
+const ManageStorageModal: React.FC<AddStorageModalProps> = ({ existingData, onClose }) => {
   const isStorageClassesAvailable = useIsAreaAvailable(SupportedArea.STORAGE_CLASSES).status;
   const preferredStorageClass = usePreferredStorageClass();
   const defaultStorageClass = useDefaultStorageClass();
@@ -53,7 +52,7 @@ const ManageStorageModal: React.FC<AddStorageModalProps> = ({ existingData, isOp
   ]);
 
   React.useEffect(() => {
-    if (!existingData && isOpen) {
+    if (!existingData) {
       if (isStorageClassesAvailable) {
         setCreateData('storageClassName', defaultStorageClass?.metadata.name);
       } else {
@@ -65,7 +64,6 @@ const ManageStorageModal: React.FC<AddStorageModalProps> = ({ existingData, isOp
     defaultStorageClass,
     preferredStorageClass,
     existingData,
-    isOpen,
     setCreateData,
   ]);
 
@@ -150,7 +148,7 @@ const ManageStorageModal: React.FC<AddStorageModalProps> = ({ existingData, isOp
           : 'Add storage and optionally connect it with an existing workbench.'
       }
       variant="medium"
-      isOpen={isOpen}
+      isOpen
       onClose={() => onBeforeClose(false)}
       showClose
       footer={
diff --git a/frontend/src/pages/projects/screens/detail/storage/StorageList.tsx b/frontend/src/pages/projects/screens/detail/storage/StorageList.tsx
index ee2f2b54a3..612b34f358 100644
--- a/frontend/src/pages/projects/screens/detail/storage/StorageList.tsx
+++ b/frontend/src/pages/projects/screens/detail/storage/StorageList.tsx
@@ -72,15 +72,16 @@ const StorageList: React.FC = () => {
           <StorageTable pvcs={pvcs} refresh={refresh} onAddPVC={() => setOpen(true)} />
         ) : null}
       </DetailsSection>
-      <ManageStorageModal
-        isOpen={isOpen}
-        onClose={(submit: boolean) => {
-          setOpen(false);
-          if (submit) {
-            refresh();
-          }
-        }}
-      />
+      {isOpen ? (
+        <ManageStorageModal
+          onClose={(submit: boolean) => {
+            setOpen(false);
+            if (submit) {
+              refresh();
+            }
+          }}
+        />
+      ) : null}
     </>
   );
 };
diff --git a/frontend/src/pages/projects/screens/detail/storage/StorageTable.tsx b/frontend/src/pages/projects/screens/detail/storage/StorageTable.tsx
index 3c9abee323..de8c73748c 100644
--- a/frontend/src/pages/projects/screens/detail/storage/StorageTable.tsx
+++ b/frontend/src/pages/projects/screens/detail/storage/StorageTable.tsx
@@ -84,16 +84,17 @@ const StorageTable: React.FC<StorageTableProps> = ({ pvcs, refresh, onAddPVC })
           />
         )}
       />
-      <ManageStorageModal
-        isOpen={!!editPVC}
-        existingData={editPVC}
-        onClose={(updated) => {
-          if (updated) {
-            refresh();
-          }
-          setEditPVC(undefined);
-        }}
-      />
+      {editPVC ? (
+        <ManageStorageModal
+          existingData={editPVC}
+          onClose={(updated) => {
+            if (updated) {
+              refresh();
+            }
+            setEditPVC(undefined);
+          }}
+        />
+      ) : null}
       {deleteStorage ? (
         <DeletePVCModal
           pvcToDelete={deleteStorage}
diff --git a/frontend/src/utilities/notebookControllerUtils.ts b/frontend/src/utilities/notebookControllerUtils.ts
index 5d62bcaa82..bb71b1d691 100644
--- a/frontend/src/utilities/notebookControllerUtils.ts
+++ b/frontend/src/utilities/notebookControllerUtils.ts
@@ -289,13 +289,11 @@ const filterEvents = (
   return [filteredEvents, thisInstanceEvents, gracePeriod];
 };
 
-const useLastActivity = (storeValue: boolean, annotationValue?: string): Date | null => {
+const useLastActivity = (annotationValue?: string): Date | null => {
   const lastOpenActivity = React.useRef<Date | null>(null);
 
-  if (storeValue && annotationValue && !lastOpenActivity.current) {
+  if (annotationValue && !lastOpenActivity.current) {
     lastOpenActivity.current = new Date(annotationValue);
-  } else if (!storeValue && lastOpenActivity.current) {
-    lastOpenActivity.current = null;
   }
 
   return lastOpenActivity.current;
@@ -303,7 +301,6 @@ const useLastActivity = (storeValue: boolean, annotationValue?: string): Date |
 
 export const useNotebookStatus = (
   spawnInProgress: boolean,
-  open: boolean,
 ): [status: NotebookStatus | null, events: K8sEvent[]] => {
   const {
     currentUserNotebook: notebook,
@@ -318,10 +315,7 @@ export const useNotebookStatus = (
   );
 
   const lastActivity =
-    useLastActivity(
-      open,
-      notebook?.metadata.annotations?.['notebooks.kubeflow.org/last-activity'],
-    ) ||
+    useLastActivity(notebook?.metadata.annotations?.['notebooks.kubeflow.org/last-activity']) ||
     (notebook && (spawnInProgress || isNotebookRunning)
       ? new Date(notebook.metadata.creationTimestamp ?? 0)
       : null);