From 975e81dba3a00bdc7dae7efb0024745624dfb8e7 Mon Sep 17 00:00:00 2001
From: chiba_kento <chiba_kento@cyberagent.co.jp>
Date: Thu, 7 Dec 2023 19:34:06 +0900
Subject: [PATCH] fix(spindle-ui): esc handler in DropdownMenu

The escape handler should be attached only when the drop menu is opened.
---
 .../src/DropdownMenu/DropdownMenu.test.tsx    | 137 +++++++++++++++++-
 .../src/DropdownMenu/DropdownMenu.tsx         |  31 ++--
 2 files changed, 151 insertions(+), 17 deletions(-)

diff --git a/packages/spindle-ui/src/DropdownMenu/DropdownMenu.test.tsx b/packages/spindle-ui/src/DropdownMenu/DropdownMenu.test.tsx
index 24dcf19ce..108486407 100644
--- a/packages/spindle-ui/src/DropdownMenu/DropdownMenu.test.tsx
+++ b/packages/spindle-ui/src/DropdownMenu/DropdownMenu.test.tsx
@@ -8,8 +8,8 @@ import { Button } from '../Button';
 import { BLOCK_NAME, DropdownMenu } from './DropdownMenu';
 
 const ANIMATION_DURATION = 300;
-const useDropdownMenuOpen = () => {
-  const [open, setOpen] = useState(true);
+const useDropdownMenuOpen = (initialState?: boolean) => {
+  const [open, setOpen] = useState(initialState ?? true);
   const onClick = () => {
     setOpen((prevOpen) => !prevOpen);
   };
@@ -227,4 +227,137 @@ describe('<DropdownMenu />', () => {
       screen.getByText(/triggerButton/).nextElementSibling,
     ).not.toBeInTheDocument();
   });
+
+  test('The DropdownMenu should be closed when you trigger esc key.', async () => {
+    const triggerRef = createRef<HTMLButtonElement>();
+    const onMenuButtonClick = jest.fn();
+    const { result } = renderHook(() => useDropdownMenuOpen());
+    const user = userEvent.setup({ advanceTimers: jest.advanceTimersByTime });
+
+    expect(result.current.open).toBe(true);
+
+    const { rerender } = render(
+      <>
+        <DropdownMenu.Frame>
+          <Button onClick={result.current.onClick} ref={triggerRef}>
+            triggerButton
+          </Button>
+          <DropdownMenu.List
+            onClose={result.current.onClose}
+            open={result.current.open}
+            triggerRef={triggerRef}
+          >
+            <DropdownMenu.ListItem onClick={onMenuButtonClick}>
+              <DropdownMenu.Title>testTitle</DropdownMenu.Title>
+            </DropdownMenu.ListItem>
+          </DropdownMenu.List>
+        </DropdownMenu.Frame>
+      </>,
+    );
+
+    const menu = screen.getByText(/triggerButton/).nextElementSibling;
+    expect(menu).toHaveClass(`${BLOCK_NAME}-menu`);
+    expect(menu).toBeInTheDocument();
+
+    await user.keyboard('{esc}');
+
+    expect(menu).toHaveClass('is-fade-out');
+
+    act(() => {
+      jest.advanceTimersByTime(ANIMATION_DURATION);
+      if (menu) fireEvent.animationEnd(menu);
+    });
+
+    rerender(
+      <>
+        <DropdownMenu.Frame>
+          <Button onClick={result.current.onClick} ref={triggerRef}>
+            triggerButton
+          </Button>
+          <DropdownMenu.List
+            onClose={result.current.onClose}
+            open={result.current.open}
+            triggerRef={triggerRef}
+          >
+            <DropdownMenu.ListItem onClick={onMenuButtonClick}>
+              <DropdownMenu.Title>testTitle</DropdownMenu.Title>
+            </DropdownMenu.ListItem>
+          </DropdownMenu.List>
+        </DropdownMenu.Frame>
+      </>,
+    );
+
+    expect(result.current.open).toBe(false);
+    expect(
+      screen.getByText(/triggerButton/).nextElementSibling,
+    ).not.toBeInTheDocument();
+  });
+
+  test('The active element should be trigger element after the escape keydown when the Dropdown menu is opened.', async () => {
+    const triggerRef = createRef<HTMLButtonElement>();
+    const onMenuButtonClick = jest.fn();
+    const { result } = renderHook(() => useDropdownMenuOpen());
+    const user = userEvent.setup({ advanceTimers: jest.advanceTimersByTime });
+
+    expect(result.current.open).toBe(true);
+
+    render(
+      <>
+        <DropdownMenu.Frame>
+          <Button onClick={result.current.onClick} ref={triggerRef}>
+            triggerButton
+          </Button>
+          <DropdownMenu.List
+            onClose={result.current.onClose}
+            open={result.current.open}
+            triggerRef={triggerRef}
+          >
+            <DropdownMenu.ListItem onClick={onMenuButtonClick}>
+              <DropdownMenu.Title>testTitle</DropdownMenu.Title>
+            </DropdownMenu.ListItem>
+          </DropdownMenu.List>
+        </DropdownMenu.Frame>
+      </>,
+    );
+
+    await user.keyboard('{esc}');
+    act(() => {
+      jest.advanceTimersByTime(ANIMATION_DURATION);
+    });
+    expect(document.activeElement).toBe(triggerRef.current);
+  });
+
+  test('The active element should not be trigger element after the escape keydown when the Dropdown menu is closed.', async () => {
+    const triggerRef = createRef<HTMLButtonElement>();
+    const onMenuButtonClick = jest.fn();
+    const { result } = renderHook(() => useDropdownMenuOpen(false));
+    const user = userEvent.setup({ advanceTimers: jest.advanceTimersByTime });
+
+    expect(result.current.open).toBe(false);
+
+    render(
+      <>
+        <DropdownMenu.Frame>
+          <Button onClick={result.current.onClick} ref={triggerRef}>
+            triggerButton
+          </Button>
+          <DropdownMenu.List
+            onClose={result.current.onClose}
+            open={result.current.open}
+            triggerRef={triggerRef}
+          >
+            <DropdownMenu.ListItem onClick={onMenuButtonClick}>
+              <DropdownMenu.Title>testTitle</DropdownMenu.Title>
+            </DropdownMenu.ListItem>
+          </DropdownMenu.List>
+        </DropdownMenu.Frame>
+      </>,
+    );
+
+    await user.keyboard('{esc}');
+    act(() => {
+      jest.advanceTimersByTime(ANIMATION_DURATION);
+    });
+    expect(document.activeElement).not.toBe(triggerRef.current);
+  });
 });
