From 63049234a31b124775a3b6b67d6113cf6d9d0cb6 Mon Sep 17 00:00:00 2001
From: Siddharth Kshetrapal <siddharthkp@github.com>
Date: Wed, 31 May 2023 16:37:51 +0200
Subject: [PATCH] fix: prevent closing menu when `event.preventDefault()` is
 called on `ActionList.Item`'s `onSelect` handler by @gr2m (#3346)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

* fix: prevent closing menu when `event.preventDefault()` is called on `ActionList.Item`'s `onSelect` handler (#3163)

* test: prevent closing menu when `event.preventDefault()` is called on `ActionList.Item`'s `onSelect` handler

Failing test for https://github.com/primer/react/issues/3162

* fix: prevent closing menu when `event.preventDefault()` is called on `ActionList.Item`ߴs `onSelect` handler

* add storybook example: Delayed Menu Close

* update docs

* docs: changeset

* Update changelog

---------

Co-authored-by: Siddharth Kshetrapal <siddharthkp@github.com>

* Update generated/components.json

---------

Co-authored-by: Gregor Martynus <39992+gr2m@users.noreply.github.com>
Co-authored-by: siddharthkp <siddharthkp@users.noreply.github.com>
---
 .changeset/brave-cherries-march.md            |  5 +++
 generated/components.json                     |  2 +-
 src/ActionList/ActionList.docs.json           |  2 +-
 src/ActionList/Item.tsx                       | 27 +++++++++------
 .../ActionMenu.examples.stories.tsx           | 34 +++++++++++++++++++
 src/__tests__/ActionMenu.test.tsx             |  2 +-
 6 files changed, 59 insertions(+), 13 deletions(-)
 create mode 100644 .changeset/brave-cherries-march.md

diff --git a/.changeset/brave-cherries-march.md b/.changeset/brave-cherries-march.md
new file mode 100644
index 00000000000..d347b7a9a3b
--- /dev/null
+++ b/.changeset/brave-cherries-march.md
@@ -0,0 +1,5 @@
+---
+"@primer/react": minor
+---
+
+ActionMenu: Calling `event.preventDefault` inside `onSelect` of `ActionList.Item` will prevent the default behavior of closing the menu
diff --git a/generated/components.json b/generated/components.json
index 8964f1d7935..4df9e9e8dcc 100644
--- a/generated/components.json
+++ b/generated/components.json
@@ -163,7 +163,7 @@
               "name": "onSelect",
               "type": "(event: React.MouseEvent<HTMLLIElement> | React.KeyboardEvent<HTMLLIElement>) => void",
               "defaultValue": "",
