From 0186ed1961b8cd76c4a07322a7a64eb802a83e47 Mon Sep 17 00:00:00 2001 From: Adam Obuchowicz Date: Thu, 2 May 2024 13:17:13 +0200 Subject: [PATCH] Fix table viz context menu (#9820) Fixes #9700 [Screencast from 2024-04-30 15-09-17.webm](https://github.com/enso-org/enso/assets/3919101/a11ae875-7e2f-4c84-8d91-33aa1ca8bdbd) # Important Notes During implementation, I discovered a bug that the context menu wasn't hidden by click at background (or node) - all because of preventDefault, as it turned out. --- .../src/components/VisualizationContainer.vue | 6 ++--- .../visualizations/TableVisualization.vue | 2 ++ .../composables/__tests__/selection.test.ts | 8 +++++-- app/gui2/src/composables/events.ts | 22 +++---------------- app/gui2/src/util/__tests__/shortcuts.test.ts | 8 ++++++- app/gui2/src/util/shortcuts.ts | 5 ++++- 6 files changed, 25 insertions(+), 26 deletions(-) diff --git a/app/gui2/src/components/VisualizationContainer.vue b/app/gui2/src/components/VisualizationContainer.vue index 1ffc2d8f1c..4e97c74258 100644 --- a/app/gui2/src/components/VisualizationContainer.vue +++ b/app/gui2/src/components/VisualizationContainer.vue @@ -104,9 +104,9 @@ const nodeShortType = computed(() => @pointerup.stop @click.stop > -
-
-
+
+
+
rowCount.value >= 1000) @@ -410,6 +411,7 @@ onMounted(() => { origValidateLicense.call(this) } } + // TODO: consider using Vue component instead: https://ag-grid.com/vue-data-grid/getting-started/ new Grid(tableNode.value!, agGridOptions.value) updateColumnWidths() }) diff --git a/app/gui2/src/composables/__tests__/selection.test.ts b/app/gui2/src/composables/__tests__/selection.test.ts index 1377becba6..cd2f4b08ac 100644 --- a/app/gui2/src/composables/__tests__/selection.test.ts +++ b/app/gui2/src/composables/__tests__/selection.test.ts @@ -4,7 +4,7 @@ import { assert } from '@/util/assert' import { Rect } from '@/util/data/rect' import { Vec2 } from '@/util/data/vec2' import { isPointer, pointerButtonToEventInfo, type BindingInfo } from '@/util/shortcuts' -import { expect, test, vi } from 'vitest' +import { beforeAll, expect, test, vi } from 'vitest' import { proxyRefs, ref, type Ref } from 'vue' function selectionWithMockData(sceneMousePos?: Ref) { @@ -104,7 +104,7 @@ test.each` dragCase(new Vec2(area.right, area.top), new Vec2(area.left, area.bottom)) }) -// There is no PointerEvent class in jsdom (yet). +// See https://github.com/thymikee/jest-preset-angular/issues/245#issuecomment-576296325 class MockPointerEvent extends MouseEvent { readonly pointerId: number constructor(type: string, options: MouseEventInit & { currentTarget?: Element | undefined }) { @@ -116,6 +116,10 @@ class MockPointerEvent extends MouseEvent { } } +beforeAll(() => { + ;(window as any).PointerEvent = MockPointerEvent +}) + function mockPointerEvent(type: string, pos: Vec2, binding: BindingInfo): PointerEvent { const modifiersSet = new Set(binding.modifiers) assert(isPointer(binding.key)) diff --git a/app/gui2/src/composables/events.ts b/app/gui2/src/composables/events.ts index 80a0aa72d2..afdcba42ce 100644 --- a/app/gui2/src/composables/events.ts +++ b/app/gui2/src/composables/events.ts @@ -305,7 +305,7 @@ export function usePointer( if (trackedElement != null && initialGrabPos != null && lastPos != null) { if (handler(computePosition(e, initialGrabPos, lastPos), e, 'stop') !== false) { - e.preventDefault() + e.stopImmediatePropagation() } lastPos = null @@ -317,7 +317,7 @@ export function usePointer( function doMove(e: PointerEvent) { if (trackedElement != null && initialGrabPos != null && lastPos != null) { if (handler(computePosition(e, initialGrabPos, lastPos), e, 'move') !== false) { - e.preventDefault() + e.stopImmediatePropagation() } lastPos = new Vec2(e.clientX, e.clientY) } @@ -339,7 +339,7 @@ export function usePointer( initialGrabPos = new Vec2(e.clientX, e.clientY) lastPos = initialGrabPos if (handler(computePosition(e, initialGrabPos, lastPos), e, 'start') !== false) { - e.preventDefault() + e.stopImmediatePropagation() } } }, @@ -362,24 +362,8 @@ export function usePointer( }, } - const stopEvents = { - pointerdown(e: PointerEvent) { - e.stopImmediatePropagation() - events.pointerdown(e) - }, - pointerup(e: PointerEvent) { - e.stopImmediatePropagation() - events.pointerup(e) - }, - pointermove(e: PointerEvent) { - e.stopImmediatePropagation() - events.pointermove(e) - }, - } - return proxyRefs({ events, - stop: { events: stopEvents }, dragging, }) } diff --git a/app/gui2/src/util/__tests__/shortcuts.test.ts b/app/gui2/src/util/__tests__/shortcuts.test.ts index cd15c9ea00..8bb19d05af 100644 --- a/app/gui2/src/util/__tests__/shortcuts.test.ts +++ b/app/gui2/src/util/__tests__/shortcuts.test.ts @@ -3,7 +3,13 @@ import { defineKeybinds, normalizedKeyboardSegmentLookup, } from '@/util/shortcuts' -import { expect, test, vi, type MockInstance } from 'vitest' +import { beforeAll, expect, test, vi, type MockInstance } from 'vitest' + +// See https://github.com/thymikee/jest-preset-angular/issues/245#issuecomment-576296325 +class MockPointerEvent extends MouseEvent {} +beforeAll(() => { + ;(window as any).PointerEvent = MockPointerEvent +}) test.each([ { keybind: 'A', expected: { modifiers: [], key: 'A' } }, diff --git a/app/gui2/src/util/shortcuts.ts b/app/gui2/src/util/shortcuts.ts index 4ab208bd87..d07a83f57c 100644 --- a/app/gui2/src/util/shortcuts.ts +++ b/app/gui2/src/util/shortcuts.ts @@ -415,7 +415,10 @@ export function defineKeybinds< } if (handled && stopAndPrevent) { event.stopImmediatePropagation() - event.preventDefault() + // We don't prevent default on PointerEvents, because it may prevent emitting + // mousedown/mouseup events, on which external libraries may rely (like AGGrid for hiding + // context menu). + if (!(event instanceof PointerEvent)) event.preventDefault() } return handled }