fix(ui): only trigger useOnClickOutside when needed

useOnClickOutside should start listening for events only if there's an action to take.

Fixes #1754

ui/src/Components/Grid/AlertGrid/AlertGroup/Alert/AlertMenu.js

@@ -64,9 +64,10 @@ const MenuContent = ({
         ))}
         <div className="dropdown-divider" />
         <div
-          className={`dropdown-item cursor-pointer ${
-            Object.keys(alertStore.data.clustersWithoutReadOnly).length === 0 &&
-            "disabled"
+          className={`dropdown-item ${
+            Object.keys(alertStore.data.clustersWithoutReadOnly).length === 0
+              ? "disabled"
+              : "cursor-pointer"
           }`}
           onClick={() => {
             if (Object.keys(alertStore.data.clustersWithoutReadOnly).length) {
@@ -99,22 +100,22 @@ const AlertMenu = ({
   setIsMenuOpen,
 }) => {
   const collapse = useLocalStore(() => ({
-    value: true,
+    isHidden: true,
     toggle() {
-      this.value = !this.value;
-      setIsMenuOpen(!this.value);
+      this.isHidden = !this.isHidden;
+      setIsMenuOpen(!this.isHidden);
     },
     hide() {
-      this.value = true;
-      setIsMenuOpen(!this.value);
+      this.isHidden = true;
+      setIsMenuOpen(!this.isHidden);
     },
   }));
 
-  const ref = useRef(null);
-  useOnClickOutside(ref, collapse.hide);
+  const rootRef = useRef(null);
+  useOnClickOutside(rootRef, collapse.hide, !collapse.isHidden);
 
   return useObserver(() => (
-    <span ref={ref}>
+    <span ref={rootRef}>
       <Manager>
         <Reference>
           {({ ref }) => (
@@ -133,7 +134,7 @@ const AlertMenu = ({
             </span>
           )}
         </Reference>
-        <DropdownSlide in={!collapse.value} unmountOnExit>
+        <DropdownSlide in={!collapse.isHidden} unmountOnExit>
           <Popper
             placement="bottom-start"
             modifiers={[

ui/src/Components/Grid/AlertGrid/AlertGroup/GroupHeader/GroupMenu.js

@@ -72,9 +72,10 @@ const MenuContent = ({
           <FontAwesomeIcon icon={faShareSquare} /> Copy link to this group
         </div>
         <div
-          className={`dropdown-item cursor-pointer ${
-            Object.keys(alertStore.data.clustersWithoutReadOnly).length === 0 &&
-            "disabled"
+          className={`dropdown-item ${
+            Object.keys(alertStore.data.clustersWithoutReadOnly).length === 0
+              ? "disabled"
+              : "cursor-pointer"
           }`}
           onClick={() => {
             if (Object.keys(alertStore.data.clustersWithoutReadOnly).length) {
@@ -116,11 +117,11 @@ const GroupMenu = ({
     },
   }));
 
-  const ref = useRef(null);
-  useOnClickOutside(ref, collapse.hide);
+  const rootRef = useRef(null);
+  useOnClickOutside(rootRef, collapse.hide, !collapse.isHidden);
 
   return useObserver(() => (
-    <span ref={ref}>
+    <span ref={rootRef}>
       <Manager>
         <Reference>
           {({ ref }) => (

ui/src/Components/InlineEdit/index.js

@@ -45,7 +45,7 @@ const InlineEdit = ({
     }
   };
 
-  useOnClickOutside(ref, doneEditing);
+  useOnClickOutside(ref, doneEditing, isEditing);
 
   useEffect(() => {
     if (isEditing && ref.current) {

ui/src/Components/NavBar/FilterInput/History.js

@@ -198,7 +198,7 @@ const History = ({ alertStore, settingsStore }) => {
   });
 
   const ref = useRef(null);
-  useOnClickOutside(ref, collapse.hide);
+  useOnClickOutside(ref, collapse.hide, !collapse.isHidden);
 
   return useObserver(() => (
     // data-filters is there to register filters for observation in mobx

ui/src/Components/TooltipWrapper/index.js

@@ -1,4 +1,4 @@
-import React, { useState, useCallback, useEffect } from "react";
+import React, { useState, useEffect } from "react";
 import { createPortal } from "react-dom";
 import PropTypes from "prop-types";
 
@@ -26,9 +26,8 @@ const TooltipWrapper = ({ title, children, className }) => {
   const [isVisible, setIsVisible] = useState(false);
   const [wasClicked, setWasClicked] = useState(false);
 
-  const showTooltip = useCallback(() => setIsHovering(true), []);
-  const hideTooltip = useCallback(() => setIsHovering(false), []);
-  const handleClick = useCallback(() => setWasClicked(true), []);
+  const showTooltip = () => setIsHovering(true);
+  const hideTooltip = () => setIsHovering(false);
 
   useEffect(() => {
     let timerShow;
@@ -57,7 +56,7 @@ const TooltipWrapper = ({ title, children, className }) => {
   return (
     <React.Fragment>
       <div
-        onClick={handleClick}
+        onClick={() => setWasClicked(true)}
         onMouseOver={supportsTouch ? null : showTooltip}
         onMouseLeave={supportsTouch ? null : hideTooltip}
         onTouchStart={supportsTouch ? showTooltip : null}

ui/src/Hooks/useOnClickOutside.js

@@ -1,7 +1,7 @@
 import { useEffect } from "react";
 
 // https://usehooks.com/useOnClickOutside/
-function useOnClickOutside(ref, handler) {
+function useOnClickOutside(ref, handler, enabled) {
   useEffect(() => {
     const listener = (event) => {
       if (!ref.current || ref.current.contains(event.target)) {
@@ -10,14 +10,16 @@ function useOnClickOutside(ref, handler) {
       handler(event);
     };
 
-    document.addEventListener("mousedown", listener);
-    document.addEventListener("touchstart", listener);
+    if (enabled) {
+      document.addEventListener("mousedown", listener);
+      document.addEventListener("touchstart", listener);
+    }
 
     return () => {
       document.removeEventListener("mousedown", listener);
       document.removeEventListener("touchstart", listener);
     };
-  }, [ref, handler]);
+  }, [ref, handler, enabled]);
 }
 
 export { useOnClickOutside };

ui/src/Hooks/useOnClickOutside.test.js

@@ -6,10 +6,10 @@ import { mount } from "enzyme";
 import { useOnClickOutside } from "./useOnClickOutside";
 
 describe("useOnClickOutside", () => {
-  const Component = () => {
+  const Component = ({ enabled }) => {
     const ref = useRef();
     const [isModalOpen, setModalOpen] = useState(true);
-    useOnClickOutside(ref, () => setModalOpen(false));
+    useOnClickOutside(ref, () => setModalOpen(false), enabled);
 
     return (
       <div>
@@ -25,7 +25,7 @@ describe("useOnClickOutside", () => {
   };
 
   it("closes modal on click outside", () => {
-    const tree = mount(<Component />);
+    const tree = mount(<Component enabled />);
     expect(tree.text()).toBe("Open");
 
     const clickEvent = document.createEvent("MouseEvents");
@@ -38,7 +38,7 @@ describe("useOnClickOutside", () => {
   });
 
   it("ignores events when hidden", () => {
-    const tree = mount(<Component />);
+    const tree = mount(<Component enabled />);
     expect(tree.text()).toBe("Open");
 
     const clickEvent = document.createEvent("MouseEvents");
@@ -54,9 +54,34 @@ describe("useOnClickOutside", () => {
   });
 
   it("modal stays open on click inside", () => {
-    const tree = mount(<Component />);
+    const tree = mount(<Component enabled />);
     expect(tree.text()).toBe("Open");
     tree.find("span").simulate("click");
     expect(tree.text()).toBe("Open");
   });
+
+  it("only runs when enabled", () => {
+    const tree = mount(<Component enabled={false} />);
+    expect(tree.text()).toBe("Open");
+
+    const clickEvent = document.createEvent("MouseEvents");
+    clickEvent.initEvent("mousedown", true, true);
+    act(() => {
+      document.dispatchEvent(clickEvent);
+    });
+
+    expect(tree.text()).toBe("Open");
+
+    tree.setProps({ enabled: true });
+    act(() => {
+      document.dispatchEvent(clickEvent);
+    });
+    expect(tree.text()).toBe("Hidden");
+  });
+
+  it("unmounts cleanly", () => {
+    const tree = mount(<Component enabled />);
+    expect(tree.text()).toBe("Open");
+    tree.unmount();
+  });
 });