From a2d43775a818a8b393e4488dcdc4c8653b47727f Mon Sep 17 00:00:00 2001
From: Charlie Park <charlie@oxidecomputer.com>
Date: Thu, 12 Dec 2024 11:10:22 -0800
Subject: [PATCH 1/3] Remove sidemodal footer when read-only

---
 app/components/form/SideModalForm.tsx | 24 +++++++++++++++++-------
 app/ui/lib/SideModal.tsx              | 12 +-----------
 2 files changed, 18 insertions(+), 18 deletions(-)

diff --git a/app/components/form/SideModalForm.tsx b/app/components/form/SideModalForm.tsx
index d1e6075fea..c037074bd3 100644
--- a/app/components/form/SideModalForm.tsx
+++ b/app/components/form/SideModalForm.tsx
@@ -5,11 +5,13 @@
  *
  * Copyright Oxide Computer Company
  */
+import * as Dialog from '@radix-ui/react-dialog'
 import { useEffect, useId, useState, type ReactNode } from 'react'
 import type { FieldValues, UseFormReturn } from 'react-hook-form'
 import { NavigationType, useNavigationType } from 'react-router-dom'
 
 import type { ApiError } from '@oxide/api'
+import { Close12Icon } from '@oxide/design-system/icons/react'
 
 import { Button } from '~/ui/lib/Button'
 import { Modal } from '~/ui/lib/Modal'
@@ -127,11 +129,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 +144,16 @@ export function SideModalForm<TFieldValues extends FieldValues>({
           >
             {label}
           </Button>
-        )}
-      </SideModal.Footer>
+        </SideModal.Footer>
+      )}
+
+      {/* 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>
 
       {showNavGuard && (
         <Modal
diff --git a/app/ui/lib/SideModal.tsx b/app/ui/lib/SideModal.tsx
index 494901b356..4fd21f8260 100644
--- a/app/ui/lib/SideModal.tsx
+++ b/app/ui/lib/SideModal.tsx
@@ -10,7 +10,7 @@ import { animated, useTransition } from '@react-spring/web'
 import cn from 'classnames'
 import React, { useRef, type ReactNode } from 'react'
 
-import { Close12Icon, Error12Icon } from '@oxide/design-system/icons/react'
+import { Error12Icon } from '@oxide/design-system/icons/react'
 
 import { useIsOverflow } from '~/hooks/use-is-overflow'
 import { Message } from '~/ui/lib/Message'
@@ -161,15 +161,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>
 )

From 240fce0bbcf450936c23e406c61edc2f7f70370e Mon Sep 17 00:00:00 2001
From: Charlie Park <charlie@oxidecomputer.com>
Date: Thu, 12 Dec 2024 13:16:45 -0800
Subject: [PATCH 2/3] Update test to use X to close modal

---
 test/e2e/silos.e2e.ts | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/test/e2e/silos.e2e.ts b/test/e2e/silos.e2e.ts
index 87a33a84da..e0b8dd1dc8 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

From 7fe3cfe08b5e2777247fcf15dc2028174bb11718 Mon Sep 17 00:00:00 2001
From: David Crespo <david.crespo@oxidecomputer.com>
Date: Fri, 13 Dec 2024 11:44:42 -0600
Subject: [PATCH 3/3] move close button back into SideModal

---
 app/components/form/SideModalForm.tsx | 11 +----------
 app/ui/lib/SideModal.tsx              | 10 +++++++++-
 2 files changed, 10 insertions(+), 11 deletions(-)

diff --git a/app/components/form/SideModalForm.tsx b/app/components/form/SideModalForm.tsx
index c037074bd3..4aadd41152 100644
--- a/app/components/form/SideModalForm.tsx
+++ b/app/components/form/SideModalForm.tsx
@@ -5,13 +5,12 @@
  *
  * Copyright Oxide Computer Company
  */
-import * as Dialog from '@radix-ui/react-dialog'
+
 import { useEffect, useId, useState, type ReactNode } from 'react'
 import type { FieldValues, UseFormReturn } from 'react-hook-form'
 import { NavigationType, useNavigationType } from 'react-router-dom'
 
 import type { ApiError } from '@oxide/api'
-import { Close12Icon } from '@oxide/design-system/icons/react'
 
 import { Button } from '~/ui/lib/Button'
 import { Modal } from '~/ui/lib/Modal'
@@ -147,14 +146,6 @@ export function SideModalForm<TFieldValues extends FieldValues>({
         </SideModal.Footer>
       )}
 
-      {/* 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>
-
       {showNavGuard && (
         <Modal
           isOpen
diff --git a/app/ui/lib/SideModal.tsx b/app/ui/lib/SideModal.tsx
index 4fd21f8260..01aae0f70c 100644
--- a/app/ui/lib/SideModal.tsx
+++ b/app/ui/lib/SideModal.tsx
@@ -10,7 +10,7 @@ import { animated, useTransition } from '@react-spring/web'
 import cn from 'classnames'
 import React, { useRef, type ReactNode } from 'react'
 
-import { Error12Icon } from '@oxide/design-system/icons/react'
+import { Close12Icon, Error12Icon } from '@oxide/design-system/icons/react'
 
 import { useIsOverflow } from '~/hooks/use-is-overflow'
 import { Message } from '~/ui/lib/Message'
@@ -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>