From dc402c8ff5f4d639a22b3488c07ea0b548153c89 Mon Sep 17 00:00:00 2001
From: Alejandro Celaya <alejandro.celaya@hypothes.is>
Date: Mon, 22 May 2023 17:17:53 +0200
Subject: [PATCH] Make sure Dialogs do not render close button when onClose
 callback is not provided

---
 src/components/feedback/Dialog.tsx          | 11 +++++------
 src/components/feedback/test/Dialog-test.js |  7 +++++++
 2 files changed, 12 insertions(+), 6 deletions(-)

diff --git a/src/components/feedback/Dialog.tsx b/src/components/feedback/Dialog.tsx
index 10fddf80..d72ca496 100644
--- a/src/components/feedback/Dialog.tsx
+++ b/src/components/feedback/Dialog.tsx
@@ -54,8 +54,6 @@ export type DialogProps = PresentationalProps &
   ComponentProps &
   Omit<PanelProps, 'fullWidthHeader'>;
 
-const noop = () => {};
-
 /**
  * Show a dialog
  */
@@ -74,7 +72,7 @@ const Dialog = function Dialog({
   // Forwarded to Panel
   buttons,
   icon,
-  onClose = noop,
+  onClose,
   paddingSize = 'md',
   scrollable = true,
   title,
@@ -87,6 +85,7 @@ const Dialog = function Dialog({
   );
   const [transitionComponentVisible, setTransitionComponentVisible] =
     useState(false);
+  const isClosableDialog = !!onClose;
 
   const closeHandler = useCallback(() => {
     if (TransitionComponent) {
@@ -94,7 +93,7 @@ const Dialog = function Dialog({
       // called by that component, once the "out" transition has finished
       setTransitionComponentVisible(false);
     } else {
-      onClose();
+      onClose?.();
     }
   }, [onClose, TransitionComponent]);
 
@@ -127,7 +126,7 @@ const Dialog = function Dialog({
     if (direction === 'in') {
       initializeDialog();
     } else {
-      onClose();
+      onClose?.();
     }
   };
 
@@ -216,7 +215,7 @@ const Dialog = function Dialog({
           buttons={buttons}
           fullWidthHeader={true}
           icon={icon}
-          onClose={closeHandler}
+          onClose={isClosableDialog ? closeHandler : undefined}
           paddingSize={paddingSize}
           title={title}
           scrollable={scrollable}
diff --git a/src/components/feedback/test/Dialog-test.js b/src/components/feedback/test/Dialog-test.js
index 0a376eb3..4fa585b5 100644
--- a/src/components/feedback/test/Dialog-test.js
+++ b/src/components/feedback/test/Dialog-test.js
@@ -282,6 +282,13 @@ describe('Dialog', () => {
         assert.called(onClose);
       });
     });
+
+    context('when no onClose callback is provided', () => {
+      it('does not render a close button', () => {
+        const wrapper = mount(<Dialog title="My dialog" />);
+        assert.isFalse(wrapper.find('CancelIcon').exists());
+      });
+    });
   });
 
   describe('aria-describedby', () => {