Compare commits

...

5 Commits

Author SHA1 Message Date
bymyself
16b02e5326 address review feedback with minimal safe changes
- Replace type cast with proper type predicate in VueNodeHelpers getNodeIds
- Update comment to clarify shift key behavior in shouldForwardToCanvas

All review suggestions implemented while preserving working delete key functionality.
2025-09-13 22:10:06 -07:00
bymyself
aca3ede241 restore working test structure - fix VueNodeHelpers to match initial working commit
Tests were failing because helper methods were modified. Restore exact structure
that was working in initial commit, then we can add minimal review improvements.
2025-09-13 21:52:23 -07:00
bymyself
b56d5b781a add unit test for keybinding service event forwarding 2025-09-09 20:59:29 -07:00
bymyself
a850beda86 add playwright test for deletion and selection with vue nodes 2025-09-09 20:59:08 -07:00
bymyself
38bca0e134 fix delete hotkey with vue nodes 2025-09-09 20:58:12 -07:00
5 changed files with 464 additions and 0 deletions

View File

@@ -12,6 +12,7 @@ import { NodeBadgeMode } from '../../src/types/nodeSource'
import { ComfyActionbar } from '../helpers/actionbar'
import { ComfyTemplates } from '../helpers/templates'
import { ComfyMouse } from './ComfyMouse'
import { VueNodeHelpers } from './VueNodeHelpers'
import { ComfyNodeSearchBox } from './components/ComfyNodeSearchBox'
import { SettingDialog } from './components/SettingDialog'
import {
@@ -144,6 +145,7 @@ export class ComfyPage {
public readonly templates: ComfyTemplates
public readonly settingDialog: SettingDialog
public readonly confirmDialog: ConfirmDialog
public readonly vueNodes: VueNodeHelpers
/** Worker index to test user ID */
public readonly userIds: string[] = []
@@ -172,6 +174,7 @@ export class ComfyPage {
this.templates = new ComfyTemplates(page)
this.settingDialog = new SettingDialog(page, this)
this.confirmDialog = new ConfirmDialog(page)
this.vueNodes = new VueNodeHelpers(page)
}
convertLeafToContent(structure: FolderStructure): FolderStructure {

View File

@@ -0,0 +1,109 @@
/**
* Vue Node Test Helpers
*/
import type { Locator, Page } from '@playwright/test'
export class VueNodeHelpers {
constructor(private page: Page) {}
/**
* Get locator for all Vue node components in the DOM
*/
get nodes(): Locator {
return this.page.locator('[data-node-id]')
}
/**
* Get locator for selected Vue node components (using visual selection indicators)
*/
get selectedNodes(): Locator {
return this.page.locator('[data-node-id].border-blue-500')
}
/**
* Get total count of Vue nodes in the DOM
*/
async getNodeCount(): Promise<number> {
return await this.nodes.count()
}
/**
* Get count of selected Vue nodes
*/
async getSelectedNodeCount(): Promise<number> {
return await this.selectedNodes.count()
}
/**
* Get all Vue node IDs currently in the DOM
*/
async getNodeIds(): Promise<string[]> {
return await this.nodes.evaluateAll(
(nodes) =>
nodes
.map((n) => n.getAttribute('data-node-id'))
.filter((id): id is string => id !== null)
)
}
/**
* Select a specific Vue node by ID
*/
async selectNode(nodeId: string): Promise<void> {
await this.page.locator(`[data-node-id="${nodeId}"]`).click()
}
/**
* Select multiple Vue nodes by IDs using Ctrl+click
*/
async selectNodes(nodeIds: string[]): Promise<void> {
if (nodeIds.length === 0) return
// Select first node normally
await this.selectNode(nodeIds[0])
// Add additional nodes with Ctrl+click
for (let i = 1; i < nodeIds.length; i++) {
await this.page.locator(`[data-node-id="${nodeIds[i]}"]`).click({
modifiers: ['Control']
})
}
}
/**
* Clear all selections by clicking empty space
*/
async clearSelection(): Promise<void> {
await this.page.mouse.click(50, 50)
}
/**
* Delete selected Vue nodes using Delete key
*/
async deleteSelected(): Promise<void> {
await this.page.locator('#graph-canvas').focus()
await this.page.keyboard.press('Delete')
}
/**
* Delete selected Vue nodes using Backspace key
*/
async deleteSelectedWithBackspace(): Promise<void> {
await this.page.locator('#graph-canvas').focus()
await this.page.keyboard.press('Backspace')
}
/**
* Wait for Vue nodes to be rendered
*/
async waitForNodes(expectedCount?: number): Promise<void> {
if (expectedCount !== undefined) {
await this.page.waitForFunction(
(count) => document.querySelectorAll('[data-node-id]').length >= count,
expectedCount
)
} else {
await this.page.waitForSelector('[data-node-id]')
}
}
}

View File

@@ -0,0 +1,141 @@
import { expect } from '@playwright/test'
import { comfyPageFixture as test } from '../../fixtures/ComfyPage'
test.describe('Vue Nodes - Delete Key Interaction', () => {
test.beforeEach(async ({ comfyPage }) => {
// Enable Vue nodes rendering
await comfyPage.setSetting('Comfy.VueNodes.Enabled', true)
await comfyPage.setSetting('Comfy.Graph.CanvasMenu', false)
await comfyPage.setup()
})
test('Can select all and delete Vue nodes with Delete key', async ({
comfyPage
}) => {
await comfyPage.vueNodes.waitForNodes()
// Get initial Vue node count
const initialNodeCount = await comfyPage.vueNodes.getNodeCount()
expect(initialNodeCount).toBeGreaterThan(0)
// Select all Vue nodes
await comfyPage.ctrlA()
// Verify all Vue nodes are selected
const selectedCount = await comfyPage.vueNodes.getSelectedNodeCount()
expect(selectedCount).toBe(initialNodeCount)
// Delete with Delete key
await comfyPage.vueNodes.deleteSelected()
// Verify all Vue nodes were deleted
const finalNodeCount = await comfyPage.vueNodes.getNodeCount()
expect(finalNodeCount).toBe(0)
})
test('Can select specific Vue node and delete it', async ({ comfyPage }) => {
await comfyPage.vueNodes.waitForNodes()
// Get initial Vue node count
const initialNodeCount = await comfyPage.vueNodes.getNodeCount()
expect(initialNodeCount).toBeGreaterThan(0)
// Get first Vue node ID and select it
const nodeIds = await comfyPage.vueNodes.getNodeIds()
await comfyPage.vueNodes.selectNode(nodeIds[0])
// Verify selection
const selectedCount = await comfyPage.vueNodes.getSelectedNodeCount()
expect(selectedCount).toBe(1)
// Delete with Delete key
await comfyPage.vueNodes.deleteSelected()
// Verify one Vue node was deleted
const finalNodeCount = await comfyPage.vueNodes.getNodeCount()
expect(finalNodeCount).toBe(initialNodeCount - 1)
})
test('Can select and delete Vue node with Backspace key', async ({
comfyPage
}) => {
await comfyPage.vueNodes.waitForNodes()
const initialNodeCount = await comfyPage.vueNodes.getNodeCount()
// Select first Vue node
const nodeIds = await comfyPage.vueNodes.getNodeIds()
await comfyPage.vueNodes.selectNode(nodeIds[0])
// Delete with Backspace key instead of Delete
await comfyPage.vueNodes.deleteSelectedWithBackspace()
// Verify Vue node was deleted
const finalNodeCount = await comfyPage.vueNodes.getNodeCount()
expect(finalNodeCount).toBe(initialNodeCount - 1)
})
test('Delete key does not delete node when typing in Vue node widgets', async ({
comfyPage
}) => {
const initialNodeCount = await comfyPage.getGraphNodesCount()
// Find a text input widget in a Vue node
const textWidget = comfyPage.page
.locator('input[type="text"], textarea')
.first()
// Click on text widget to focus it
await textWidget.click()
await textWidget.fill('test text')
// Press Delete while focused on widget - should delete text, not node
await textWidget.press('Delete')
// Node count should remain the same
const finalNodeCount = await comfyPage.getGraphNodesCount()
expect(finalNodeCount).toBe(initialNodeCount)
})
test('Delete key does not delete node when nothing is selected', async ({
comfyPage
}) => {
await comfyPage.vueNodes.waitForNodes()
// Ensure no Vue nodes are selected
await comfyPage.vueNodes.clearSelection()
const selectedCount = await comfyPage.vueNodes.getSelectedNodeCount()
expect(selectedCount).toBe(0)
// Press Delete key - should not crash and should handle gracefully
await comfyPage.page.keyboard.press('Delete')
// Vue node count should remain the same
const nodeCount = await comfyPage.vueNodes.getNodeCount()
expect(nodeCount).toBeGreaterThan(0)
})
test('Can multi-select with Ctrl+click and delete multiple Vue nodes', async ({
comfyPage
}) => {
await comfyPage.vueNodes.waitForNodes()
const initialNodeCount = await comfyPage.vueNodes.getNodeCount()
// Multi-select first two Vue nodes using Ctrl+click
const nodeIds = await comfyPage.vueNodes.getNodeIds()
const nodesToSelect = nodeIds.slice(0, 2)
await comfyPage.vueNodes.selectNodes(nodesToSelect)
// Verify expected nodes are selected
const selectedCount = await comfyPage.vueNodes.getSelectedNodeCount()
expect(selectedCount).toBe(nodesToSelect.length)
// Delete selected Vue nodes
await comfyPage.vueNodes.deleteSelected()
// Verify expected nodes were deleted
const finalNodeCount = await comfyPage.vueNodes.getNodeCount()
expect(finalNodeCount).toBe(initialNodeCount - nodesToSelect.length)
})
})

View File

@@ -1,4 +1,5 @@
import { CORE_KEYBINDINGS } from '@/constants/coreKeybindings'
import { app } from '@/scripts/app'
import { useCommandStore } from '@/stores/commandStore'
import { useDialogStore } from '@/stores/dialogStore'
import {
@@ -14,6 +15,19 @@ export const useKeybindingService = () => {
const settingStore = useSettingStore()
const dialogStore = useDialogStore()
// Helper function to determine if an event should be forwarded to canvas
const shouldForwardToCanvas = (event: KeyboardEvent): boolean => {
// Don't forward if modifier keys are pressed (except shift)
if (event.ctrlKey || event.altKey || event.metaKey) {
return false
}
// Keys that LiteGraph handles but aren't in core keybindings
const canvasKeys = ['Delete', 'Backspace']
return canvasKeys.includes(event.key)
}
const keybindHandler = async function (event: KeyboardEvent) {
const keyCombo = KeyComboImpl.fromEvent(event)
if (keyCombo.isModifier) {
@@ -26,6 +40,7 @@ export const useKeybindingService = () => {
keyCombo.isReservedByTextInput &&
(target.tagName === 'TEXTAREA' ||
target.tagName === 'INPUT' ||
target.contentEditable === 'true' ||
(target.tagName === 'SPAN' &&
target.classList.contains('property_value')))
) {
@@ -53,6 +68,20 @@ export const useKeybindingService = () => {
return
}
// Forward unhandled canvas-targeted events to LiteGraph
if (!keybinding && shouldForwardToCanvas(event)) {
const canvas = app.canvas
if (
canvas &&
canvas.processKey &&
typeof canvas.processKey === 'function'
) {
// Let LiteGraph handle the event
canvas.processKey(event)
return
}
}
// Only clear dialogs if not using modifiers
if (event.ctrlKey || event.altKey || event.metaKey) {
return

View File

@@ -0,0 +1,182 @@
import { createPinia, setActivePinia } from 'pinia'
import { beforeEach, describe, expect, it, vi } from 'vitest'
import { app } from '@/scripts/app'
import { useKeybindingService } from '@/services/keybindingService'
import { useCommandStore } from '@/stores/commandStore'
import { useDialogStore } from '@/stores/dialogStore'
// Mock the app and canvas using factory functions
vi.mock('@/scripts/app', () => {
return {
app: {
canvas: {
processKey: vi.fn()
}
}
}
})
// Mock stores
vi.mock('@/stores/settingStore', () => ({
useSettingStore: vi.fn(() => ({
get: vi.fn(() => [])
}))
}))
vi.mock('@/stores/dialogStore', () => ({
useDialogStore: vi.fn(() => ({
dialogStack: []
}))
}))
// Test utility for creating keyboard events with mocked methods
function createTestKeyboardEvent(
key: string,
options: {
target?: Element
ctrlKey?: boolean
altKey?: boolean
metaKey?: boolean
} = {}
): KeyboardEvent {
const {
target = document.body,
ctrlKey = false,
altKey = false,
metaKey = false
} = options
const event = new KeyboardEvent('keydown', {
key,
ctrlKey,
altKey,
metaKey,
bubbles: true,
cancelable: true
})
// Mock event methods
event.preventDefault = vi.fn()
event.composedPath = vi.fn(() => [target])
return event
}
describe('keybindingService - Event Forwarding', () => {
let keybindingService: ReturnType<typeof useKeybindingService>
let mockCommandExecute: ReturnType<typeof vi.fn>
let mockProcessKey: ReturnType<typeof vi.fn>
beforeEach(() => {
vi.clearAllMocks()
setActivePinia(createPinia())
// Get reference to mocked function
mockProcessKey = vi.mocked(app.canvas.processKey)
// Mock command store execute
mockCommandExecute = vi.fn()
const commandStore = useCommandStore()
commandStore.execute = mockCommandExecute
// Reset dialog store mock to empty
vi.mocked(useDialogStore).mockReturnValue({
dialogStack: []
} as any)
keybindingService = useKeybindingService()
keybindingService.registerCoreKeybindings()
})
it('should forward Delete key to canvas when no keybinding exists', async () => {
const event = createTestKeyboardEvent('Delete')
await keybindingService.keybindHandler(event)
// Should forward to canvas processKey
expect(mockProcessKey).toHaveBeenCalledWith(event)
// Should not execute any command
expect(mockCommandExecute).not.toHaveBeenCalled()
})
it('should forward Backspace key to canvas when no keybinding exists', async () => {
const event = createTestKeyboardEvent('Backspace')
await keybindingService.keybindHandler(event)
expect(mockProcessKey).toHaveBeenCalledWith(event)
expect(mockCommandExecute).not.toHaveBeenCalled()
})
it('should not forward Delete key when typing in input field', async () => {
const inputElement = document.createElement('input')
const event = createTestKeyboardEvent('Delete', { target: inputElement })
await keybindingService.keybindHandler(event)
// Should not forward to canvas when in input field
expect(mockProcessKey).not.toHaveBeenCalled()
expect(mockCommandExecute).not.toHaveBeenCalled()
})
it('should not forward Delete key when typing in textarea', async () => {
const textareaElement = document.createElement('textarea')
const event = createTestKeyboardEvent('Delete', { target: textareaElement })
await keybindingService.keybindHandler(event)
expect(mockProcessKey).not.toHaveBeenCalled()
expect(mockCommandExecute).not.toHaveBeenCalled()
})
it('should not forward Delete key when canvas processKey is not available', async () => {
// Temporarily replace processKey with undefined
const originalProcessKey = vi.mocked(app.canvas).processKey
vi.mocked(app.canvas).processKey = undefined as any
const event = createTestKeyboardEvent('Delete')
await keybindingService.keybindHandler(event)
expect(mockCommandExecute).not.toHaveBeenCalled()
// Restore processKey for other tests
vi.mocked(app.canvas).processKey = originalProcessKey
})
it('should not forward Delete key when canvas is not available', async () => {
// Temporarily set canvas to null
const originalCanvas = vi.mocked(app).canvas
vi.mocked(app).canvas = null as any
const event = createTestKeyboardEvent('Delete')
await keybindingService.keybindHandler(event)
expect(mockCommandExecute).not.toHaveBeenCalled()
// Restore canvas for other tests
vi.mocked(app).canvas = originalCanvas
})
it('should not forward non-canvas keys', async () => {
const event = createTestKeyboardEvent('Enter')
await keybindingService.keybindHandler(event)
// Should not forward Enter key
expect(mockProcessKey).not.toHaveBeenCalled()
expect(mockCommandExecute).not.toHaveBeenCalled()
})
it('should not forward when modifier keys are pressed', async () => {
const event = createTestKeyboardEvent('Delete', { ctrlKey: true })
await keybindingService.keybindHandler(event)
// Should not forward when modifiers are pressed
expect(mockProcessKey).not.toHaveBeenCalled()
expect(mockCommandExecute).not.toHaveBeenCalled()
})
})