Skip to content

Commit 7402429

Browse files
committed
NXT-4175: FileExplorerContextMenu improvements
- Add onContextMenuOutside hook from knime-ui - Add contextmenu emits to MenuItems and BaseMenuItems - Toggle context menu on options button clicks - Close context menu on right-click outside - Combine shift and flip floating-ui middleware NXT-4175 (Layout rework)
1 parent 229539d commit 7402429

File tree

8 files changed

+315
-7
lines changed

8 files changed

+315
-7
lines changed

.changeset/proud-dragons-march.md

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
---
2+
"@knime/components": minor
3+
---
4+
5+
- Add onContextMenuOutside hook from knime-ui (vueuse `onClickOutside` ignores `contextmenu` events)
6+
- Add contextmenu emits to `MenuItems` and `BaseMenuItems` components
7+
- Close file explorer context menu on file explorer options button click, if context menu was already open by option button
8+
- Close file explorer context menu on right-click (`contextmenu` event) outside
9+
- Combine `shift` and `flip` middleware from `floating-ui` on file explorer context menu

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

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -492,6 +492,11 @@ const openContextMenuByOptionsMenu = (
492492
clickedItem: FileExplorerItemType,
493493
index: number,
494494
) => {
495+
if (isContextMenuVisible.value) {
496+
closeContextMenu();
497+
return;
498+
}
499+
495500
const rect = element.getBoundingClientRect();
496501
497502
// only the one with the menu should be selected

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

Lines changed: 20 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,9 @@
11
<script setup lang="ts">
22
import { computed, onMounted, onUnmounted, ref, toRefs, watch } from "vue";
33
import { onClickOutside } from "@vueuse/core";
4-
import { autoUpdate, flip, offset, useFloating } from "@floating-ui/vue";
4+
import { autoUpdate, flip, offset, shift, useFloating } from "@floating-ui/vue";
55
6+
import { onContextMenuOutside } from "../../../composables";
67
import MenuItems from "../../base/MenuItem/MenuItems.vue";
78
import type { MenuItem as BaseMenuItem } from "../../types";
89
import type {
@@ -72,6 +73,7 @@ const offsetY = computed(() => {
7273
const middleware = computed(() => [
7374
offset({ mainAxis: offsetY.value, crossAxis: offsetX.value }),
7475
flip(),
76+
shift(),
7577
]);
7678
7779
const { floatingStyles, update: updateFloating } = useFloating(
@@ -161,12 +163,27 @@ const closeMenu = () => {
161163
emit("close");
162164
};
163165
164-
onClickOutside(menuWrapper, closeMenu);
166+
onClickOutside(menuWrapper, (event) => {
167+
if (
168+
props.anchor.openedBy === "optionsMenu" &&
169+
props.anchor.element.contains(event.target as Node)
170+
) {
171+
return;
172+
}
173+
closeMenu();
174+
});
175+
onContextMenuOutside(menuWrapper, (event) => {
176+
if (props.anchor.element.contains(event.target as Node)) {
177+
return;
178+
}
179+
event.preventDefault();
180+
closeMenu();
181+
});
165182
</script>
166183

167184
<template>
168185
<div
169-
v-show="shouldShowMenu"
186+
v-if="shouldShowMenu"
170187
ref="menuWrapper"
171188
:style="floatingStyles"
172189
class="menu-wrapper"
@@ -190,9 +207,6 @@ onClickOutside(menuWrapper, closeMenu);
190207

191208
<style lang="postcss" scoped>
192209
.menu-wrapper {
193-
position: absolute;
194-
top: calc(v-bind("$props.position.y") * 1px);
195-
left: calc(v-bind("$props.position.x") * 1px);
196210
z-index: var(--file-explorer-context-menu-z-index, 5);
197211
}
198212
</style>

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,7 @@ const props = withDefaults(defineProps<Props>(), {
5454
5555
const emit = defineEmits<{
5656
"item-click": [MouseEvent, MenuItem, string];
57+
"item-contextmenu": [MouseEvent, MenuItem, string];
5758
"item-focused": [id: string | null, MenuItem | null];
5859
"item-hovered": [MenuItem | null, string, number | null];
5960
}>();
@@ -208,6 +209,7 @@ const onItemClick = (event: MouseEvent, item: MenuItem, id: string) => {
208209
:style="useMaxMenuWidth ? { 'max-width': `${maxMenuWidth}px` } : {}"
209210
:title="item.title"
210211
@click="onItemClick($event, item, id)"
212+
@contextmenu="(event) => $emit('item-contextmenu', event, item, id)"
211213
@pointerenter="onPointerEnter($event, item, index)"
212214
>
213215
<slot

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

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,8 @@
2424
* otherwise we use the generic `button` and leave the handling of the action to the wrapping component that reacts
2525
* to `item-click` and calls any action.
2626
*
27+
* Right-clicking an item emits `@item-contextmenu`.
28+
*
2729
* Hovering an item emits `@item-hovered`.
2830
*
2931
* There is support for sub menus with the `children` key in items. The sublevel menus are recursive and create
@@ -106,6 +108,7 @@ const props = withDefaults(defineProps<MenuItemsProps>(), {
106108
type Emits = {
107109
close: [];
108110
"item-click": [event: MouseEvent, item: MenuItem, menuId: string];
111+
"item-contextmenu": [event: MouseEvent, item: MenuItem, menuId: string];
109112
"item-focused": [itemId: string | null, item: MenuItem | null];
110113
"item-hovered": [item: MenuItem | null, menuId: string, index: number | null];
111114
"close-submenu": [];
@@ -234,6 +237,9 @@ const hasSelectedChildItem = (item: MenuItem) => {
234237
:clipping-boundary="clippingBoundary"
235238
@keydown="props.registerKeydown && onKeydown($event)"
236239
@item-click="(...args: Emits['item-click']) => $emit('item-click', ...args)"
240+
@item-contextmenu="
241+
(...args: Emits['item-contextmenu']) => $emit('item-contextmenu', ...args)
242+
"
237243
@item-hovered="(...args: Emits['item-hovered']) => onItemHovered(...args)"
238244
@item-focused="
239245
(...args: Emits['item-focused']) => $emit('item-focused', ...args)
@@ -282,6 +288,10 @@ const hasSelectedChildItem = (item: MenuItem) => {
282288
@item-click="
283289
(...args: Emits['item-click']) => $emit('item-click', ...args)
284290
"
291+
@item-contextmenu="
292+
(...args: Emits['item-contextmenu']) =>
293+
$emit('item-contextmenu', ...args)
294+
"
285295
@item-hovered="
286296
(...args: Emits['item-hovered']) =>
287297
$emit('item-hovered', ...args)
Lines changed: 232 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,232 @@
1+
import { beforeEach, describe, expect, it, vi } from "vitest";
2+
import { ref } from "vue";
3+
import { unrefElement, useEventListener } from "@vueuse/core";
4+
5+
import onContextMenuOutside from "../onContextMenuOutside";
6+
7+
vi.mock("@vueuse/core", () => ({
8+
useEventListener: vi.fn(),
9+
unrefElement: vi.fn(),
10+
}));
11+
12+
describe("onContextMenuOutside", () => {
13+
let mockHandler: ReturnType<typeof vi.fn>;
14+
let mockUseEventListener: ReturnType<typeof vi.fn>;
15+
let mockUnrefElement: ReturnType<typeof vi.fn>;
16+
let capturedEventHandler: ((event: MouseEvent) => void) | undefined;
17+
18+
beforeEach(() => {
19+
mockUseEventListener = useEventListener as ReturnType<typeof vi.fn>;
20+
mockUnrefElement = unrefElement as ReturnType<typeof vi.fn>;
21+
22+
mockHandler = vi.fn();
23+
capturedEventHandler = undefined;
24+
25+
mockUseEventListener.mockImplementation((_, handler) => {
26+
capturedEventHandler = handler as (event: MouseEvent) => void;
27+
});
28+
29+
vi.clearAllMocks();
30+
});
31+
32+
it("should set up contextmenu event listener", () => {
33+
const targetElement = document.createElement("div");
34+
const targetRef = ref(targetElement);
35+
36+
onContextMenuOutside(targetRef, mockHandler);
37+
38+
expect(mockUseEventListener).toHaveBeenCalledWith(
39+
"contextmenu",
40+
expect.any(Function),
41+
);
42+
});
43+
44+
it("should call handler when contextmenu event occurs outside target element", () => {
45+
const targetElement = document.createElement("div");
46+
const outsideElement = document.createElement("div");
47+
const targetRef = ref(targetElement);
48+
49+
mockUnrefElement.mockReturnValue(targetElement);
50+
51+
targetElement.contains = vi.fn().mockReturnValue(false);
52+
53+
onContextMenuOutside(targetRef, mockHandler);
54+
55+
const mockEvent = new MouseEvent("contextmenu", { bubbles: true });
56+
Object.defineProperty(mockEvent, "target", {
57+
value: outsideElement,
58+
});
59+
capturedEventHandler?.(mockEvent);
60+
61+
expect(mockUnrefElement).toHaveBeenCalledWith(targetRef);
62+
expect(targetElement.contains).toHaveBeenCalledWith(outsideElement);
63+
expect(mockHandler).toHaveBeenCalledWith(mockEvent);
64+
});
65+
66+
it("should not call handler when contextmenu event occurs inside target element", () => {
67+
const targetElement = document.createElement("div");
68+
const insideElement = document.createElement("div");
69+
const targetRef = ref(targetElement);
70+
71+
mockUnrefElement.mockReturnValue(targetElement);
72+
73+
targetElement.contains = vi.fn().mockReturnValue(true);
74+
75+
onContextMenuOutside(targetRef, mockHandler);
76+
77+
const mockEvent = new MouseEvent("contextmenu", { bubbles: true });
78+
Object.defineProperty(mockEvent, "target", { value: insideElement });
79+
80+
capturedEventHandler?.(mockEvent);
81+
82+
expect(mockUnrefElement).toHaveBeenCalledWith(targetRef);
83+
expect(targetElement.contains).toHaveBeenCalledWith(insideElement);
84+
expect(mockHandler).not.toHaveBeenCalled();
85+
});
86+
87+
it("should not call handler when target element is null", () => {
88+
const targetRef = ref(null);
89+
90+
mockUnrefElement.mockReturnValue(null);
91+
92+
onContextMenuOutside(targetRef, mockHandler);
93+
94+
const mockEvent = new MouseEvent("contextmenu", { bubbles: true });
95+
Object.defineProperty(mockEvent, "target", {
96+
value: document.createElement("div"),
97+
});
98+
99+
capturedEventHandler?.(mockEvent);
100+
101+
expect(mockUnrefElement).toHaveBeenCalledWith(targetRef);
102+
expect(mockHandler).not.toHaveBeenCalled();
103+
});
104+
105+
it("should not call handler when target element is undefined", () => {
106+
const targetRef = ref(undefined);
107+
108+
mockUnrefElement.mockReturnValue(undefined);
109+
110+
onContextMenuOutside(targetRef, mockHandler);
111+
112+
const mockEvent = new MouseEvent("contextmenu", { bubbles: true });
113+
Object.defineProperty(mockEvent, "target", {
114+
value: document.createElement("div"),
115+
});
116+
117+
capturedEventHandler?.(mockEvent);
118+
119+
expect(mockUnrefElement).toHaveBeenCalledWith(targetRef);
120+
expect(mockHandler).not.toHaveBeenCalled();
121+
});
122+
123+
it("should handle different MaybeElementRef types", () => {
124+
const targetElement = document.createElement("div");
125+
126+
mockUnrefElement.mockReturnValue(targetElement);
127+
targetElement.contains = vi.fn().mockReturnValue(false);
128+
129+
onContextMenuOutside(targetElement, mockHandler);
130+
131+
const mockEvent = new MouseEvent("contextmenu", { bubbles: true });
132+
Object.defineProperty(mockEvent, "target", {
133+
value: document.createElement("div"),
134+
});
135+
136+
capturedEventHandler?.(mockEvent);
137+
138+
expect(mockUnrefElement).toHaveBeenCalledWith(targetElement);
139+
expect(mockHandler).toHaveBeenCalledWith(mockEvent);
140+
});
141+
142+
describe("event target handling", () => {
143+
it("should handle event target as HTMLElement", () => {
144+
const targetElement = document.createElement("div");
145+
const eventTarget = document.createElement("span");
146+
const targetRef = ref(targetElement);
147+
148+
mockUnrefElement.mockReturnValue(targetElement);
149+
targetElement.contains = vi.fn().mockReturnValue(false);
150+
151+
onContextMenuOutside(targetRef, mockHandler);
152+
153+
const mockEvent = new MouseEvent("contextmenu", { bubbles: true });
154+
Object.defineProperty(mockEvent, "target", { value: eventTarget });
155+
156+
capturedEventHandler?.(mockEvent);
157+
158+
expect(targetElement.contains).toHaveBeenCalledWith(eventTarget);
159+
expect(mockHandler).toHaveBeenCalledWith(mockEvent);
160+
});
161+
162+
it("should handle nested elements correctly", () => {
163+
const targetElement = document.createElement("div");
164+
const childElement = document.createElement("span");
165+
const nestedElement = document.createElement("p");
166+
167+
targetElement.appendChild(childElement);
168+
childElement.appendChild(nestedElement);
169+
170+
const targetRef = ref(targetElement);
171+
172+
mockUnrefElement.mockReturnValue(targetElement);
173+
targetElement.contains = vi.fn().mockReturnValue(true);
174+
175+
onContextMenuOutside(targetRef, mockHandler);
176+
177+
const mockEvent = new MouseEvent("contextmenu", { bubbles: true });
178+
Object.defineProperty(mockEvent, "target", { value: nestedElement });
179+
180+
capturedEventHandler?.(mockEvent);
181+
182+
expect(targetElement.contains).toHaveBeenCalledWith(nestedElement);
183+
expect(mockHandler).not.toHaveBeenCalled();
184+
});
185+
});
186+
187+
describe("integration scenarios", () => {
188+
it("should work correctly when element is found but click is outside", () => {
189+
const container = document.createElement("div");
190+
const targetElement = document.createElement("div");
191+
const outsideElement = document.createElement("div");
192+
193+
container.appendChild(targetElement);
194+
container.appendChild(outsideElement);
195+
196+
const targetRef = ref(targetElement);
197+
198+
mockUnrefElement.mockReturnValue(targetElement);
199+
200+
onContextMenuOutside(targetRef, mockHandler);
201+
202+
const mockEvent = new MouseEvent("contextmenu", { bubbles: true });
203+
Object.defineProperty(mockEvent, "target", { value: outsideElement });
204+
205+
capturedEventHandler?.(mockEvent);
206+
207+
expect(mockHandler).toHaveBeenCalledWith(mockEvent);
208+
});
209+
210+
it("should work correctly when element is found and click is inside", () => {
211+
const container = document.createElement("div");
212+
const targetElement = document.createElement("div");
213+
const childElement = document.createElement("span");
214+
215+
container.appendChild(targetElement);
216+
targetElement.appendChild(childElement);
217+
218+
const targetRef = ref(targetElement);
219+
220+
mockUnrefElement.mockReturnValue(targetElement);
221+
222+
onContextMenuOutside(targetRef, mockHandler);
223+
224+
const mockEvent = new MouseEvent("contextmenu", { bubbles: true });
225+
Object.defineProperty(mockEvent, "target", { value: childElement });
226+
227+
capturedEventHandler?.(mockEvent);
228+
229+
expect(mockHandler).not.toHaveBeenCalled();
230+
});
231+
});
232+
});

packages/components/src/composables/index.ts

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
import onContextMenuOutside from "./onContextMenuOutside";
12
import useClickOutside from "./useClickOutside";
23
import useDropdownNavigation from "./useDropdownNavigation";
34
import useKeyPressedUntilMouseClick from "./useKeyPressedUntilMouseClick";
@@ -9,4 +10,9 @@ export * from "./useFileSizeFormatting";
910
export * from "./useBeforeUnload";
1011
export * from "./useAutoCloseOnCompletion";
1112

12-
export { useClickOutside, useDropdownNavigation, useKeyPressedUntilMouseClick };
13+
export {
14+
useClickOutside,
15+
onContextMenuOutside,
16+
useDropdownNavigation,
17+
useKeyPressedUntilMouseClick,
18+
};

0 commit comments

Comments
 (0)