From 8166d7dc4d70b45ab3a76ba25e1d83a380d5d0f7 Mon Sep 17 00:00:00 2001
From: Ashley Harrison <ashley.harrison@grafana.com>
Date: Tue, 24 May 2022 15:34:47 +0100
Subject: [PATCH] Fix escape in Modal/DashboardSettings + add some unit tests
 (#49500)

---
 .../src/components/Drawer/Drawer.tsx          |  2 +
 .../src/components/Modal/Modal.test.tsx       | 19 +++++++
 .../grafana-ui/src/components/Modal/Modal.tsx |  2 +-
 .../DashboardSettings.test.tsx                | 51 +++++++++++++++++++
 .../DashboardSettings/DashboardSettings.tsx   |  8 ++-
 5 files changed, 80 insertions(+), 2 deletions(-)
 create mode 100644 public/app/features/dashboard/components/DashboardSettings/DashboardSettings.test.tsx

diff --git a/packages/grafana-ui/src/components/Drawer/Drawer.tsx b/packages/grafana-ui/src/components/Drawer/Drawer.tsx
index 7bf729d0334..55b61dc0cd5 100644
--- a/packages/grafana-ui/src/components/Drawer/Drawer.tsx
+++ b/packages/grafana-ui/src/components/Drawer/Drawer.tsx
@@ -55,6 +55,8 @@ export function Drawer({
   const { overlayProps } = useOverlay(
     {
       isDismissable: true,
+      isOpen,
+      onClose,
     },
     overlayRef
   );
diff --git a/packages/grafana-ui/src/components/Modal/Modal.test.tsx b/packages/grafana-ui/src/components/Modal/Modal.test.tsx
index 08d558d395b..7f4a79d8480 100644
--- a/packages/grafana-ui/src/components/Modal/Modal.test.tsx
+++ b/packages/grafana-ui/src/components/Modal/Modal.test.tsx
@@ -1,4 +1,5 @@
 import { render, screen } from '@testing-library/react';
+import userEvent from '@testing-library/user-event';
 import React from 'react';
 
 import { Modal } from './Modal';
@@ -22,4 +23,22 @@ describe('Modal', () => {
 
     expect(screen.getByTestId('modal-content')).toBeInTheDocument();
   });
+
+  it('pressing escape calls onDismiss correctly', async () => {
+    const onDismiss = jest.fn();
+
+    render(
+      <Modal title="Some Title" isOpen onDismiss={onDismiss}>
+        <div data-testid="modal-content">Content</div>
+      </Modal>
+    );
+
+    expect(screen.getByRole('dialog')).toBeInTheDocument();
+    expect(screen.getByLabelText('Some Title')).toBeInTheDocument();
+    expect(screen.getByTestId('modal-content')).toBeInTheDocument();
+
+    await userEvent.keyboard('{Escape}');
+
+    expect(onDismiss).toHaveBeenCalled();
+  });
 });
diff --git a/packages/grafana-ui/src/components/Modal/Modal.tsx b/packages/grafana-ui/src/components/Modal/Modal.tsx
index 8b2e6adc175..a3f6aeba5df 100644
--- a/packages/grafana-ui/src/components/Modal/Modal.tsx
+++ b/packages/grafana-ui/src/components/Modal/Modal.tsx
@@ -53,7 +53,7 @@ export function Modal(props: PropsWithChildren<Props>) {
   // Handle interacting outside the dialog and pressing
   // the Escape key to close the modal.
   const { overlayProps, underlayProps } = useOverlay(
-    { isKeyboardDismissDisabled: closeOnEscape, isOpen, onClose: onDismiss },
+    { isKeyboardDismissDisabled: !closeOnEscape, isOpen, onClose: onDismiss },
     ref
   );
 
diff --git a/public/app/features/dashboard/components/DashboardSettings/DashboardSettings.test.tsx b/public/app/features/dashboard/components/DashboardSettings/DashboardSettings.test.tsx
new file mode 100644
index 00000000000..a694be7c3f6
--- /dev/null
+++ b/public/app/features/dashboard/components/DashboardSettings/DashboardSettings.test.tsx
@@ -0,0 +1,51 @@
+import { render, screen } from '@testing-library/react';
+import userEvent from '@testing-library/user-event';
+import React from 'react';
+import { Provider } from 'react-redux';
+import { BrowserRouter } from 'react-router-dom';
+
+import { locationService, setBackendSrv } from '@grafana/runtime';
+import { configureStore } from 'app/store/configureStore';
+
+import { DashboardModel } from '../../state';
+
+import { DashboardSettings } from './DashboardSettings';
+
+jest.mock('@grafana/runtime', () => ({
+  ...jest.requireActual('@grafana/runtime'),
+  locationService: {
+    partial: jest.fn(),
+  },
+}));
+
+setBackendSrv({
+  get: jest.fn().mockResolvedValue({}),
+} as any);
+
+describe('DashboardSettings', () => {
+  it('pressing escape navigates away correctly', async () => {
+    jest.spyOn(locationService, 'partial');
+    const dashboard = new DashboardModel(
+      {
+        title: 'Foo',
+      },
+      {
+        folderId: 1,
+      }
+    );
+    const store = configureStore();
+    render(
+      <Provider store={store}>
+        <BrowserRouter>
+          <DashboardSettings editview="settings" dashboard={dashboard} />
+        </BrowserRouter>
+      </Provider>
+    );
+
+    expect(screen.getByText('Foo / Settings')).toBeInTheDocument();
+
+    await userEvent.keyboard('{Escape}');
+
+    expect(locationService.partial).toHaveBeenCalledWith({ editview: null });
+  });
+});
diff --git a/public/app/features/dashboard/components/DashboardSettings/DashboardSettings.tsx b/public/app/features/dashboard/components/DashboardSettings/DashboardSettings.tsx
index d8494fe6c59..363eae8acc6 100644
--- a/public/app/features/dashboard/components/DashboardSettings/DashboardSettings.tsx
+++ b/public/app/features/dashboard/components/DashboardSettings/DashboardSettings.tsx
@@ -49,7 +49,13 @@ const MakeEditable = (props: { onMakeEditable: () => any }) => (
 
 export function DashboardSettings({ dashboard, editview }: Props) {
   const ref = useRef<HTMLDivElement>(null);
-  const { overlayProps } = useOverlay({}, ref);
+  const { overlayProps } = useOverlay(
+    {
+      isOpen: true,
+      onClose,
+    },
+    ref
+  );
   const { dialogProps } = useDialog(
     {
       'aria-label': 'Dashboard settings',