Merge pull request #5347 from Sage/FE-5282-popover-container-click-in…

…side-portal

fix(popover-container): ensure clicks inside popover do not close it

cypress/e2e/common/popoverContainer.feature

@@ -43,4 +43,12 @@ Feature: Popover container component
     Examples:
       | key   |
       | Enter |
-      | Space |
+      | Space |
+
+  @positive
+  Scenario Outline: Select component inside Popover container does not close the Popover container when an option is selected
+    Given I open "Popover container Test" component page "with select"
+      And I open popover container
+      And I click on Select text
+    When I select the "green" Option
+    Then Popover container is visible

src/components/date/date.component.js

@@ -57,7 +57,6 @@ const DateInput = ({
   const wrapperRef = useRef();
   const parentRef = useRef();
   const inputRef = useRef();
-  const pickerRef = useRef();
   const alreadyFocused = useRef(false);
   const isBlurBlocked = useRef(false);
   const focusedViaPicker = useRef(false);
@@ -107,6 +106,19 @@ const DateInput = ({
     return ev;
   };
 
+  const handleClickAway = () => {
+    if (open) {
+      alreadyFocused.current = true;
+      inputRef.current.focus();
+      isBlurBlocked.current = false;
+      inputRef.current.blur();
+      setOpen(false);
+      alreadyFocused.current = false;
+    }
+  };
+
+  const handleClickInside = useClickAwayListener(handleClickAway, "mousedown");
+
   const handleChange = (ev) => {
     isInitialValue.current = false;
     onChange(buildCustomEvent(ev));
@@ -214,6 +226,8 @@ const DateInput = ({
   };
 
   const handleMouseDown = (ev) => {
+    handleClickInside(ev);
+
     if (disabled || readOnly) {
       return;
     }
@@ -235,8 +249,9 @@ const DateInput = ({
     handleMouseDown(e);
   };
 
-  const handlePickerMouseDown = () => {
+  const handlePickerMouseDown = (ev) => {
     isBlurBlocked.current = true;
+    handleClickInside(ev);
   };
 
   const assignInput = (input) => {
@@ -304,19 +319,6 @@ const DateInput = ({
     return value;
   };
 
-  const handleClickAway = () => {
-    if (open) {
-      alreadyFocused.current = true;
-      inputRef.current.focus();
-      isBlurBlocked.current = false;
-      inputRef.current.blur();
-      setOpen(false);
-      alreadyFocused.current = false;
-    }
-  };
-
-  useClickAwayListener([parentRef, pickerRef], handleClickAway, "mousedown");
-
   return (
     <StyledDateInput
       ref={wrapperRef}
@@ -359,7 +361,6 @@ const DateInput = ({
         onDayClick={handleDayClick}
         minDate={minDate}
         maxDate={maxDate}
-        ref={pickerRef}
         pickerMouseDown={handlePickerMouseDown}
         open={open}
       />

src/components/date/date.spec.js

@@ -957,7 +957,11 @@ function simulateMouseDownOnPicker(wrapper) {
   };
 
   act(() => {
-    input.simulate("mousedown", mockEvent);
+    input
+      .getDOMNode()
+      .dispatchEvent(
+        new MouseEvent("mousedown", { ...mockEvent, bubbles: true })
+      );
   });
 }
 

src/components/menu/__internal__/submenu/submenu.component.js

@@ -48,7 +48,6 @@ const Submenu = React.forwardRef(
     const [submenuOpen, setSubmenuOpen] = useState(false);
     const [submenuFocusIndex, setSubmenuFocusIndex] = useState(undefined);
     const [characterString, setCharacterString] = useState("");
-    const submenuRef = useRef();
     const formattedChildren = React.Children.map(children, (child) => {
       if (child.type === ScrollableBlock) {
         return [...child.props.children];
@@ -253,16 +252,16 @@ const Submenu = React.forwardRef(
       // eslint-disable-next-line react-hooks/exhaustive-deps
     }, [characterString]);
 
-    useClickAwayListener([submenuRef], handleClickAway);
+    const handleClickInside = useClickAwayListener(handleClickAway);
 
     if (inFullscreenView) {
       return (
         <StyledSubmenuWrapper
           data-component="submenu-wrapper"
-          ref={submenuRef}
           inFullscreenView={inFullscreenView}
           menuType={menuContext.menuType}
           asPassiveItem={asPassiveItem}
+          onClick={handleClickInside}
         >
           <StyledMenuItemWrapper
             {...rest}
@@ -308,8 +307,8 @@ const Submenu = React.forwardRef(
         data-component="submenu-wrapper"
         onMouseOver={!clickToOpen ? () => openSubmenu() : undefined}
         onMouseLeave={() => closeSubmenu()}
-        ref={submenuRef}
         isSubmenuOpen={submenuOpen}
+        onClick={handleClickInside}
       >
         <StyledMenuItemWrapper
           {...rest}

src/components/multi-action-button/__snapshots__/multi-action-button.spec.tsx.snap

@@ -361,6 +361,7 @@ exports[`MultiActionButton when rendered should match the snapshot 1`] = `
   aria-haspopup="true"
   className="c0"
   data-component="multi-action-button"
+  onClick={[Function]}
   onMouseLeave={[Function]}
 >
   <button

src/components/multi-action-button/multi-action-button.component.tsx

@@ -143,12 +143,13 @@ export const MultiActionButton = ({
     </Popover>
   );
 
-  useClickAwayListener([ref], hideButtons);
+  const handleClick = useClickAwayListener(hideButtons);
 
   return (
     <StyledMultiActionButton
       aria-haspopup="true"
       onMouseLeave={hideButtons}
+      onClick={handleClick}
       ref={ref}
       data-component="multi-action-button"
       data-element={dataElement}

src/components/popover-container/popover-container-test.stories.tsx

@@ -3,6 +3,7 @@ import React from "react";
 
 import specialCharacters from "../../__internal__/utils/argTypes/specialCharacters";
 import PopoverContainer from "./popover-container.component";
+import { Select, Option } from "../select";
 
 export default {
   title: "Popover Container/Test",
@@ -27,6 +28,28 @@ export const Default = ({
   open: boolean;
 }) => <PopoverContainer title={title || titleSpecialCharacters} open={open} />;
 
+export const WithSelect = () => {
+  return (
+    <div style={{ height: 100 }}>
+      <PopoverContainer
+        containerAriaLabel="popover-container"
+        openButtonAriaLabel="open"
+        title="select example"
+      >
+        <Select label="my select">
+          <Option value="red" text="red" />
+          <Option value="green" text="green" />
+          <Option value="blue" text="blue" />
+        </Select>
+      </PopoverContainer>
+    </div>
+  );
+};
+
+WithSelect.story = {
+  name: "with select",
+};
+
 Default.story = {
   name: "default",
   args: {

src/components/popover-container/popover-container.component.tsx

@@ -151,7 +151,6 @@ export const PopoverContainer = ({
   const isControlled = open !== undefined;
   const [isOpenInternal, setIsOpenInternal] = useState(false);
 
-  const ref = useRef<HTMLDivElement>(null);
   const closeButtonRef = useRef<HTMLButtonElement>(null);
   const openButtonRef = useRef<HTMLButtonElement>(null);
   const guid = useRef(createGuid());
@@ -209,14 +208,14 @@ export const PopoverContainer = ({
     if (onClose) onClose(e);
   };
 
-  useClickAwayListener([ref], handleClickAway, "mousedown");
+  const handleClick = useClickAwayListener(handleClickAway, "mousedown");
 
   return (
     <PopoverContainerWrapperStyle
       data-component="popover-container"
       role="region"
       aria-labelledby={popoverContainerId}
-      ref={ref}
+      onMouseDown={handleClick}
     >
       {renderOpenComponent(renderOpenComponentProps)}
       <Transition

src/components/split-button/split-button.component.tsx

@@ -242,12 +242,13 @@ export const SplitButton = ({
     );
   }
 
-  useClickAwayListener([splitButtonNode], hideButtons);
+  const handleClick = useClickAwayListener(hideButtons);
 
   return (
     <StyledSplitButton
       aria-haspopup="true"
       onMouseLeave={hideButtons}
+      onClick={handleClick}
       ref={splitButtonNode}
       {...componentTags()}
       {...filterStyledSystemMarginProps(rest)}

src/hooks/__internal__/useClickAwayListener/useClickAwayListener.spec.tsx

@@ -1,4 +1,4 @@
-import React, { useRef } from "react";
+import React from "react";
 import { render, screen, fireEvent } from "@testing-library/react";
 import "@testing-library/jest-dom";
 import useClickAwayListener from "./useClickAwayListener";
@@ -8,13 +8,19 @@ interface ClickAwayProps {
   eventTypeId?: "mousedown" | "click";
 }
 
-const MockComponent = ({ handleClickAway, eventTypeId }: ClickAwayProps) => {
-  const ref = useRef<HTMLDivElement>(null);
+const eventHandlerPropNames = { click: "onClick", mousedown: "onMouseDown" };
 
-  useClickAwayListener([ref], handleClickAway, eventTypeId);
+const MockComponent = ({
+  handleClickAway,
+  eventTypeId = "click",
+}: ClickAwayProps) => {
+  const onInsideClick = useClickAwayListener(handleClickAway, eventTypeId);
+  const onInsideClickProp = {
+    [eventHandlerPropNames[eventTypeId]]: onInsideClick,
+  };
 
   return (
-    <div data-testid="target-element" ref={ref}>
+    <div data-testid="target-element" {...onInsideClickProp}>
       Child
     </div>
   );

src/hooks/__internal__/useClickAwayListener/useClickAwayListener.ts

@@ -1,29 +1,38 @@
-import React, { useEffect, useRef } from "react";
+import { useEffect, useRef, useCallback } from "react";
+
+// Needs to also take Portals into account (so can't just check DOM containment), but ideally without using
+// event.stopPropagation() which could have unexpected and frustrating consequences for consumers.
+// Simple approach taken from https://github.com/facebook/react/issues/10962#issuecomment-444622208
 
 export default (
-  targets: React.RefObject<HTMLElement>[],
   handleClickAway: (ev: Event) => void,
   eventTypeId: "mousedown" | "click" = "click"
 ) => {
-  const targetsRef = useRef(targets);
-  targetsRef.current = targets;
+  const clickIsInside = useRef(false);
 
-  useEffect(() => {
-    const fnClickAway = (ev: Event) => {
-      const clickedElements = targetsRef.current.filter(
-        (targetRef: React.RefObject<HTMLElement>) =>
-          targetRef.current?.contains(ev?.target as Node)
-      );
-
-      if (!clickedElements?.length) {
-        handleClickAway(ev);
+  const onDocumentClick = useCallback(
+    (ev: Event) => {
+      if (clickIsInside.current) {
+        clickIsInside.current = false;
+        return;
       }
-    };
 
-    document.addEventListener(eventTypeId, fnClickAway as EventListener);
+      handleClickAway(ev);
+    },
+    [handleClickAway]
+  );
+
+  const onInsideClick = useCallback(() => {
+    clickIsInside.current = true;
+  }, []);
+
+  useEffect(() => {
+    document.addEventListener(eventTypeId, onDocumentClick);
 
     return function cleanup() {
-      document.removeEventListener(eventTypeId, fnClickAway as EventListener);
+      document.removeEventListener(eventTypeId, onDocumentClick);
     };
-  }, [handleClickAway, eventTypeId]);
+  }, [onDocumentClick, eventTypeId]);
+
+  return onInsideClick;
 };