From 2b0c3f125dfc9f147417873dbcdd94139d82b916 Mon Sep 17 00:00:00 2001
From: Charlie Park <charlie@oxidecomputer.com>
Date: Fri, 13 Dec 2024 10:51:42 -0800
Subject: [PATCH] Remove footer when side modal is read-only (#2621)

* Remove sidemodal footer when read-only

* Update test to use X to close modal

* move close button back into SideModal

---------

Co-authored-by: David Crespo <david.crespo@oxidecomputer.com>
---
 app/components/form/SideModalForm.tsx | 15 ++++++++-------
 app/ui/lib/SideModal.tsx              | 18 ++++++++----------
 test/e2e/silos.e2e.ts                 |  4 ++--
 3 files changed, 18 insertions(+), 19 deletions(-)

diff --git a/app/components/form/SideModalForm.tsx b/app/components/form/SideModalForm.tsx
index d1e6075fe..4aadd4115 100644
--- a/app/components/form/SideModalForm.tsx
+++ b/app/components/form/SideModalForm.tsx
@@ -5,6 +5,7 @@
  *
  * Copyright Oxide Computer Company
  */
+
 import { useEffect, useId, useState, type ReactNode } from 'react'
 import type { FieldValues, UseFormReturn } from 'react-hook-form'
 import { NavigationType, useNavigationType } from 'react-router-dom'
@@ -127,11 +128,11 @@ export function SideModalForm<TFieldValues extends FieldValues>({
           {children}
         </form>
       </SideModal.Body>
-      <SideModal.Footer error={!!submitError}>
-        <Button variant="ghost" size="sm" onClick={onDismiss}>
-          Cancel
-        </Button>
-        {onSubmit && (
+      {onSubmit && (
+        <SideModal.Footer error={!!submitError}>
+          <Button variant="ghost" size="sm" onClick={onDismiss}>
+            Cancel
+          </Button>
           <Button
             type="submit"
             size="sm"
@@ -142,8 +143,8 @@ export function SideModalForm<TFieldValues extends FieldValues>({
           >
             {label}
           </Button>
-        )}
-      </SideModal.Footer>
+        </SideModal.Footer>
+      )}
 
       {showNavGuard && (
         <Modal
diff --git a/app/ui/lib/SideModal.tsx b/app/ui/lib/SideModal.tsx
index 494901b35..01aae0f70 100644
--- a/app/ui/lib/SideModal.tsx
+++ b/app/ui/lib/SideModal.tsx
@@ -115,6 +115,14 @@ export function SideModal({
                     </div>
                   )}
                   {children}
+
+                  {/* Close button is here at the end so we aren't automatically focusing on it when the side modal is opened. Positioned in the safe area at the top */}
+                  <Dialog.Close
+                    className="absolute right-[var(--content-gutter)] top-10 -m-2 flex rounded p-2 hover:bg-hover"
+                    aria-label="Close"
+                  >
+                    <Close12Icon className="text-default" />
+                  </Dialog.Close>
                 </AnimatedDialogContent>
               </Dialog.Portal>
             </Dialog.Root>
@@ -161,15 +169,5 @@ SideModal.Footer = ({ children, error }: { children: ReactNode; error?: boolean
       </div>
     )}
     {children}
-    {/*
-      Close button is here at the end so we aren't automatically focusing on it
-      when the side modal is opened. Positioned in the safe area at the top
-     */}
-    <Dialog.Close
-      className="absolute right-[var(--content-gutter)] top-10 -m-2 flex rounded p-2 hover:bg-hover"
-      aria-label="Close"
-    >
-      <Close12Icon className="text-default" />
-    </Dialog.Close>
   </footer>
 )
diff --git a/test/e2e/silos.e2e.ts b/test/e2e/silos.e2e.ts
index 87a33a84d..e0b8dd1dc 100644
--- a/test/e2e/silos.e2e.ts
+++ b/test/e2e/silos.e2e.ts
@@ -200,7 +200,7 @@ test('Identity providers', async ({ page }) => {
     'groups'
   )
 
-  await page.getByRole('button', { name: 'Cancel' }).click()
+  await page.getByRole('button', { name: 'Close' }).click()
 
   await expect(dialog).toBeHidden()
 
@@ -320,7 +320,7 @@ test('Silo IP pools link pool', async ({ page }) => {
   await expect(modal).toBeVisible()
 
   // close modal works
-  await page.getByRole('button', { name: 'Cancel' }).click()
+  await page.getByRole('button', { name: 'Close' }).click()
   await expect(modal).toBeHidden()
 
   // reopen