Fix area select regression

app/gui2/e2e/actions.ts

@@ -13,6 +13,11 @@ export async function goToGraph(page: Page) {
   await expect(page.locator('.App')).toBeVisible()
   // Wait until nodes are loaded.
   await customExpect.toExist(locate.graphNode(page))
+  // Wait for position initialization
+  await expect(locate.graphNode(page).first()).toHaveCSS(
+    'transform',
+    'matrix(1, 0, 0, 1, -16, -16)',
+  )
 }
 
 // =================

app/gui2/e2e/customExpect.ts

@@ -11,3 +11,9 @@ export function toExist(locator: Locator) {
 export function toBeSelected(locator: Locator) {
   return expect(locator).toHaveClass(/(?<=^| )selected(?=$| )/)
 }
+
+export module not {
+  export function toBeSelected(locator: Locator) {
+    return expect(locator).not.toHaveClass(/(?<=^| )selected(?=$| )/)
+  }
+}

app/gui2/e2e/locate.ts

@@ -116,7 +116,7 @@ export const toggleFullscreenButton = or(enterFullscreenButton, exitFullscreenBu
 // === Nodes ===
 
 declare const nodeLocatorBrand: unique symbol
-type Node = Locator & { [nodeLocatorBrand]: never }
+export type Node = Locator & { [nodeLocatorBrand]: never }
 
 export function graphNode(page: Page | Locator): Node {
   return page.locator('.GraphNode') as Node

app/gui2/e2e/selectingNodes.spec.ts

@@ -0,0 +1,58 @@
+import { expect, test } from '@playwright/test'
+import assert from 'assert'
+import { nextTick } from 'vue'
+import * as actions from './actions'
+import * as customExpect from './customExpect'
+import * as locate from './locate'
+
+test('Selecting nodes by click', async ({ page }) => {
+  await actions.goToGraph(page)
+  const node1 = locate.graphNodeByBinding(page, 'five')
+  const node2 = locate.graphNodeByBinding(page, 'ten')
+  await customExpect.not.toBeSelected(node1)
+  await customExpect.not.toBeSelected(node2)
+
+  await locate.graphNodeIcon(node1).click()
+  await customExpect.toBeSelected(node1)
+  await customExpect.not.toBeSelected(node2)
+
+  await locate.graphNodeIcon(node2).click()
+  await customExpect.not.toBeSelected(node1)
+  await customExpect.toBeSelected(node2)
+
+  await page.waitForTimeout(600) // Avoid double clicks
+  await locate.graphNodeIcon(node1).click({ modifiers: ['Shift'] })
+  await customExpect.toBeSelected(node1)
+  await customExpect.toBeSelected(node2)
+
+  await locate.graphNodeIcon(node2).click()
+  await customExpect.not.toBeSelected(node1)
+  await customExpect.toBeSelected(node2)
+
+  await page.mouse.click(200, 200)
+  await customExpect.not.toBeSelected(node1)
+  await customExpect.not.toBeSelected(node2)
+})
+
+test('Selecting nodes by area drag', async ({ page }) => {
+  await actions.goToGraph(page)
+  const node1 = locate.graphNodeByBinding(page, 'five')
+  const node2 = locate.graphNodeByBinding(page, 'ten')
+  await customExpect.not.toBeSelected(node1)
+  await customExpect.not.toBeSelected(node2)
+
+  const node1BBox = await node1.locator('.selection').boundingBox()
+  const node2BBox = await node2.boundingBox()
+  assert(node1BBox)
+  assert(node2BBox)
+  await page.mouse.move(node1BBox.x - 50, node1BBox.y - 50)
+  await page.mouse.down()
+  await page.mouse.move(node1BBox.x - 49, node1BBox.y - 49)
+  await expect(page.locator('.SelectionBrush')).toBeVisible()
+  await page.mouse.move(node2BBox.x + node2BBox.width, node2BBox.y + node2BBox.height)
+  await customExpect.toBeSelected(node1)
+  await customExpect.toBeSelected(node2)
+  await page.mouse.up()
+  await customExpect.toBeSelected(node1)
+  await customExpect.toBeSelected(node2)
+})

app/gui2/src/composables/__tests__/selection.test.ts

@@ -0,0 +1,126 @@
+import { Rect } from '@/util/data/rect'
+import { Vec2 } from '@/util/data/vec2'
+import { expect, test } from 'vitest'
+import { proxyRefs, ref, type Ref } from 'vue'
+import { useSelection } from '../selection'
+
+function selectionWithMockData(sceneMousePos?: Ref<Vec2>) {
+  const rects = new Map()
+  rects.set(1, Rect.FromBounds(1, 1, 10, 10))
+  rects.set(2, Rect.FromBounds(20, 1, 30, 10))
+  rects.set(3, Rect.FromBounds(1, 20, 10, 30))
+  rects.set(4, Rect.FromBounds(20, 20, 30, 30))
+  const selection = useSelection(
+    proxyRefs({ sceneMousePos: sceneMousePos ?? ref(Vec2.Zero), scale: 1 }),
+    rects,
+    0,
+  )
+  selection.setSelection(new Set([1, 2]))
+  return selection
+}
+
+// TODO[ao]: We should read the modifiers from bindings.ts, but I don't know how yet.
+
+test.each`
+  click | modifiers                  | expected
+  ${1}  | ${[]}                      | ${[1]}
+  ${3}  | ${[]}                      | ${[3]}
+  ${1}  | ${['Shift']}               | ${[2]}
+  ${3}  | ${['Shift']}               | ${[1, 2, 3]}
+  ${1}  | ${['Mod', 'Shift']}        | ${[1, 2]}
+  ${3}  | ${['Mod', 'Shift']}        | ${[1, 2, 3]}
+  ${1}  | ${['Mod', 'Shift']}        | ${[1, 2]}
+  ${3}  | ${['Mod', 'Shift']}        | ${[1, 2, 3]}
+  ${1}  | ${['Alt', 'Shift']}        | ${[2]}
+  ${3}  | ${['Alt', 'Shift']}        | ${[1, 2]}
+  ${1}  | ${['Mod', 'Alt', 'Shift']} | ${[2]}
+  ${3}  | ${['Mod', 'Alt', 'Shift']} | ${[1, 2, 3]}
+`('Selection by single click at $click with $modifiers', ({ click, modifiers, expected }) => {
+  const selection = selectionWithMockData()
+  // Position is zero, because this method should not depend on click position
+  selection.handleSelectionOf(mockPointerEvent('click', Vec2.Zero, modifiers), new Set([click]))
+  expect(Array.from(selection.selected)).toEqual(expected)
+})
+
+const areas: Record<string, Rect> = {
+  left: Rect.FromBounds(0, 0, 10, 30),
+  right: Rect.FromBounds(20, 0, 30, 30),
+  top: Rect.FromBounds(0, 0, 30, 10),
+  bottom: Rect.FromBounds(0, 20, 30, 30),
+  all: Rect.FromBounds(0, 0, 30, 30),
+}
+
+test.each`
+  areaId      | modifiers                  | expected
+  ${'left'}   | ${[]}                      | ${[1, 3]}
+  ${'right'}  | ${[]}                      | ${[2, 4]}
+  ${'top'}    | ${[]}                      | ${[1, 2]}
+  ${'bottom'} | ${[]}                      | ${[3, 4]}
+  ${'all'}    | ${[]}                      | ${[1, 2, 3, 4]}
+  ${'left'}   | ${['Shift']}               | ${[1, 2, 3]}
+  ${'right'}  | ${['Shift']}               | ${[1, 2, 4]}
+  ${'top'}    | ${['Shift']}               | ${[]}
+  ${'bottom'} | ${['Shift']}               | ${[1, 2, 3, 4]}
+  ${'all'}    | ${['Shift']}               | ${[1, 2, 3, 4]}
+  ${'left'}   | ${['Mod', 'Shift']}        | ${[1, 2, 3]}
+  ${'right'}  | ${['Mod', 'Shift']}        | ${[1, 2, 4]}
+  ${'top'}    | ${['Mod', 'Shift']}        | ${[1, 2]}
+  ${'bottom'} | ${['Mod', 'Shift']}        | ${[1, 2, 3, 4]}
+  ${'all'}    | ${['Mod', 'Shift']}        | ${[1, 2, 3, 4]}
+  ${'left'}   | ${['Alt', 'Shift']}        | ${[2]}
+  ${'right'}  | ${['Alt', 'Shift']}        | ${[1]}
+  ${'top'}    | ${['Alt', 'Shift']}        | ${[]}
+  ${'bottom'} | ${['Alt', 'Shift']}        | ${[1, 2]}
+  ${'all'}    | ${['Alt', 'Shift']}        | ${[]}
+  ${'left'}   | ${['Mod', 'Alt', 'Shift']} | ${[2, 3]}
+  ${'right'}  | ${['Mod', 'Alt', 'Shift']} | ${[1, 4]}
+  ${'top'}    | ${['Mod', 'Alt', 'Shift']} | ${[]}
+  ${'bottom'} | ${['Mod', 'Alt', 'Shift']} | ${[1, 2, 3, 4]}
+  ${'all'}    | ${['Mod', 'Alt', 'Shift']} | ${[3, 4]}
+`('Selection by dragging $area area with $modifiers', ({ areaId, modifiers, expected }) => {
+  const area = areas[areaId]!
+  const dragCase = (start: Vec2, stop: Vec2) => {
+    const mousePos = ref(start)
+    const selection = selectionWithMockData(mousePos)
+
+    selection.events.pointerdown(mockPointerEvent('pointerdown', mousePos.value, modifiers))
+    selection.events.pointermove(mockPointerEvent('pointermove', mousePos.value, modifiers))
+    mousePos.value = stop
+    selection.events.pointermove(mockPointerEvent('pointermove', mousePos.value, modifiers))
+    expect(selection.selected).toEqual(new Set(expected))
+    selection.events.pointerdown(mockPointerEvent('pointerup', mousePos.value, modifiers))
+    expect(selection.selected).toEqual(new Set(expected))
+  }
+
+  // We should select same set of nodes, regardless of drag direction
+  dragCase(new Vec2(area.left, area.top), new Vec2(area.right, area.bottom))
+  dragCase(new Vec2(area.right, area.bottom), new Vec2(area.left, area.top))
+  dragCase(new Vec2(area.left, area.bottom), new Vec2(area.right, area.top))
+  dragCase(new Vec2(area.right, area.top), new Vec2(area.left, area.bottom))
+})
+
+class MockPointerEvent extends MouseEvent {
+  currentTarget: EventTarget | null
+  pointerId: number
+  constructor(type: string, options: MouseEventInit & { currentTarget?: Element | undefined }) {
+    super(type, options)
+    this.currentTarget = options.currentTarget ?? null
+    this.pointerId = 4
+  }
+}
+
+function mockPointerEvent(type: string, pos: Vec2, modifiers: string[]): PointerEvent {
+  const modifiersSet = new Set(modifiers)
+  const event = new MockPointerEvent(type, {
+    altKey: modifiersSet.has('Alt'),
+    ctrlKey: modifiersSet.has('Mod'),
+    shiftKey: modifiersSet.has('Shift'),
+    metaKey: modifiersSet.has('Meta'),
+    clientX: pos.x,
+    clientY: pos.y,
+    button: 0,
+    buttons: 1,
+    currentTarget: document.createElement('div'),
+  }) as PointerEvent
+  return event
+}

app/gui2/src/composables/selection.ts

@@ -7,11 +7,11 @@ import type { PortId } from '@/providers/portInfo.ts'
 import { type NodeId } from '@/stores/graph'
 import type { Rect } from '@/util/data/rect'
 import type { Vec2 } from '@/util/data/vec2'
-import { computed, proxyRefs, reactive, ref, shallowRef } from 'vue'
+import { computed, proxyRefs, reactive, ref, shallowRef, watch, type Ref } from 'vue'
 
 export type SelectionComposable<T> = ReturnType<typeof useSelection<T>>
 export function useSelection<T>(
-  navigator: NavigatorComposable,
+  navigator: { sceneMousePos: Vec2 | null; scale: number },
   elementRects: Map<T, Rect>,
   margin: number,
   callbacks: {
@@ -126,13 +126,19 @@ export function useSelection<T>(
   const pointer = usePointer((_pos, event, eventType) => {
     if (eventType === 'start') {
       readInitiallySelected()
-    } else if (pointer.dragging && anchor.value == null) {
-      anchor.value = navigator.sceneMousePos?.copy()
+    } else if (pointer.dragging) {
+      if (anchor.value == null) {
+        anchor.value = navigator.sceneMousePos?.copy()
+      }
+      selectionEventHandler(event)
     } else if (eventType === 'stop') {
+      if (anchor.value == null) {
+        // If there was no drag, we want to handle "clicking-off" selected nodes.
+        selectionEventHandler(event)
+      }
       anchor.value = undefined
       initiallySelected.clear()
     }
-    selectionEventHandler(event)
   })
 
   return proxyRefs({
@@ -154,6 +160,20 @@ export function useSelection<T>(
 
 function countCommonInSets(a: Set<unknown>, b: Set<unknown>): number {
   let count = 0
-  for (const item in a) count += +b.has(item)
+  for (const item of a) count += +b.has(item)
   return count
 }
+
+if (import.meta.vitest) {
+  const { test, expect } = import.meta.vitest
+  test.each`
+    left         | right        | expected
+    ${[]}        | ${[]}        | ${0}
+    ${[3]}       | ${[]}        | ${0}
+    ${[]}        | ${[3]}       | ${0}
+    ${[3]}       | ${[3]}       | ${1}
+    ${[1, 2, 3]} | ${[2, 3, 4]} | ${2}
+  `('Count common in sets $left and $right', ({ left, right, expected }) => {
+    expect(countCommonInSets(new Set(left), new Set(right))).toBe(expected)
+  })
+}