APP-6535: only fire clickOutside if click starts and ends outside (#616)

viamrobotics · Jan 14, 2025 · 2c8cd1d · 2c8cd1d
1 parent dbe2e5f
commit 2c8cd1d
Show file tree

Hide file tree

Showing 3 changed files with 69 additions and 15 deletions.
diff --git a/packages/core/package.json b/packages/core/package.json
@@ -1,6 +1,6 @@
 {
   "name": "@viamrobotics/prime-core",
-  "version": "0.0.172",
+  "version": "0.0.173",
   "repository": {
     "type": "git",
     "url": "https://github.com/viamrobotics/prime.git",

diff --git a/packages/core/src/lib/__tests__/click-outside.spec.ts b/packages/core/src/lib/__tests__/click-outside.spec.ts
@@ -8,9 +8,10 @@ describe('use:clickOutside', () => {
   const onClickOutside = vi.fn();
 
   it('should trigger a callback only when clicked outside', async () => {
+    const user = userEvent.setup();
+
     render(Subject, { onClickOutside });
 
-    const user = userEvent.setup();
     const subject = screen.getByTestId('subject');
     const insideButton = screen.getByTestId('inside');
     const outsideButton = screen.getByTestId('outside');
@@ -26,9 +27,9 @@ describe('use:clickOutside', () => {
   });
 
   it('should not trigger if clicked element gets removed from the DOM', async () => {
+    const user = userEvent.setup();
     render(Subject, { onClickOutside });
 
-    const user = userEvent.setup();
     const insideButton = screen.getByTestId('inside');
 
     insideButton.addEventListener('click', () => {
@@ -39,4 +40,38 @@ describe('use:clickOutside', () => {
 
     expect(onClickOutside).not.toHaveBeenCalled();
   });
+
+  it('should not trigger if click starts inside element and moves out', async () => {
+    const user = userEvent.setup();
+    render(Subject, { onClickOutside });
+
+    const insideButton = screen.getByTestId('inside');
+    const outsideButton = screen.getByTestId('outside');
+
+    // 1. press the left mouse button on the inside button
+    // 2. release the left mouse button on the outside button
+    await user.pointer([
+      { keys: '[MouseLeft>]', target: insideButton },
+      { keys: '[/MouseLeft]', target: outsideButton },
+    ]);
+
+    expect(onClickOutside).not.toHaveBeenCalled();
+  });
+
+  it('should not trigger if click starts outside element and moves in', async () => {
+    const user = userEvent.setup();
+    render(Subject, { onClickOutside });
+
+    const insideButton = screen.getByTestId('inside');
+    const outsideButton = screen.getByTestId('outside');
+
+    // 1. press the left mouse button on the inside button
+    // 2. release the left mouse button on the outside button
+    await user.pointer([
+      { keys: '[MouseLeft>]', target: outsideButton },
+      { keys: '[/MouseLeft]', target: insideButton },
+    ]);
+
+    expect(onClickOutside).not.toHaveBeenCalled();
+  });
 });
diff --git a/packages/core/src/lib/click-outside.ts b/packages/core/src/lib/click-outside.ts
@@ -33,31 +33,50 @@ export const clickOutside: Action<
   ClickOutsideHandler | undefined
 > = (node, handler) => {
   let handleClickOutside = handler;
+  let isMouseDown = false;
 
-  const handleWindowClick = (event: MouseEvent): void => {
-    if (!node || !handleClickOutside) {
-      return;
+  const getOutsideTarget = (event: MouseEvent): Element | undefined => {
+    const { target } = event;
+
+    return node &&
+      target instanceof Element &&
+      window.document.contains(target) &&
+      !node.contains(target)
+      ? target
+      : undefined;
+  };
+
+  const handleMouseDown = (event: MouseEvent): void => {
+    const target = getOutsideTarget(event);
+
+    if (target) {
+      isMouseDown = true;
     }
+  };
 
-    const target = event.target as Element;
+  const handleMouseUp = (event: MouseEvent): void => {
+    const previousIsMouseDown = isMouseDown;
+    const target = getOutsideTarget(event);
+    isMouseDown = false;
 
-    if (
-      window.document.contains(target) &&
-      !node.contains(target) &&
-      !event.defaultPrevented
-    ) {
-      handleClickOutside(target);
+    if (target && previousIsMouseDown) {
+      handleClickOutside?.(target);
     }
   };
 
-  window.document.addEventListener('click', handleWindowClick, true);
+  // Listen to mousedown and mouseup rather than click
+  // so don't trigger if the click starts inside the element and moves out.
+  // TODO(mc, 2025-01-14): investigate whether these need to be in the capture phase
+  window.document.addEventListener('mousedown', handleMouseDown, true);
+  window.document.addEventListener('mouseup', handleMouseUp, true);
 
   return {
     update: (nextHandler: ClickOutsideHandler | undefined) => {
       handleClickOutside = nextHandler;
     },
     destroy: () => {
-      window.document.removeEventListener('click', handleWindowClick, true);
+      window.document.removeEventListener('mousedown', handleMouseDown, true);
+      window.document.removeEventListener('mouseup', handleMouseUp, true);
     },
   };
 };