Require conditional rendering of modals

opendatahub-io · Oct 10, 2024 · dcf71cc · dcf71cc
1 parent c8d7822
commit dcf71cc
Show file tree

Hide file tree

Showing 29 changed files with 262 additions and 275 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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/...c/pages/acceleratorProfiles/screens/manage/ManageAcceleratorProfileTolerationsSection.tsx b/...c/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
@@ -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
@@ -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
@@ -104,7 +104,9 @@ const NotebookAdminControl: React.FC = () => {
           />
         </StackItem>
       </Stack>
-      <StopServerModal notebooksToStop={notebooksToStop} onNotebooksStop={onNotebooksStop} />
+      {notebooksToStop.length ? (
+        <StopServerModal notebooksToStop={notebooksToStop} onNotebooksStop={onNotebooksStop} />
+      ) : null}
     </ApplicationsPage>
   );
 };