diff --git a/packages/spindle-ui/src/DropdownMenu/DropdownMenu.tsx b/packages/spindle-ui/src/DropdownMenu/DropdownMenu.tsx
index f1988645d..d167d0e08 100644
--- a/packages/spindle-ui/src/DropdownMenu/DropdownMenu.tsx
+++ b/packages/spindle-ui/src/DropdownMenu/DropdownMenu.tsx
@@ -40,7 +40,7 @@ interface ListProps extends DefaultProps {
 
 export const BLOCK_NAME = 'spui-DropdownMenu';
 const FADE_IN_ANIMATION = 'spui-DropdownMenu-fade-in';
-const CLOSE_KEY_LIST = ['Escape', 'Esc'];
+const CLOSE_KEY_LIST = ['ESCAPE', 'ESC'];
 const MENU_WIDTH = 256;
 
 const Caption = ({ children }: DefaultProps) => {
@@ -86,7 +86,7 @@ const List = ({
 
   const handleKeyDown = useCallback(
     (e: KeyboardEvent) => {
-      if (CLOSE_KEY_LIST.includes(e.key)) {
+      if (CLOSE_KEY_LIST.includes(e.key.toUpperCase())) {
         onClickCloser();
       }
     },
@@ -123,27 +123,28 @@ const List = ({
 
   useEffect(() => {
     const menu = menuEl.current;
-    menu?.addEventListener('animationend', handleAnimationEnd, false);
+    if (open) {
+      menu?.addEventListener('animationend', handleAnimationEnd, false);
+    }
 
     return () =>
       menu?.removeEventListener('animationend', handleAnimationEnd, false);
-  }, [menuEl, handleAnimationEnd]);
+  }, [menuEl, handleAnimationEnd, open]);
 
   useEffect(() => {
-    window.addEventListener('click', onClickBody, false);
+    if (open) {
+      window.addEventListener('click', onClickBody, false);
+    }
 
-    return () => {
-      window.removeEventListener('click', onClickBody, false);
-    };
-  }, [onClickBody]);
+    return () => window.removeEventListener('click', onClickBody, false);
+  }, [onClickBody, open]);
 
   useEffect(() => {
-    window.addEventListener('keydown', handleKeyDown);
-
-    return () => {
-      window.removeEventListener('keydown', handleKeyDown);
-    };
-  }, [handleKeyDown]);
+    if (open) {
+      window.addEventListener('keydown', handleKeyDown);
+    }
+    return () => window.removeEventListener('keydown', handleKeyDown);
+  }, [handleKeyDown, open]);
 
   if (!open) {
     return <></>;