From 5fa1eccbee01df8df37af5ebcc9e6736b5908fea Mon Sep 17 00:00:00 2001 From: Adam Obuchowicz Date: Fri, 23 Feb 2024 10:55:16 +0100 Subject: [PATCH 1/3] Proper fix for editing node --- .../src/components/GraphEditor/GraphNode.vue | 2 +- app/gui2/src/util/shortcuts.ts | 29 +++++++++++++++---- 2 files changed, 24 insertions(+), 7 deletions(-) diff --git a/app/gui2/src/components/GraphEditor/GraphNode.vue b/app/gui2/src/components/GraphEditor/GraphNode.vue index 356718b4977a..37bf7e1c3a2d 100644 --- a/app/gui2/src/components/GraphEditor/GraphNode.vue +++ b/app/gui2/src/components/GraphEditor/GraphNode.vue @@ -172,7 +172,7 @@ const dragPointer = usePointer((pos, event, type) => { startEvent != null && pos.absolute.distanceSquared(startPos) <= MAXIMUM_CLICK_DISTANCE_SQ ) { - nodeSelection?.handleSelectionOf(startEvent, new Set([nodeId.value])) + nodeSelection?.handleSelectionOf(event, new Set([nodeId.value])) handleNodeClick(event) menuVisible.value = MenuState.Partial } diff --git a/app/gui2/src/util/shortcuts.ts b/app/gui2/src/util/shortcuts.ts index 393835625eb2..198f72a651f0 100644 --- a/app/gui2/src/util/shortcuts.ts +++ b/app/gui2/src/util/shortcuts.ts @@ -47,7 +47,7 @@ function modifierFlagsForEvent(event: EventWithModifiers): ModifierFlags { (event.metaKey ? RAW_MODIFIER_FLAG.Meta : 0)) as ModifierFlags } -/** These values MUST match the flags on `MouseEvent#button`. +/** These values MUST match the flags on `MouseEvent#buttons`. * See https://developer.mozilla.org/en-US/docs/Web/API/MouseEvent/buttons */ const POINTER_BUTTON_FLAG = { PointerMain: 1 << 0, @@ -57,6 +57,27 @@ const POINTER_BUTTON_FLAG = { PointerForward: 1 << 4, } satisfies Record as Record +/** + * Mapping from the MouseEvent's `button` field to PointerButtonFlags. + * + * No, it is not as simple as (1 << event.button) as PointerButtonFlags; compare + * https://developer.mozilla.org/en-US/docs/Web/API/MouseEvent/buttons with + * https://developer.mozilla.org/en-US/docs/Web/API/MouseEvent/button + */ +const flagsOfButtonField = [ + POINTER_BUTTON_FLAG.PointerMain, + POINTER_BUTTON_FLAG.PointerAux, + POINTER_BUTTON_FLAG.PointerSecondary, + POINTER_BUTTON_FLAG.PointerBack, + POINTER_BUTTON_FLAG.PointerForward, +] + +function buttonFlagsForEvent(event: MouseEvent): PointerButtonFlags { + // event.buttons keeps information about buttons being pressed, but in case of `click` or + // `pointerup` events we also want to know what buttons were just released. + return (event.buttons | (flagsOfButtonField[event.button] ?? 0)) as PointerButtonFlags +} + // ========================== // === Autocomplete types === // ========================== @@ -333,11 +354,7 @@ export function defineKeybinds< const keybinds = event instanceof KeyboardEvent ? keyboardShortcuts[event.key.toLowerCase() as Key_]?.[eventModifierFlags] - : mouseShortcuts[ - (event.type === 'click' // `click` events don't have 'buttons' field initialized. - ? POINTER_BUTTON_FLAG.PointerMain - : event.buttons) as PointerButtonFlags - ]?.[eventModifierFlags] + : mouseShortcuts[buttonFlagsForEvent(event)]?.[eventModifierFlags] let handle = handlers[DefaultHandler] if (keybinds != null) { for (const bindingName in handlers) { From 51714749fef8a0ad1b332a9dee6707b84434ea8d Mon Sep 17 00:00:00 2001 From: Adam Obuchowicz Date: Fri, 23 Feb 2024 11:32:44 +0100 Subject: [PATCH 2/3] Add tests --- app/gui2/e2e/componentBrowser.spec.ts | 33 +++++++++++++++++++ app/gui2/e2e/locate.ts | 21 +++++++++--- .../src/components/GraphEditor/GraphNode.vue | 2 +- 3 files changed, 51 insertions(+), 5 deletions(-) diff --git a/app/gui2/e2e/componentBrowser.spec.ts b/app/gui2/e2e/componentBrowser.spec.ts index ccebe783e672..181b8990d716 100644 --- a/app/gui2/e2e/componentBrowser.spec.ts +++ b/app/gui2/e2e/componentBrowser.spec.ts @@ -148,3 +148,36 @@ test('Filtering list', async ({ page }) => { const highlighted = locate.componentBrowserEntry(page).locator('.component-label-segment.match') await expect(highlighted).toHaveText(['re', '_te']) }) + +test('Editing existing nodes', async ({ page }) => { + await actions.goToGraph(page) + const node = locate.graphNodeByBinding(page, 'data') + const ADDED_PATH = '"/home/enso/Input.txt"' + + // Start node editing + await locate.graphNodeIcon(node).click({ modifiers: ['Control'] }) + await expect(locate.componentBrowser(page)).toBeVisible() + const input = locate.componentBrowserInput(page).locator('input') + await expect(input).toHaveValue('Data.read') + + // Add argument and accept + await page.keyboard.press('End') + await input.pressSequentially(` ${ADDED_PATH}`) + await expect(input).toHaveValue(`Data.read ${ADDED_PATH}`) + await page.keyboard.press('Enter') + await expect(locate.componentBrowser(page)).not.toBeVisible() + await expect(node.locator('.WidgetToken')).toHaveText(['Data', '.', 'read']) + await expect(node.locator('.WidgetText input')).toHaveValue(ADDED_PATH) + + // Edit again, using "edit" button + await locate.graphNodeIcon(node).click() + await node.locator('.CircularMenu .slot6').click() + await expect(locate.componentBrowser(page)).toBeVisible() + await expect(input).toHaveValue(`Data.read ${ADDED_PATH}`) + for (let i = 0; i < ADDED_PATH.length; ++i) await page.keyboard.press('Backspace') + await expect(input).toHaveValue('Data.read ') + await page.keyboard.press('Enter') + await expect(locate.componentBrowser(page)).not.toBeVisible() + await expect(node.locator('.WidgetToken')).toHaveText(['Data', '.', 'read']) + await expect(node.locator('.WidgetText')).not.toBeVisible() +}) diff --git a/app/gui2/e2e/locate.ts b/app/gui2/e2e/locate.ts index 190c0303e8c6..1dee7549ab66 100644 --- a/app/gui2/e2e/locate.ts +++ b/app/gui2/e2e/locate.ts @@ -113,6 +113,23 @@ export function exitFullscreenButton(page: Locator | Page) { export const toggleFullscreenButton = or(enterFullscreenButton, exitFullscreenButton) +// === Nodes === + +declare const nodeLocatorBrand: unique symbol +type Node = Locator & { [nodeLocatorBrand]: never } + +export function graphNode(page: Page | Locator): Node { + return page.locator('.GraphNode') as Node +} +export function graphNodeByBinding(page: Locator | Page, binding: string): Node { + return graphNode(page).filter({ + has: page.locator('.binding').and(page.getByText(binding)), + }) as Node +} +export function graphNodeIcon(node: Node) { + return node.locator('.icon') +} + // === Data locators === type SanitizeClassName = T extends `${infer A}.${infer B}` @@ -128,10 +145,6 @@ function componentLocator(className: SanitizeClassName) { } export const graphEditor = componentLocator('GraphEditor') -export const graphNode = componentLocator('GraphNode') -export function graphNodeByBinding(page: Locator | Page, binding: string) { - return graphNode(page).filter({ has: page.locator('.binding').and(page.getByText(binding)) }) -} // @ts-expect-error export const anyVisualization = componentLocator('GraphVisualization > *') export const circularMenu = componentLocator('CircularMenu') diff --git a/app/gui2/src/components/GraphEditor/GraphNode.vue b/app/gui2/src/components/GraphEditor/GraphNode.vue index 37bf7e1c3a2d..b72ffc74373a 100644 --- a/app/gui2/src/components/GraphEditor/GraphNode.vue +++ b/app/gui2/src/components/GraphEditor/GraphNode.vue @@ -256,7 +256,7 @@ const nodeEditHandler = nodeEditBindings.handler({ }) function startEditingNode(position: Vec2 | undefined) { - let sourceOffset = 0 + let sourceOffset = props.node.rootSpan.code().length if (position != null) { let domNode, domOffset if ((document as any).caretPositionFromPoint) { From 66f42512e155cf01fe2369eaf9d749a687261433 Mon Sep 17 00:00:00 2001 From: Adam Obuchowicz Date: Fri, 23 Feb 2024 14:45:17 +0100 Subject: [PATCH 3/3] Use data-testid for locaing button --- app/gui2/e2e/componentBrowser.spec.ts | 2 +- app/gui2/src/components/CircularMenu.vue | 7 ++++++- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/app/gui2/e2e/componentBrowser.spec.ts b/app/gui2/e2e/componentBrowser.spec.ts index 181b8990d716..ae7093605599 100644 --- a/app/gui2/e2e/componentBrowser.spec.ts +++ b/app/gui2/e2e/componentBrowser.spec.ts @@ -171,7 +171,7 @@ test('Editing existing nodes', async ({ page }) => { // Edit again, using "edit" button await locate.graphNodeIcon(node).click() - await node.locator('.CircularMenu .slot6').click() + await node.getByTestId('edit-button').click() await expect(locate.componentBrowser(page)).toBeVisible() await expect(input).toHaveValue(`Data.read ${ADDED_PATH}`) for (let i = 0; i < ADDED_PATH.length; ++i) await page.keyboard.press('Backspace') diff --git a/app/gui2/src/components/CircularMenu.vue b/app/gui2/src/components/CircularMenu.vue index b5dcb5f9124a..b34d3ae9d781 100644 --- a/app/gui2/src/components/CircularMenu.vue +++ b/app/gui2/src/components/CircularMenu.vue @@ -31,7 +31,12 @@ const emit = defineEmits<{ :modelValue="props.isVisualizationVisible" @update:modelValue="emit('update:isVisualizationVisible', $event)" /> - +