fix: clicking outside with user-select: none closes popover

omsimos · Oct 1, 2024 · 49444a5 · 49444a5
2 parents 4952599 + b9626a0
commit 49444a5
Show file tree

Hide file tree

Showing 4 changed files with 20 additions and 70 deletions.
diff --git a/lib/CHANGELOG.md b/lib/CHANGELOG.md
@@ -1,10 +1,28 @@
 # @omsimos/react-highlight-popover
 
+## 1.3.2
+
+### Patch Changes
+
+- ### Bug Fixes 🐞
+
+  - **Removed Redundant Click-Outside Logic**:
+    - The `useEffect` that handled clicks outside the popover has been removed. This was causing issues where clicking outside on elements with `user-select: none` would unintentionally close the popover, even though the text remained highlighted.
+    - Since clicking outside naturally removes text selection, the previous logic was redundant and caused bugs by closing the popover prematurely.
+
+  ***
+
+  To upgrade to v1.3.2, run:
+
+  ```bash
+  npm install @omsimos/react-highlight-popover@latest
+  ```
+
 ## 1.3.1
 
 ### Patch Changes
 
-- ### Fixes 🛠
+- ### Bug Fixes 🐞
 
   - **Removed Unintended `.mjs` File**: An unnecessary file was unintentionally included in the v1.3.0 build. This patch removes the file, ensuring a cleaner build and reducing the package size.
 

diff --git a/lib/package.json b/lib/package.json
@@ -1,6 +1,6 @@
 {
   "name": "@omsimos/react-highlight-popover",
-  "version": "1.3.1",
+  "version": "1.3.2",
   "main": "dist/index.js",
   "module": "dist/index.js",
   "types": "dist/index.d.ts",

diff --git a/lib/src/HighlightPopover.tsx b/lib/src/HighlightPopover.tsx
@@ -230,24 +230,6 @@ export const HighlightPopover = memo(function HighlightPopover({
     };
   }, [handleSelection]);
 
-  // Handle clicks outside the popover
-  useEffect(() => {
-    const handleClickOutside = (event: MouseEvent) => {
-      if (
-        containerRef.current &&
-        !containerRef.current.contains(event.target as Node)
-      ) {
-        setShowPopover(false);
-        onPopoverHide?.();
-      }
-    };
-
-    document.addEventListener("mousedown", handleClickOutside);
-    return () => {
-      document.removeEventListener("mousedown", handleClickOutside);
-    };
-  }, [onPopoverHide]);
-
   const contextValue = useMemo<HighlightPopoverContextType>(
     () => ({
       showPopover,

diff --git a/tests/HighlightPopover.test.tsx b/tests/HighlightPopover.test.tsx
@@ -83,56 +83,6 @@ describe("HighlightPopover Component", () => {
     });
   });
 
-  test("hides popover when clicking outside", async () => {
-    const renderPopoverMock = mock(() => (
-      <div data-testid="popover-hide">Popover Content</div>
-    ));
-    const onPopoverHideMock = mock();
-
-    render(
-      <HighlightPopover
-        renderPopover={renderPopoverMock}
-        onPopoverHide={onPopoverHideMock}
-      >
-        <p data-testid="selectable-text-hide">Select this text</p>
-      </HighlightPopover>,
-    );
-
-    const textElement = screen.getByTestId("selectable-text-hide");
-
-    await act(async () => {
-      const range = document.createRange();
-      range.selectNodeContents(textElement);
-      const selection = window.getSelection();
-      if (selection) {
-        selection.removeAllRanges();
-        selection.addRange(range);
-      }
-
-      const event = new Event("selectionchange", {
-        bubbles: true,
-        cancelable: true,
-      });
-      document.dispatchEvent(event);
-    });
-
-    // Wait for the popover to appear
-    await waitFor(() => {
-      expect(screen.getByTestId("popover-hide")).toBeDefined();
-    });
-
-    // Click outside
-    await act(async () => {
-      fireEvent.mouseDown(document.body);
-    });
-
-    // Wait for the popover to disappear
-    await waitFor(() => {
-      expect(onPopoverHideMock).toHaveBeenCalled();
-      expect(screen.queryByTestId("popover-hide")).toBeNull();
-    });
-  });
-
   test("applies offset to popover position", async () => {
     const renderPopoverMock = mock(({ position }) => (
       <div