-              "description": "Callback that is called when the item is selected using either the mouse or keyboard."
+              "description": "Callback that is called when the item is selected using either the mouse or keyboard. `event.preventDefault()` will prevent a menu from closing when within an `<ActionMenu />`"
             },
             {
               "name": "selected",
diff --git a/src/ActionList/ActionList.docs.json b/src/ActionList/ActionList.docs.json
index d74f3a0290a..18610f7f622 100644
--- a/src/ActionList/ActionList.docs.json
+++ b/src/ActionList/ActionList.docs.json
@@ -62,7 +62,7 @@
           "name": "onSelect",
           "type": "(event: React.MouseEvent<HTMLLIElement> | React.KeyboardEvent<HTMLLIElement>) => void",
           "defaultValue": "",
-          "description": "Callback that is called when the item is selected using either the mouse or keyboard."
+          "description": "Callback that is called when the item is selected using either the mouse or keyboard. `event.preventDefault()` will prevent a menu from closing when within an `<ActionMenu />`"
         },
         {
           "name": "selected",
diff --git a/src/ActionList/Item.tsx b/src/ActionList/Item.tsx
index 8064ff2f1c1..56e91d0070c 100644
--- a/src/ActionList/Item.tsx
+++ b/src/ActionList/Item.tsx
@@ -24,7 +24,7 @@ export const Item = React.forwardRef<HTMLLIElement, ActionListItemProps>(
       disabled = false,
       selected = undefined,
       active = false,
-      onSelect,
+      onSelect: onSelectUser,
       sx: sxProp = defaultSxProp,
       id,
       role,
@@ -42,6 +42,19 @@ export const Item = React.forwardRef<HTMLLIElement, ActionListItemProps>(
     const {container, afterSelect, selectionAttribute} = React.useContext(ActionListContainerContext)
     const {selectionVariant: groupSelectionVariant} = React.useContext(GroupContext)
 
+    const onSelect = React.useCallback(
+      (
+        event: React.MouseEvent<HTMLLIElement> | React.KeyboardEvent<HTMLLIElement>,
+        // eslint-disable-next-line @typescript-eslint/ban-types
+        afterSelect?: Function,
+      ) => {
+        if (typeof onSelectUser === 'function') onSelectUser(event)
+        if (event.defaultPrevented) return
+        if (typeof afterSelect === 'function') afterSelect()
+      },
+      [onSelectUser],
+    )
+
     const selectionVariant: ActionListProps['selectionVariant'] = groupSelectionVariant
       ? groupSelectionVariant
       : listSelectionVariant
@@ -149,11 +162,7 @@ export const Item = React.forwardRef<HTMLLIElement, ActionListItemProps>(
     const clickHandler = React.useCallback(
       (event: React.MouseEvent<HTMLLIElement>) => {
         if (disabled) return
-        if (!event.defaultPrevented) {
-          if (typeof onSelect === 'function') onSelect(event)
-          // if this Item is inside a Menu, close the Menu
-          if (typeof afterSelect === 'function') afterSelect()
-        }
+        onSelect(event, afterSelect)
       },
       [onSelect, disabled, afterSelect],
     )
@@ -161,10 +170,8 @@ export const Item = React.forwardRef<HTMLLIElement, ActionListItemProps>(
     const keyPressHandler = React.useCallback(
       (event: React.KeyboardEvent<HTMLLIElement>) => {
         if (disabled) return
-        if (!event.defaultPrevented && [' ', 'Enter'].includes(event.key)) {
-          if (typeof onSelect === 'function') onSelect(event)
-          // if this Item is inside a Menu, close the Menu
-          if (typeof afterSelect === 'function') afterSelect()
+        if ([' ', 'Enter'].includes(event.key)) {
+          onSelect(event, afterSelect)
         }
       },
       [onSelect, disabled, afterSelect],
diff --git a/src/ActionMenu/ActionMenu.examples.stories.tsx b/src/ActionMenu/ActionMenu.examples.stories.tsx
index 88fec6fa0ec..308d2d2d26f 100644
--- a/src/ActionMenu/ActionMenu.examples.stories.tsx
+++ b/src/ActionMenu/ActionMenu.examples.stories.tsx
@@ -11,6 +11,8 @@ import {
   NumberIcon,
   CalendarIcon,
   XIcon,
+  CheckIcon,
+  CopyIcon,
 } from '@primer/octicons-react'
 
 export default {
@@ -282,3 +284,35 @@ export const MultipleSections = () => {
     </ActionMenu>
   )
 }
+
+export const DelayedMenuClose = () => {
+  const [open, setOpen] = React.useState(false)
+  const [copyLinkSuccess, setCopyLinkSuccess] = React.useState(false)
+  const onSelect = (event: React.MouseEvent<HTMLLIElement> | React.KeyboardEvent<HTMLLIElement>) => {
+    event.preventDefault()
+
+    setCopyLinkSuccess(true)
+    setTimeout(() => {
+      setOpen(false)
+      setCopyLinkSuccess(false)
+    }, 700)
+  }
+
+  return (
+    <>
+      <h1>Delayed Menu Close</h1>
+
+      <ActionMenu open={open} onOpenChange={setOpen}>
+        <ActionMenu.Button>Anchor</ActionMenu.Button>
+        <ActionMenu.Overlay>
+          <ActionList>
+            <ActionList.Item onSelect={onSelect}>
+              <ActionList.LeadingVisual>{copyLinkSuccess ? <CheckIcon /> : <CopyIcon />}</ActionList.LeadingVisual>
+              {copyLinkSuccess ? 'Copied!' : 'Copy link'}
+            </ActionList.Item>
+          </ActionList>
+        </ActionMenu.Overlay>
+      </ActionMenu>
+    </>
+  )
+}
diff --git a/src/__tests__/ActionMenu.test.tsx b/src/__tests__/ActionMenu.test.tsx
index f2b41c86df1..bf33d15c491 100644
--- a/src/__tests__/ActionMenu.test.tsx
+++ b/src/__tests__/ActionMenu.test.tsx
@@ -22,7 +22,7 @@ function Example(): JSX.Element {
                 <ActionList.Divider />
                 <ActionList.Item>Copy link</ActionList.Item>
                 <ActionList.Item>Edit file</ActionList.Item>
-                <ActionList.Item variant="danger" onClick={event => event.preventDefault()}>
+                <ActionList.Item variant="danger" onSelect={event => event.preventDefault()}>
                   Delete file
                 </ActionList.Item>
                 <ActionList.LinkItem href="//github.com" title="anchor" aria-keyshortcuts="s">