NXT-4175: FileExplorerContextMenu improvements

.changeset/proud-dragons-march.md

@@ -0,0 +1,9 @@
+---
+"@knime/components": minor
+---
+
+- Add onContextMenuOutside hook from knime-ui (vueuse `onClickOutside` ignores `contextmenu` events)
+- Prevent default contextmenu behavior on BaseMenuItems
+- Close file explorer context menu on file explorer options button click, if context menu was already open by option button
+- Close file explorer context menu on right-click (`contextmenu` event) outside
+- Combine `shift` and `flip` middleware from `floating-ui` on file explorer context menu

packages/components/src/components/FileExplorer/components/FileExplorer.vue

@@ -454,7 +454,7 @@ const openContextMenuByMouse = (
   index: number,
 ) => {
   if (isContextMenuVisible.value) {
-    closeContextMenu();
+    closeContextMenu(false);
     return;
   }
 
@@ -492,6 +492,11 @@ const openContextMenuByOptionsMenu = (
   clickedItem: FileExplorerItemType,
   index: number,
 ) => {
+  if (isContextMenuVisible.value) {
+    closeContextMenu(false);
+    return;
+  }
+
   const rect = element.getBoundingClientRect();
 
   // only the one with the menu should be selected
@@ -691,6 +696,7 @@ const hasOptionsMenu = computed(() => {
 
           <template v-if="hasOptionsMenu" #optionsMenu>
             <FunctionButton
+              tabindex="-1"
               :active="
                 contextMenuAnchor?.openedBy === 'optionsMenu' &&
                 contextMenuAnchor?.item.id === item.id
@@ -737,7 +743,7 @@ const hasOptionsMenu = computed(() => {
       :anchor="contextMenuAnchor"
       :selected-items="selectedItems"
       @item-click="onContextMenuItemClick"
-      @close="() => closeContextMenu()"
+      @close="closeContextMenu(false)"
     >
       <template #default="slotProps">
         <slot

packages/components/src/components/FileExplorer/components/FileExplorerContextMenu.vue

@@ -1,8 +1,9 @@
 <script setup lang="ts">
 import { computed, onMounted, onUnmounted, ref, toRefs, watch } from "vue";
 import { onClickOutside } from "@vueuse/core";
-import { autoUpdate, flip, offset, useFloating } from "@floating-ui/vue";
+import { autoUpdate, flip, offset, shift, useFloating } from "@floating-ui/vue";
 
+import { onContextMenuOutside } from "../../../composables";
 import MenuItems from "../../base/MenuItem/MenuItems.vue";
 import type { MenuItem as BaseMenuItem } from "../../types";
 import type {
@@ -72,6 +73,7 @@ const offsetY = computed(() => {
 const middleware = computed(() => [
   offset({ mainAxis: offsetY.value, crossAxis: offsetX.value }),
   flip(),
+  shift(),
 ]);
 
 const { floatingStyles, update: updateFloating } = useFloating(
@@ -161,7 +163,21 @@ const closeMenu = () => {
   emit("close");
 };
 
-onClickOutside(menuWrapper, closeMenu);
+onClickOutside(menuWrapper, (event) => {
+  if (
+    props.anchor.openedBy === "optionsMenu" &&
+    props.anchor.element.contains(event.target as Node)
+  ) {
+    return;
+  }
+  closeMenu();
+});
+onContextMenuOutside(menuWrapper, (event) => {
+  if (props.anchor.element.contains(event.target as Node)) {
+    return;
+  }
+  closeMenu();
+});
 </script>
 
 <template>
@@ -190,9 +206,6 @@ onClickOutside(menuWrapper, closeMenu);
 
 <style lang="postcss" scoped>
 .menu-wrapper {
-  position: absolute;
-  top: calc(v-bind("$props.position.y") * 1px);
-  left: calc(v-bind("$props.position.x") * 1px);
   z-index: var(--file-explorer-context-menu-z-index, 5);
 }
 </style>

packages/components/src/components/base/MenuItem/BaseMenuItems.vue

@@ -208,6 +208,7 @@ const onItemClick = (event: MouseEvent, item: MenuItem, id: string) => {
       :style="useMaxMenuWidth ? { 'max-width': `${maxMenuWidth}px` } : {}"
       :title="item.title"
       @click="onItemClick($event, item, id)"
+      @contextmenu.prevent
       @pointerenter="onPointerEnter($event, item, index)"
     >
       <slot

packages/components/src/composables/__tests__/onContextMenuOutside.test.ts

@@ -0,0 +1,232 @@
+import { beforeEach, describe, expect, it, vi } from "vitest";
+import { ref } from "vue";
+import { unrefElement, useEventListener } from "@vueuse/core";
+
+import onContextMenuOutside from "../onContextMenuOutside";
+
+vi.mock("@vueuse/core", () => ({
+  useEventListener: vi.fn(),
+  unrefElement: vi.fn(),
+}));
+
+describe("onContextMenuOutside", () => {
+  let mockHandler: ReturnType<typeof vi.fn>;
+  let mockUseEventListener: ReturnType<typeof vi.fn>;
+  let mockUnrefElement: ReturnType<typeof vi.fn>;
+  let capturedEventHandler: ((event: MouseEvent) => void) | undefined;
+
+  beforeEach(() => {
+    mockUseEventListener = useEventListener as ReturnType<typeof vi.fn>;
+    mockUnrefElement = unrefElement as ReturnType<typeof vi.fn>;
+
+    mockHandler = vi.fn();
+    capturedEventHandler = undefined;
+
+    mockUseEventListener.mockImplementation((_, handler) => {
+      capturedEventHandler = handler as (event: MouseEvent) => void;
+    });
+
+    vi.clearAllMocks();
+  });
+
+  it("should set up contextmenu event listener", () => {
+    const targetElement = document.createElement("div");
+    const targetRef = ref(targetElement);
+
+    onContextMenuOutside(targetRef, mockHandler);
+
+    expect(mockUseEventListener).toHaveBeenCalledWith(
+      "contextmenu",
+      expect.any(Function),
+    );
+  });
+
+  it("should call handler when contextmenu event occurs outside target element", () => {
+    const targetElement = document.createElement("div");
+    const outsideElement = document.createElement("div");
+    const targetRef = ref(targetElement);
+
+    mockUnrefElement.mockReturnValue(targetElement);
+
+    targetElement.contains = vi.fn().mockReturnValue(false);
+
+    onContextMenuOutside(targetRef, mockHandler);
+
+    const mockEvent = new MouseEvent("contextmenu", { bubbles: true });
+    Object.defineProperty(mockEvent, "target", {
+      value: outsideElement,
+    });
+    capturedEventHandler?.(mockEvent);
+
+    expect(mockUnrefElement).toHaveBeenCalledWith(targetRef);
+    expect(targetElement.contains).toHaveBeenCalledWith(outsideElement);
+    expect(mockHandler).toHaveBeenCalledWith(mockEvent);
+  });
+
+  it("should not call handler when contextmenu event occurs inside target element", () => {
+    const targetElement = document.createElement("div");
+    const insideElement = document.createElement("div");
+    const targetRef = ref(targetElement);
+
+    mockUnrefElement.mockReturnValue(targetElement);
+
+    targetElement.contains = vi.fn().mockReturnValue(true);
+
+    onContextMenuOutside(targetRef, mockHandler);
+
+    const mockEvent = new MouseEvent("contextmenu", { bubbles: true });
+    Object.defineProperty(mockEvent, "target", { value: insideElement });
+
+    capturedEventHandler?.(mockEvent);
+
+    expect(mockUnrefElement).toHaveBeenCalledWith(targetRef);
+    expect(targetElement.contains).toHaveBeenCalledWith(insideElement);
+    expect(mockHandler).not.toHaveBeenCalled();
+  });
+
+  it("should not call handler when target element is null", () => {
+    const targetRef = ref(null);
+
+    mockUnrefElement.mockReturnValue(null);
+
+    onContextMenuOutside(targetRef, mockHandler);
+
+    const mockEvent = new MouseEvent("contextmenu", { bubbles: true });
+    Object.defineProperty(mockEvent, "target", {
+      value: document.createElement("div"),
+    });
+
+    capturedEventHandler?.(mockEvent);
+
+    expect(mockUnrefElement).toHaveBeenCalledWith(targetRef);
+    expect(mockHandler).not.toHaveBeenCalled();
+  });
+
+  it("should not call handler when target element is undefined", () => {
+    const targetRef = ref(undefined);
+
+    mockUnrefElement.mockReturnValue(undefined);
+
+    onContextMenuOutside(targetRef, mockHandler);
+
+    const mockEvent = new MouseEvent("contextmenu", { bubbles: true });
+    Object.defineProperty(mockEvent, "target", {
+      value: document.createElement("div"),
+    });
+
+    capturedEventHandler?.(mockEvent);
+
+    expect(mockUnrefElement).toHaveBeenCalledWith(targetRef);
+    expect(mockHandler).not.toHaveBeenCalled();
+  });
+
+  it("should handle different MaybeElementRef types", () => {
+    const targetElement = document.createElement("div");
+
+    mockUnrefElement.mockReturnValue(targetElement);
+    targetElement.contains = vi.fn().mockReturnValue(false);
+
+    onContextMenuOutside(targetElement, mockHandler);
+
+    const mockEvent = new MouseEvent("contextmenu", { bubbles: true });
+    Object.defineProperty(mockEvent, "target", {
+      value: document.createElement("div"),
+    });
+
+    capturedEventHandler?.(mockEvent);
+
+    expect(mockUnrefElement).toHaveBeenCalledWith(targetElement);
+    expect(mockHandler).toHaveBeenCalledWith(mockEvent);
+  });
+
+  describe("event target handling", () => {
+    it("should handle event target as HTMLElement", () => {
+      const targetElement = document.createElement("div");
+      const eventTarget = document.createElement("span");
+      const targetRef = ref(targetElement);
+
+      mockUnrefElement.mockReturnValue(targetElement);
+      targetElement.contains = vi.fn().mockReturnValue(false);
+
+      onContextMenuOutside(targetRef, mockHandler);
+
+      const mockEvent = new MouseEvent("contextmenu", { bubbles: true });
+      Object.defineProperty(mockEvent, "target", { value: eventTarget });
+
+      capturedEventHandler?.(mockEvent);
+
+      expect(targetElement.contains).toHaveBeenCalledWith(eventTarget);
+      expect(mockHandler).toHaveBeenCalledWith(mockEvent);
+    });
+
+    it("should handle nested elements correctly", () => {
+      const targetElement = document.createElement("div");
+      const childElement = document.createElement("span");
+      const nestedElement = document.createElement("p");
+
+      targetElement.appendChild(childElement);
+      childElement.appendChild(nestedElement);
+
+      const targetRef = ref(targetElement);
+
+      mockUnrefElement.mockReturnValue(targetElement);
+      targetElement.contains = vi.fn().mockReturnValue(true);
+
+      onContextMenuOutside(targetRef, mockHandler);
+
+      const mockEvent = new MouseEvent("contextmenu", { bubbles: true });
+      Object.defineProperty(mockEvent, "target", { value: nestedElement });
+
+      capturedEventHandler?.(mockEvent);
+
+      expect(targetElement.contains).toHaveBeenCalledWith(nestedElement);
+      expect(mockHandler).not.toHaveBeenCalled();
+    });
+  });
+
+  describe("integration scenarios", () => {
+    it("should work correctly when element is found but click is outside", () => {
+      const container = document.createElement("div");
+      const targetElement = document.createElement("div");
+      const outsideElement = document.createElement("div");
+
+      container.appendChild(targetElement);
+      container.appendChild(outsideElement);
+
+      const targetRef = ref(targetElement);
+
+      mockUnrefElement.mockReturnValue(targetElement);
+
+      onContextMenuOutside(targetRef, mockHandler);
+
+      const mockEvent = new MouseEvent("contextmenu", { bubbles: true });
+      Object.defineProperty(mockEvent, "target", { value: outsideElement });
+
+      capturedEventHandler?.(mockEvent);
+
+      expect(mockHandler).toHaveBeenCalledWith(mockEvent);
+    });
+
+    it("should work correctly when element is found and click is inside", () => {
+      const container = document.createElement("div");
+      const targetElement = document.createElement("div");
+      const childElement = document.createElement("span");
+
+      container.appendChild(targetElement);
+      targetElement.appendChild(childElement);
+
+      const targetRef = ref(targetElement);
+
+      mockUnrefElement.mockReturnValue(targetElement);
+
+      onContextMenuOutside(targetRef, mockHandler);
+
+      const mockEvent = new MouseEvent("contextmenu", { bubbles: true });
+      Object.defineProperty(mockEvent, "target", { value: childElement });
+
+      capturedEventHandler?.(mockEvent);
+
+      expect(mockHandler).not.toHaveBeenCalled();
+    });
+  });
+});

packages/components/src/composables/index.ts

@@ -1,3 +1,4 @@
+import onContextMenuOutside from "./onContextMenuOutside";
 import useClickOutside from "./useClickOutside";
 import useDropdownNavigation from "./useDropdownNavigation";
 import useKeyPressedUntilMouseClick from "./useKeyPressedUntilMouseClick";
@@ -9,4 +10,9 @@ export * from "./useFileSizeFormatting";
 export * from "./useBeforeUnload";
 export * from "./useAutoCloseOnCompletion";
 
-export { useClickOutside, useDropdownNavigation, useKeyPressedUntilMouseClick };
+export {
+  useClickOutside,
+  onContextMenuOutside,
+  useDropdownNavigation,
+  useKeyPressedUntilMouseClick,
+};

packages/components/src/composables/onContextMenuOutside.ts

@@ -0,0 +1,30 @@
+import {
+  type MaybeElementRef,
+  unrefElement,
+  useEventListener,
+} from "@vueuse/core";
+
+/**
+ * `onClickOutside` from Vueuse doesn't handle right click reliably.
+ * This hook can be used alongside `onClickOutside` to make sure that
+ * both left and right click outside are handled.
+ */
+const onContextMenuOutside = (
+  target: MaybeElementRef,
+  handler: (event: MouseEvent) => void,
+) => {
+  useEventListener("contextmenu", (event: MouseEvent) => {
+    const element = unrefElement(target);
+    if (!element) {
+      return;
+    }
+
+    const eventTarget = event.target as HTMLElement;
+    const clickedInside = element.contains(eventTarget);
+    if (!clickedInside) {
+      handler(event);
+    }
+  });
+};
+
+export default onContextMenuOutside;