Skip to content
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
118 changes: 104 additions & 14 deletions browser_tests/tests/vueNodes/interactions/canvas/pan.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,27 +27,22 @@ test.describe('Vue Nodes Canvas Pan', { tag: '@vue-nodes' }, () => {
}
)

test.describe('spacebar panning', () => {
test.beforeEach(async ({ comfyPage }) => {
await comfyPage.settings.setSetting(
'Comfy.Canvas.NavigationMode',
'standard'
)
await comfyPage.workflow.loadWorkflow('vueNodes/simple-triple')
})
test('spacebar panning', async ({ comfyPage, comfyMouse }) => {
await comfyPage.settings.setSetting(
'Comfy.Canvas.NavigationMode',
'standard'
)
await comfyPage.workflow.loadWorkflow('vueNodes/simple-triple')
const node = await comfyPage.vueNodes.getFixtureByTitle('KSampler')

test('Space + left-drag on a Vue node pans canvas', async ({
comfyPage,
comfyMouse
}) => {
const node = comfyPage.vueNodes.getNodeByTitle('KSampler')
await test.step('Space + click on a node starts a pan', async () => {
const offsetBefore = await comfyPage.canvasOps.getOffset()

await comfyPage.canvas.focus()
await comfyPage.page.keyboard.down('Space')
await expect.poll(() => comfyPage.canvasOps.isReadOnly()).toBe(true)
try {
await comfyMouse.dragElementBy(node, { x: 140, y: 90 })
await comfyMouse.dragElementBy(node.root, { x: -300, y: 0 })
} finally {
await comfyPage.page.keyboard.up('Space')
}
Expand All @@ -56,8 +51,103 @@ test.describe('Vue Nodes Canvas Pan', { tag: '@vue-nodes' }, () => {
.poll(() => comfyPage.canvasOps.getOffset())
.not.toEqual(offsetBefore)
})

await test.step('while dragging node, spacebar starts pan', async () => {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems like several steps...

await node.header.hover()
const offset1 = await comfyPage.canvasOps.getOffset()
await comfyPage.page.mouse.down()
await comfyPage.page.mouse.move(500, 500, { steps: 5 })
expect(await comfyPage.canvasOps.getOffset()).toEqual(offset1)
await comfyPage.page.keyboard.down('Space')
await comfyPage.page.mouse.move(400, 400, { steps: 5 })
await expect
.poll(() => comfyPage.canvasOps.getOffset())
.not.toEqual(offset1)
await comfyPage.page.keyboard.up('Space')
const offset2 = await comfyPage.canvasOps.getOffset()
await comfyPage.page.mouse.move(500, 500, { steps: 5 })
expect(await comfyPage.canvasOps.getOffset()).toEqual(offset2)
})
Comment thread
AustinMroz marked this conversation as resolved.
Outdated
})

test(
'Space in a focused text widget does not start canvas panning',
{ tag: ['@canvas', '@widget'] },
async ({ comfyPage }) => {
await comfyPage.workflow.loadWorkflow('inputs/string_input')
const input = comfyPage.vueNodes
.getWidgetByName('Node With String Input', 'string_input')
.first()

await input.focus()
await input.press('Space')

await expect
.poll(async () => [
await input.inputValue(),
await comfyPage.canvasOps.isReadOnly()
])
.toEqual([' ', false])
}
)

test(
'releasing the pointer during Space-pan ends the node drag',
{ tag: ['@canvas', '@node'] },
async ({ comfyPage }) => {
await comfyPage.workflow.loadWorkflow('vueNodes/simple-triple')
const node = await comfyPage.vueNodes.getFixtureByTitle('KSampler')
const [nodeRef] = await comfyPage.nodeOps.getNodeRefsByTitle('KSampler')
const headerBox = await node.header.boundingBox()
if (!nodeRef || !headerBox) throw new Error('KSampler is not rendered')
const start = {
x: headerBox.x + headerBox.width / 2,
y: headerBox.y + headerBox.height / 2
}
let mouseDown = false
let spaceDown = false

try {
await comfyPage.page.mouse.move(start.x, start.y)
await comfyPage.page.mouse.down()
mouseDown = true
await comfyPage.page.mouse.move(start.x + 40, start.y + 40, {
steps: 5
})
await comfyPage.page.keyboard.down('Space')
spaceDown = true
await expect.poll(() => comfyPage.canvasOps.isReadOnly()).toBe(true)
await comfyPage.page.mouse.move(start.x + 80, start.y + 80, {
steps: 5
})
await comfyPage.page.mouse.up()
mouseDown = false
await comfyPage.page.keyboard.up('Space')
spaceDown = false

const positionAfterRelease = [
...(await nodeRef.getProperty<[number, number]>('pos'))
]
const headerAfterRelease = await node.header.boundingBox()
if (!headerAfterRelease) throw new Error('KSampler is not rendered')
await comfyPage.page.mouse.move(
headerAfterRelease.x + 5,
headerAfterRelease.y + 5
)
await comfyPage.nextFrame()

await expect
.poll(async () => [
...(await nodeRef.getProperty<[number, number]>('pos'))
])
.toEqual(positionAfterRelease)
} finally {
if (mouseDown) await comfyPage.page.mouse.up()
if (spaceDown) await comfyPage.page.keyboard.up('Space')
}
}
)

test(
'@mobile Can pan with touch',
{ tag: '@screenshot' },
Expand Down
15 changes: 15 additions & 0 deletions src/components/graph/GraphCanvas.vue
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@
@pointerdown.capture="forwardPointerDownPanEvent"
@pointerup.capture="forwardPointerUpPanEvent"
@pointermove.capture="forwardPointerMovePanEvent"
@keydown.space="forwardSpaceKeyEvent"
>
<!-- Vue nodes rendered based on graph nodes -->
<LGraphNode
Expand Down Expand Up @@ -619,6 +620,20 @@ function forwardPointerUpPanEvent(e: PointerEvent) {
forwardPanEvent(e, isMiddleButtonEvent)
}

function forwardSpaceKeyEvent(e: KeyboardEvent) {
const target = e.target
if (
!layoutStore.isDraggingVueNodes.value ||
target instanceof HTMLInputElement ||
target instanceof HTMLTextAreaElement ||
target instanceof HTMLButtonElement ||
(target instanceof HTMLElement && target.isContentEditable)
Comment thread
coderabbitai[bot] marked this conversation as resolved.
)
return

comfyApp.canvas?.processKey(e)
}

function forwardPanEvent(
e: PointerEvent,
isMiddleInput: (event: PointerEvent) => boolean
Expand Down
26 changes: 26 additions & 0 deletions src/lib/litegraph/src/LGraphCanvas.linkDragAutoPan.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,32 @@ describe('LGraphCanvas link drag auto-pan', () => {
expect(canvas['_autoPan']).not.toBeNull()
})

it('resumes auto-pan after Space panning during a link drag', () => {
canvas.mouse[0] = 400
canvas.mouse[1] = 300
canvas.linkConnector.state.connectingTo = 'output'
canvas.pointer.isDown = true
startLinkDrag()
const autoPan = canvas['_autoPan']
if (!autoPan) throw new Error('Auto-pan controller was not created')
const stop = vi.spyOn(autoPan, 'stop')
const start = vi.spyOn(autoPan, 'start')
const updatePointer = vi.spyOn(autoPan, 'updatePointer')
const keydown = new KeyboardEvent('keydown', { key: ' ' })
const keyup = new KeyboardEvent('keyup', { key: ' ' })
Object.defineProperty(keydown, 'target', { value: canvasElement })
Object.defineProperty(keyup, 'target', { value: canvasElement })

canvas.processKey(keydown)
canvas.mouse[0] = 450
canvas.mouse[1] = 350
canvas.processKey(keyup)

expect(stop).toHaveBeenCalledOnce()
expect(updatePointer).toHaveBeenLastCalledWith(450, 350)
expect(start).toHaveBeenCalledOnce()
})
Comment thread
coderabbitai[bot] marked this conversation as resolved.

it('keeps graph_mouse consistent with offset after auto-pan', () => {
canvas.mouse[0] = 5
canvas.mouse[1] = 300
Expand Down
8 changes: 8 additions & 0 deletions src/lib/litegraph/src/LGraphCanvas.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4008,6 +4008,7 @@ export class LGraphCanvas implements CustomEventDispatcher<LGraphCanvasEventMap>
if (e.key === ' ') {
// space
this.read_only = true
this._autoPan?.stop()
Comment thread
coderabbitai[bot] marked this conversation as resolved.
if (this._previously_dragging_canvas === null) {
this._previously_dragging_canvas = this.dragging_canvas
}
Expand Down Expand Up @@ -4037,6 +4038,13 @@ export class LGraphCanvas implements CustomEventDispatcher<LGraphCanvasEventMap>
this.dragging_canvas =
(this._previously_dragging_canvas ?? false) && this.pointer.isDown
this._previously_dragging_canvas = null
if (
this.pointer.isDown &&
(this.isDragging || this.linkConnector.isConnecting)
) {
this._autoPan?.updatePointer(this.mouse[0], this.mouse[1])
this._autoPan?.start()
}
}

for (const node of Object.values(this.selected_nodes)) {
Expand Down
12 changes: 6 additions & 6 deletions src/renderer/core/canvas/useCanvasInteractions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -148,12 +148,12 @@ export function useCanvasInteractions() {
return
}

// Create new event with same properties
const EventConstructor = event.constructor as
| typeof MouseEvent
| typeof PointerEvent
const newEvent = new EventConstructor(event.type, event)
canvasEl.dispatchEvent(newEvent)
if (event instanceof PointerEvent) {
canvasEl.dispatchEvent(new PointerEvent(event.type, event))
return
}

canvasEl.dispatchEvent(new MouseEvent(event.type, event))
}

return {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,9 @@ export function useNodePointerInteractions(
const canHandlePointer = shouldHandleNodePointerEvents.value
if (!canHandlePointer) {
forwardEventToCanvas(event)
if (hasDraggingStarted || layoutStore.isDraggingVueNodes.value) {
safeDragEnd(event)
}
return
}
const wasDragging = layoutStore.isDraggingVueNodes.value
Expand Down
14 changes: 14 additions & 0 deletions src/renderer/extensions/vueNodes/layout/useNodeDrag.ts
Original file line number Diff line number Diff line change
Expand Up @@ -196,6 +196,20 @@ function useNodeDragIndividual() {
if (!dragStartPos || !dragStartMouse) {
return
}
if (canvasStore.isReadOnly) {
autoPan?.stop()
const canvas = canvasStore.getCanvas()
const delta = [event.clientX - lastPointerX, event.clientY - lastPointerY]

canvas.ds.offset[0] += delta[0] / canvas.ds.scale
canvas.ds.offset[1] += delta[1] / canvas.ds.scale
canvas.setDirty(true, true)
lastPointerX = event.clientX
lastPointerY = event.clientY
dragStartMouse.x += delta[0]
dragStartMouse.y += delta[1]
Comment on lines +204 to +210

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The fact that we have to update this many things at once is a pretty big code smell, IMO.
(Not for this PR, just whining)

return
Comment thread
AustinMroz marked this conversation as resolved.
}

// Throttle position updates using requestAnimationFrame for better performance
if (rafId !== null) return // Skip if frame already scheduled
Expand Down
Loading