Proper fix for editing node (#9153)

The replacement for #9144

We try to match bindings using "buttons" field, but it in fact does not contain the just-released buttons.

# Important Notes
I added tests for editing node. Also, I decided that the cursor position should be at the end of line when starting editing by clicking at "edit" icon.

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.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')
+  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()
+})

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 string> = T extends `${infer A}.${infer B}`
@@ -128,10 +145,6 @@ function componentLocator<T extends string>(className: SanitizeClassName<T>) {
 }
 
 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')

app/gui2/src/components/CircularMenu.vue

@@ -31,7 +31,12 @@ const emit = defineEmits<{
       :modelValue="props.isVisualizationVisible"
       @update:modelValue="emit('update:isVisualizationVisible', $event)"
     />
-    <SvgIcon name="edit" class="icon-container button slot6" @click.stop="emit('startEditing')" />
+    <SvgIcon
+      name="edit"
+      class="icon-container button slot6"
+      data-testid="edit-button"
+      @click.stop="emit('startEditing')"
+    />
     <ToggleIcon
       :icon="props.isOutputContextEnabledGlobally ? 'no_auto_replay' : 'auto_replay'"
       class="icon-container button slot7"

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
       }
@@ -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) {

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<Pointer, number> as Record<Pointer, PointerButtonFlags>
 
+/**
+ * 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) {