From 9bdb5fcf610cd0d35d5fe979570015d600f00089 Mon Sep 17 00:00:00 2001
From: Daniel Dipper <daniel.dipper@sage.com>
Date: Thu, 3 Feb 2022 11:27:45 +0000
Subject: [PATCH] fix(action-popover-item): prevent popover staying open when
 using keyboard nav

Fixes a bug whereby the popover would stay open when a dialog was opened from Action Popover using
keyboard navigation.

fixes #4662
---
 .../action-popover-item.component.js          |   9 +-
 .../action-popover-test.stories.mdx           |   1 +
 .../action-popover/action-popover.spec.js     |  31 ++--
 .../action-popover/action-popover.stories.mdx | 160 +++++++++++++++++-
 .../action-popover/action-popover.style.js    |   3 +-
 5 files changed, 188 insertions(+), 16 deletions(-)

diff --git a/src/components/action-popover/action-popover-item/action-popover-item.component.js b/src/components/action-popover/action-popover-item/action-popover-item.component.js
index fe99b29812..739f779bab 100644
--- a/src/components/action-popover/action-popover-item/action-popover-item.component.js
+++ b/src/components/action-popover/action-popover-item/action-popover-item.component.js
@@ -95,11 +95,11 @@ const MenuItem = ({
     (e) => {
       e.stopPropagation();
       if (!disabled) {
+        setOpenPopover(false);
+        focusButton();
         if (onClickProp) {
           onClickProp();
         }
-        setOpenPopover(false);
-        focusButton();
       } else {
         ref.current.focus();
         e.preventDefault();
@@ -145,7 +145,10 @@ const MenuItem = ({
           }
           e.preventDefault();
         } else if (Events.isEnterKey(e)) {
-          if (isHref && download) ref.current.click();
+          if (isHref && download) {
+            ref.current.click();
+          }
+          e.preventDefault();
           onClick(e);
         }
       } else if (Events.isEnterKey(e)) {
diff --git a/src/components/action-popover/action-popover-test.stories.mdx b/src/components/action-popover/action-popover-test.stories.mdx
index 25953ca037..005e246c10 100644
--- a/src/components/action-popover/action-popover-test.stories.mdx
+++ b/src/components/action-popover/action-popover-test.stories.mdx
@@ -1,3 +1,4 @@
+import { useState } from "react";
 import { Meta, Story, Canvas } from "@storybook/addon-docs";
 import { action } from "@storybook/addon-actions";
 
diff --git a/src/components/action-popover/action-popover.spec.js b/src/components/action-popover/action-popover.spec.js
index 2e876380b1..a783764e5d 100644
--- a/src/components/action-popover/action-popover.spec.js
+++ b/src/components/action-popover/action-popover.spec.js
@@ -259,7 +259,7 @@ describe("ActionPopover", () => {
         wrapper
           .find(MenuButton)
           .props()
-          .onClick({ stopPropagation: () => {} });
+          .onClick({ stopPropagation: () => {}, preventDefault: () => {} });
       });
 
       act(() => {
@@ -267,10 +267,11 @@ describe("ActionPopover", () => {
       });
 
       act(() => {
-        wrapper
-          .find(StyledMenuItem)
-          .props()
-          .onKeyDown({ which: 13, stopPropagation: jest.fn() });
+        wrapper.find(StyledMenuItem).props().onKeyDown({
+          which: 13,
+          stopPropagation: jest.fn(),
+          preventDefault: jest.fn(),
+        });
       });
 
       act(() => {
@@ -976,16 +977,24 @@ describe("ActionPopover", () => {
         ).find(ActionPopoverItem);
 
         expect(item.props().submenu).toBeTruthy();
-        expect(item.find("div").at(0).props().onMouseEnter).toEqual(undefined);
-        expect(item.find("div").at(0).props().onMouseLeave).toEqual(undefined);
-        expect(item.find("div").at(0).props()["aria-haspopup"]).toEqual("true");
-        expect(item.find("div").at(0).props()["aria-label"]).not.toEqual(
+        expect(item.find("button").at(0).props().onMouseEnter).toEqual(
+          undefined
+        );
+        expect(item.find("button").at(0).props().onMouseLeave).toEqual(
           undefined
         );
-        expect(item.find("div").at(0).props()["aria-controls"]).not.toEqual(
+        expect(item.find("button").at(0).props()["aria-haspopup"]).toEqual(
+          "true"
+        );
+        expect(item.find("button").at(0).props()["aria-label"]).not.toEqual(
           undefined
         );
-        expect(item.find("div").at(0).props()["aria-expanded"]).toEqual(false);
+        expect(item.find("button").at(0).props()["aria-controls"]).not.toEqual(
+          undefined
+        );
+        expect(item.find("button").at(0).props()["aria-expanded"]).toEqual(
+          false
+        );
       });
 
       it("updates the focus when an item with a submenu is clicked and does not close the menu", () => {
diff --git a/src/components/action-popover/action-popover.stories.mdx b/src/components/action-popover/action-popover.stories.mdx
index 164281da18..cfc0c6038e 100644
--- a/src/components/action-popover/action-popover.stories.mdx
+++ b/src/components/action-popover/action-popover.stories.mdx
@@ -1,5 +1,6 @@
 import { useState } from "react";
 import { Meta, Story, Canvas, ArgsTable } from "@storybook/addon-docs";
+import { action } from "@storybook/addon-actions";
 import { Preview as MyPreview } from "@storybook/components";
 import LinkTo from "@storybook/addon-links/react";
 import TranslationKeysTable from "../../../.storybook/utils/translation-keys-table";
@@ -24,6 +25,7 @@ import {
   FlatTableHeader,
   FlatTableCell,
 } from "../flat-table";
+import Confirm from "../confirm";
 
 <Meta
   title="Action Popover"
@@ -87,7 +89,100 @@ import {
 Story showing most of the functionality of the action popover and what can be achieved by using it
 
 <Canvas>
-  <Story id="action-popover-test--default" />
+  <Story name="default">
+    {()=> {
+      const submenu = (
+        <ActionPopoverMenu>
+          <ActionPopoverItem onClick={action("sub menu 1")}>
+            Sub Menu 1
+          </ActionPopoverItem>
+          <ActionPopoverItem onClick={action("sub menu 2")}>
+            Sub Menu 2
+          </ActionPopoverItem>
+          <ActionPopoverItem disabled onClick={action("sub menu 3")}>
+            Sub Menu 3
+          </ActionPopoverItem>
+        </ActionPopoverMenu>
+      );
+      const submenuWithIcons = (
+        <ActionPopoverMenu>
+          <ActionPopoverItem icon="graph" onClick={action("sub menu 1")}>
+            Sub Menu 1
+          </ActionPopoverItem>
+          <ActionPopoverItem icon="add" onClick={action("sub menu 2")}>
+            Sub Menu 2
+          </ActionPopoverItem>
+          <ActionPopoverItem icon="print" disabled onClick={action("sub menu 3")}>
+            Sub Menu 3
+          </ActionPopoverItem>
+        </ActionPopoverMenu>
+      );
+      return (
+        <div style={{ marginTop: "40px", height: "275px"}}>
+          <FlatTable isZebra>
+            <FlatTableHead>
+              <FlatTableRow>
+                <FlatTableHeader>First Name</FlatTableHeader>
+                <FlatTableHeader>Last Name</FlatTableHeader>
+                <FlatTableHeader>&nbsp;</FlatTableHeader>
+              </FlatTableRow>
+            </FlatTableHead>
+            <FlatTableBody>
+              <FlatTableRow>
+                <FlatTableCell>John</FlatTableCell>
+                <FlatTableCell>Doe</FlatTableCell>
+                <FlatTableCell>
+                  <ActionPopover onOpen={action("popover opened")} onClose={action("popover closed")}>
+                    <ActionPopoverItem disabled icon="graph" submenu={submenu} onClick={action("email")}>
+                      Business
+                    </ActionPopoverItem>
+                    <ActionPopoverItem icon="email" onClick={action("email")}>
+                      Email Invoice
+                    </ActionPopoverItem>
+                    <ActionPopoverItem icon="print" onClick={action("print")} submenu={submenu}>
+                      Print Invoice
+                    </ActionPopoverItem>
+                    <ActionPopoverItem icon="pdf" submenu={submenu} onClick={action("pdf")}>
+                      Download PDF
+                    </ActionPopoverItem>
+                    <ActionPopoverItem icon="csv" onClick={action("csv")}>
+                      Download CSV
+                    </ActionPopoverItem>
+                    <ActionPopoverDivider />
+                    <ActionPopoverItem icon="delete" onClick={action("delete")}>
+                      Delete
+                    </ActionPopoverItem>
+                  </ActionPopover>
+                </FlatTableCell>
+              </FlatTableRow>
+              <FlatTableRow>
+                <FlatTableCell>Jane</FlatTableCell>
+                <FlatTableCell>Smith</FlatTableCell>
+                <FlatTableCell>
+                  <ActionPopover>
+                    <ActionPopoverItem icon="csv" onClick={action("csv")}>
+                      Download CSV
+                    </ActionPopoverItem>
+                  </ActionPopover>
+                </FlatTableCell>
+              </FlatTableRow>
+              <FlatTableRow>
+                <FlatTableCell>Bob</FlatTableCell>
+                <FlatTableCell>Jones</FlatTableCell>
+                <FlatTableCell>
+                  <ActionPopover>
+                    <ActionPopoverItem icon="csv" submenu={submenuWithIcons} onClick={action("csv")}>
+                      Download CSV
+                    </ActionPopoverItem>
+                  </ActionPopover>
+                </FlatTableCell>
+              </FlatTableRow>
+            </FlatTableBody>
+          </FlatTable>
+        </div>
+      );
+    }}
+  </Story>
 </Canvas>
 
 ### With icons
@@ -1023,6 +1118,69 @@ It is possible to utilise the selected and onClick props on the FlatTableRow to
   </Story>
 </Canvas>
 
+## Action opening a Modal
+
+<Canvas>
+  <Story name="opening a modal">
+    {() => {
+    const [isConfirmOpen, setIsConfirmOpen] = useState(false);
+    const [isOpen, setIsOpen] = useState(false);
+    return (
+      <>
+      <FlatTable colorTheme="transparent-white">
+        <FlatTableHead>
+          <FlatTableRow>
+            <FlatTableHeader>Devices</FlatTableHeader>
+            <FlatTableHeader>Status</FlatTableHeader>
+            <FlatTableHeader>Action Popover</FlatTableHeader>
+          </FlatTableRow>
+        </FlatTableHead>
+        <FlatTableBody>
+          <FlatTableRow>
+            <FlatTableCell>Mobile Phone</FlatTableCell>
+            <FlatTableCell>Online</FlatTableCell>
+            <FlatTableCell>
+              <ActionPopover
+                renderButton={(props) => (
+                  <ActionPopoverMenuButton isOpen={isOpen} {...props}>
+                    Open Actions
+                  </ActionPopoverMenuButton>
+                )}
+              >
+                <ActionPopoverItem
+                  onClick={() => {
+                    setIsOpen(false);
+                    setIsConfirmOpen(true);
+                  }}
+                >
+                  Open Confirm Dialog
+                </ActionPopoverItem>
+                <ActionPopoverItem icon="settings" onClick={() => {}}>
+                  Do Nothing
+                </ActionPopoverItem>
+              </ActionPopover>
+            </FlatTableCell>
+          </FlatTableRow>
+        </FlatTableBody>
+      </FlatTable>
+      <Confirm
+        title="Are you sure?"
+        subtitle="Subtitle"
+        confirmButtonDestructive
+        cancelButtonDestructive
+        disableConfirm
+        open={isConfirmOpen}
+        onConfirm={() => setIsConfirmOpen(false)}
+        onCancel={() => setIsConfirmOpen(false)}
+      >
+        Content
+      </Confirm>
+    </>
+    )
+  }}
+  </Story>
+</Canvas>
+
 ## Props
 
 ### ActionPopover Props
diff --git a/src/components/action-popover/action-popover.style.js b/src/components/action-popover/action-popover.style.js
index abf77becfc..cd9f535da7 100644
--- a/src/components/action-popover/action-popover.style.js
+++ b/src/components/action-popover/action-popover.style.js
@@ -17,8 +17,9 @@ const Menu = styled.div`
     `${theme.zIndex.popover}`}; // TODO (tokens): implement elevation tokens - FE-4437
 `;
 
-const StyledMenuItem = styled.div`
+const StyledMenuItem = styled.button`
   text-decoration: none;
+  background-color: var(--colorsActionMajorYang100);
 `;
 
 const MenuItemFactory = (button) => styled(button)`