diff --git a/.github/workflows/update-electron-types.yaml b/.github/workflows/update-electron-types.yaml index 0dfcdea34..96f85f6b0 100644 --- a/.github/workflows/update-electron-types.yaml +++ b/.github/workflows/update-electron-types.yaml @@ -40,7 +40,7 @@ jobs: - name: Get new version id: get-version run: | - NEW_VERSION=$(pnpm list @comfyorg/comfyui-electron-types --json --depth=0 | jq -r '.[0].version') + NEW_VERSION=$(pnpm list @comfyorg/comfyui-electron-types --json --depth=0 | jq -r '.[0].dependencies."@comfyorg/comfyui-electron-types".version') echo "NEW_VERSION=$NEW_VERSION" >> $GITHUB_OUTPUT - name: Create Pull Request diff --git a/browser_tests/fixtures/ComfyPage.ts b/browser_tests/fixtures/ComfyPage.ts index c32dd3937..008233091 100644 --- a/browser_tests/fixtures/ComfyPage.ts +++ b/browser_tests/fixtures/ComfyPage.ts @@ -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 { @@ -1421,7 +1424,7 @@ export class ComfyPage { } async closeDialog() { - await this.page.locator('.p-dialog-close-button').click() + await this.page.locator('.p-dialog-close-button').click({ force: true }) await expect(this.page.locator('.p-dialog')).toBeHidden() } diff --git a/browser_tests/fixtures/VueNodeHelpers.ts b/browser_tests/fixtures/VueNodeHelpers.ts new file mode 100644 index 000000000..b16b1099a --- /dev/null +++ b/browser_tests/fixtures/VueNodeHelpers.ts @@ -0,0 +1,108 @@ +/** + * 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 { + return await this.nodes.count() + } + + /** + * Get count of selected Vue nodes + */ + async getSelectedNodeCount(): Promise { + return await this.selectedNodes.count() + } + + /** + * Get all Vue node IDs currently in the DOM + */ + async getNodeIds(): Promise { + 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 { + await this.page.locator(`[data-node-id="${nodeId}"]`).click() + } + + /** + * Select multiple Vue nodes by IDs using Ctrl+click + */ + async selectNodes(nodeIds: string[]): Promise { + 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 { + await this.page.mouse.click(50, 50) + } + + /** + * Delete selected Vue nodes using Delete key + */ + async deleteSelected(): Promise { + await this.page.locator('#graph-canvas').focus() + await this.page.keyboard.press('Delete') + } + + /** + * Delete selected Vue nodes using Backspace key + */ + async deleteSelectedWithBackspace(): Promise { + 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 { + if (expectedCount !== undefined) { + await this.page.waitForFunction( + (count) => document.querySelectorAll('[data-node-id]').length >= count, + expectedCount + ) + } else { + await this.page.waitForSelector('[data-node-id]') + } + } +} diff --git a/browser_tests/tests/rightClickMenu.spec.ts-snapshots/add-node-node-added-chromium-linux.png b/browser_tests/tests/rightClickMenu.spec.ts-snapshots/add-node-node-added-chromium-linux.png index 9443182e3..2755d74c5 100644 Binary files a/browser_tests/tests/rightClickMenu.spec.ts-snapshots/add-node-node-added-chromium-linux.png and b/browser_tests/tests/rightClickMenu.spec.ts-snapshots/add-node-node-added-chromium-linux.png differ diff --git a/browser_tests/tests/vueNodes/deleteKeyInteraction.spec.ts b/browser_tests/tests/vueNodes/deleteKeyInteraction.spec.ts new file mode 100644 index 000000000..a00d93eb0 --- /dev/null +++ b/browser_tests/tests/vueNodes/deleteKeyInteraction.spec.ts @@ -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) + }) +}) diff --git a/browser_tests/tests/widget.spec.ts b/browser_tests/tests/widget.spec.ts index b23faabfc..728b5d028 100644 --- a/browser_tests/tests/widget.spec.ts +++ b/browser_tests/tests/widget.spec.ts @@ -264,7 +264,13 @@ test.describe('Animated image widget', () => { expect(filename).toContain('animated_webp.webp') }) - test('Can preview saved animated webp image', async ({ comfyPage }) => { + // FIXME: This test keeps flip-flopping because it relies on animated webp timing, + // which is inherently unreliable in CI environments. The test asset is an animated + // webp with 2 frames, and the test depends on animation frame timing to verify that + // animated webp images are properly displayed (as opposed to being treated as static webp). + // While the underlying functionality works (animated webp are correctly distinguished + // from static webp), the test is flaky due to timing dependencies with webp animation frames. + test.fixme('Can preview saved animated webp image', async ({ comfyPage }) => { await comfyPage.loadWorkflow('widgets/save_animated_webp') // Get position of the load animated webp node diff --git a/browser_tests/tests/widget.spec.ts-snapshots/animated-image-preview-saved-webp-chromium-linux.png b/browser_tests/tests/widget.spec.ts-snapshots/animated-image-preview-saved-webp-chromium-linux.png deleted file mode 100644 index 3f3d831bb..000000000 Binary files a/browser_tests/tests/widget.spec.ts-snapshots/animated-image-preview-saved-webp-chromium-linux.png and /dev/null differ diff --git a/package.json b/package.json index 7c03496a4..66efdbce3 100644 --- a/package.json +++ b/package.json @@ -101,7 +101,7 @@ "dependencies": { "@alloc/quick-lru": "^5.2.0", "@atlaskit/pragmatic-drag-and-drop": "^1.3.1", - "@comfyorg/comfyui-electron-types": "^0.4.69", + "@comfyorg/comfyui-electron-types": "^0.4.72", "@iconify/json": "^2.2.380", "@primeuix/forms": "0.0.2", "@primeuix/styled": "0.3.2", diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 58c7d7aa4..ce404afc4 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -15,8 +15,8 @@ importers: specifier: ^1.3.1 version: 1.3.1 '@comfyorg/comfyui-electron-types': - specifier: ^0.4.69 - version: 0.4.69 + specifier: ^0.4.72 + version: 0.4.72 '@iconify/json': specifier: ^2.2.380 version: 2.2.380 @@ -986,8 +986,8 @@ packages: resolution: {integrity: sha512-6zABk/ECA/QYSCQ1NGiVwwbQerUCZ+TQbp64Q3AgmfNvurHH0j8TtXa1qbShXA6qqkpAj4V5W8pP6mLe1mcMqA==} engines: {node: '>=18'} - '@comfyorg/comfyui-electron-types@0.4.69': - resolution: {integrity: sha512-emEapJvbbx8lXiJ/84gmk+fYU73MmqkQKgBDQkyDwctcOb+eNe347PaH/+0AIjX8A/DtFHfnwgh9J8k3RVdqZA==} + '@comfyorg/comfyui-electron-types@0.4.72': + resolution: {integrity: sha512-Ecf0XYOKDqqIcnjSWL8GHLo6MOsuwqs0I1QgWc3Hv+BZm+qUE4vzOXCyhfFoTIGHLZFTwe37gnygPPKFzMu00Q==} '@csstools/color-helpers@5.1.0': resolution: {integrity: sha512-S11EXWJyy0Mz5SYvRmY8nJYTFFd1LCNV+7cXyAgQtOOuzb4EsgfqDufL+9esx72/eLhsRdGZwaldu/h+E4t4BA==} @@ -6356,8 +6356,8 @@ packages: vue-component-type-helpers@2.2.12: resolution: {integrity: sha512-YbGqHZ5/eW4SnkPNR44mKVc6ZKQoRs/Rux1sxC6rdwXb4qpbOSYfDr9DsTHolOTGmIKgM9j141mZbBeg05R1pw==} - vue-component-type-helpers@3.0.6: - resolution: {integrity: sha512-6CRM8X7EJqWCJOiKPvSLQG+hJPb/Oy2gyJx3pLjUEhY7PuaCthQu3e0zAGI1lqUBobrrk9IT0K8sG2GsCluxoQ==} + vue-component-type-helpers@3.0.7: + resolution: {integrity: sha512-TvyUcFXmjZcXUvU+r1MOyn4/vv4iF+tPwg5Ig33l/FJ3myZkxeQpzzQMLMFWcQAjr6Xs7BRwVy/TwbmNZUA/4w==} vue-demi@0.14.10: resolution: {integrity: sha512-nMZBOwuzabUO0nLgIcc6rycZEebF6eeUfaiQx9+WSk8e29IbLvPU9feI6tqW4kTo3hvoYAJkMh8n8D0fuISphg==} @@ -7502,7 +7502,7 @@ snapshots: '@bcoe/v8-coverage@1.0.2': {} - '@comfyorg/comfyui-electron-types@0.4.69': {} + '@comfyorg/comfyui-electron-types@0.4.72': {} '@csstools/color-helpers@5.1.0': {} @@ -8925,7 +8925,7 @@ snapshots: storybook: 9.1.1(@testing-library/dom@10.4.1)(prettier@3.3.2)(vite@5.4.19(@types/node@20.14.10)(lightningcss@1.30.1)(terser@5.39.2)) type-fest: 2.19.0 vue: 3.5.13(typescript@5.9.2) - vue-component-type-helpers: 3.0.6 + vue-component-type-helpers: 3.0.7 '@swc/helpers@0.5.17': dependencies: @@ -13654,7 +13654,7 @@ snapshots: vue-component-type-helpers@2.2.12: {} - vue-component-type-helpers@3.0.6: {} + vue-component-type-helpers@3.0.7: {} vue-demi@0.14.10(vue@3.5.13(typescript@5.9.2)): dependencies: diff --git a/src/components/graph/GraphCanvas.vue b/src/components/graph/GraphCanvas.vue index 264bdef1f..5c9d6ecb2 100644 --- a/src/components/graph/GraphCanvas.vue +++ b/src/components/graph/GraphCanvas.vue @@ -40,7 +40,7 @@ > { - viewportCulling.handleTransformUpdate( - vueNodeLifecycle.detectChangesInRAF.value - ) + viewportCulling.handleTransformUpdate() + // TODO: Fix paste position sync in separate PR + vueNodeLifecycle.detectChangesInRAF.value() } const handleNodeSelect = nodeEventHandlers.handleNodeSelect const handleNodeCollapse = nodeEventHandlers.handleNodeCollapse diff --git a/src/composables/graph/useViewportCulling.ts b/src/composables/graph/useViewportCulling.ts index b5c996f93..d4d98651c 100644 --- a/src/composables/graph/useViewportCulling.ts +++ b/src/composables/graph/useViewportCulling.ts @@ -1,16 +1,14 @@ /** - * Viewport Culling Composable + * Vue Nodes Viewport Culling * - * Handles viewport culling optimization for Vue nodes including: - * - Transform state synchronization - * - Visible node calculation with screen space transforms - * - Adaptive margin computation based on zoom level - * - Performance optimizations for large graphs + * Principles: + * 1. Query DOM directly using data attributes (no cache to maintain) + * 2. Set display none on element to avoid cascade resolution overhead + * 3. Only run when transform changes (event driven) */ -import { type Ref, computed, readonly, ref } from 'vue' +import { type Ref, computed } from 'vue' import type { VueNodeData } from '@/composables/graph/useGraphNodeManager' -import { useTransformState } from '@/renderer/core/layout/useTransformState' import { app as comfyApp } from '@/scripts/app' import { useCanvasStore } from '@/stores/graphStore' @@ -25,188 +23,84 @@ export function useViewportCulling( nodeManager: Ref ) { const canvasStore = useCanvasStore() - const { syncWithCanvas } = useTransformState() - // Transform tracking for performance optimization - const lastScale = ref(1) - const lastOffsetX = ref(0) - const lastOffsetY = ref(0) - - // Current transform state - const currentTransformState = computed(() => ({ - scale: lastScale.value, - offsetX: lastOffsetX.value, - offsetY: lastOffsetY.value - })) - - /** - * Computed property that returns nodes visible in the current viewport - * Implements sophisticated culling algorithm with adaptive margins - */ - const nodesToRender = computed(() => { - if (!isVueNodesEnabled.value) { - return [] - } - - // Access trigger to force re-evaluation after nodeManager initialization - void nodeDataTrigger.value - - if (!comfyApp.graph) { - return [] - } - - const allNodes = Array.from(vueNodeData.value.values()) - - // Apply viewport culling - check if node bounds intersect with viewport - // TODO: use quadtree - if (nodeManager.value && canvasStore.canvas && comfyApp.canvas) { - const canvas = canvasStore.canvas - const manager = nodeManager.value - - // Ensure transform is synced before checking visibility - syncWithCanvas(comfyApp.canvas) - - const ds = canvas.ds - - // Work in screen space - viewport is simply the canvas element size - const viewport_width = canvas.canvas.width - const viewport_height = canvas.canvas.height - - // Add margin that represents a constant distance in canvas space - // Convert canvas units to screen pixels by multiplying by scale - const canvasMarginDistance = 200 // Fixed margin in canvas units - const margin_x = canvasMarginDistance * ds.scale - const margin_y = canvasMarginDistance * ds.scale - - const filtered = allNodes.filter((nodeData) => { - const node = manager.getNode(nodeData.id) - if (!node) return false - - // Transform node position to screen space (same as DOM widgets) - const screen_x = (node.pos[0] + ds.offset[0]) * ds.scale - const screen_y = (node.pos[1] + ds.offset[1]) * ds.scale - const screen_width = node.size[0] * ds.scale - const screen_height = node.size[1] * ds.scale - - // Check if node bounds intersect with expanded viewport (in screen space) - const isVisible = !( - screen_x + screen_width < -margin_x || - screen_x > viewport_width + margin_x || - screen_y + screen_height < -margin_y || - screen_y > viewport_height + margin_y - ) - - return isVisible - }) - - return filtered - } - - return allNodes + const allNodes = computed(() => { + if (!isVueNodesEnabled.value) return [] + void nodeDataTrigger.value // Force re-evaluation when nodeManager initializes + return Array.from(vueNodeData.value.values()) }) /** - * Handle transform updates with performance optimization - * Only syncs when transform actually changes to avoid unnecessary reflows + * Update visibility of all nodes based on viewport + * Queries DOM directly - no cache maintenance needed */ - const handleTransformUpdate = (detectChangesInRAF: () => void) => { - // Skip all work if Vue nodes are disabled - if (!isVueNodesEnabled.value) { - return - } - - // Sync transform state only when it changes (avoids reflows) - if (comfyApp.canvas?.ds) { - const currentScale = comfyApp.canvas.ds.scale - const currentOffsetX = comfyApp.canvas.ds.offset[0] - const currentOffsetY = comfyApp.canvas.ds.offset[1] - - if ( - currentScale !== lastScale.value || - currentOffsetX !== lastOffsetX.value || - currentOffsetY !== lastOffsetY.value - ) { - syncWithCanvas(comfyApp.canvas) - lastScale.value = currentScale - lastOffsetX.value = currentOffsetX - lastOffsetY.value = currentOffsetY - } - } - - // Detect node changes during transform updates - detectChangesInRAF() - - // Trigger reactivity for nodesToRender - void nodesToRender.value.length - } - - /** - * Calculate if a specific node is visible in viewport - * Useful for individual node visibility checks - */ - const isNodeVisible = (nodeData: VueNodeData): boolean => { - if (!nodeManager.value || !canvasStore.canvas || !comfyApp.canvas) { - return true // Default to visible if culling not available - } + const updateVisibility = () => { + if (!nodeManager.value || !canvasStore.canvas || !comfyApp.canvas) return const canvas = canvasStore.canvas - const node = nodeManager.value.getNode(nodeData.id) - if (!node) return false - - syncWithCanvas(comfyApp.canvas) + const manager = nodeManager.value const ds = canvas.ds + // Viewport bounds const viewport_width = canvas.canvas.width const viewport_height = canvas.canvas.height - const canvasMarginDistance = 200 - const margin_x = canvasMarginDistance * ds.scale - const margin_y = canvasMarginDistance * ds.scale + const margin = 500 * ds.scale - const screen_x = (node.pos[0] + ds.offset[0]) * ds.scale - const screen_y = (node.pos[1] + ds.offset[1]) * ds.scale - const screen_width = node.size[0] * ds.scale - const screen_height = node.size[1] * ds.scale + // Get all node elements at once + const nodeElements = document.querySelectorAll('[data-node-id]') - return !( - screen_x + screen_width < -margin_x || - screen_x > viewport_width + margin_x || - screen_y + screen_height < -margin_y || - screen_y > viewport_height + margin_y - ) + // Update each element's visibility + for (const element of nodeElements) { + const nodeId = element.getAttribute('data-node-id') + if (!nodeId) continue + + const node = manager.getNode(nodeId) + if (!node) continue + + // Calculate if node is outside viewport + const screen_x = (node.pos[0] + ds.offset[0]) * ds.scale + const screen_y = (node.pos[1] + ds.offset[1]) * ds.scale + const screen_width = node.size[0] * ds.scale + const screen_height = node.size[1] * ds.scale + + const isNodeOutsideViewport = + screen_x + screen_width < -margin || + screen_x > viewport_width + margin || + screen_y + screen_height < -margin || + screen_y > viewport_height + margin + + // Setting display none directly avoid potential cascade resolution + if (element instanceof HTMLElement) { + element.style.display = isNodeOutsideViewport ? 'none' : '' + } + } } + // RAF throttling for smooth updates during continuous panning + let rafId: number | null = null + /** - * Get viewport bounds information for debugging + * Handle transform update - called by TransformPane event + * Uses RAF to batch updates for smooth performance */ - const getViewportInfo = () => { - if (!canvasStore.canvas || !comfyApp.canvas) { - return null + const handleTransformUpdate = () => { + if (!isVueNodesEnabled.value) return + + // Cancel previous RAF if still pending + if (rafId !== null) { + cancelAnimationFrame(rafId) } - const canvas = canvasStore.canvas - const ds = canvas.ds - - return { - viewport_width: canvas.canvas.width, - viewport_height: canvas.canvas.height, - scale: ds.scale, - offset: [ds.offset[0], ds.offset[1]], - margin_distance: 200, - margin_x: 200 * ds.scale, - margin_y: 200 * ds.scale - } + // Schedule update in next animation frame + rafId = requestAnimationFrame(() => { + updateVisibility() + rafId = null + }) } return { - nodesToRender, + allNodes, handleTransformUpdate, - isNodeVisible, - getViewportInfo, - - // Transform state - currentTransformState: readonly(currentTransformState), - lastScale: readonly(lastScale), - lastOffsetX: readonly(lastOffsetX), - lastOffsetY: readonly(lastOffsetY) + updateVisibility } } diff --git a/src/renderer/core/layout/store/layoutStore.ts b/src/renderer/core/layout/store/layoutStore.ts index ca5d67120..673344086 100644 --- a/src/renderer/core/layout/store/layoutStore.ts +++ b/src/renderer/core/layout/store/layoutStore.ts @@ -36,9 +36,19 @@ import { type Point, type RerouteId, type RerouteLayout, - type Size, type SlotLayout } from '@/renderer/core/layout/types' +import { + REROUTE_RADIUS, + boundsIntersect, + pointInBounds +} from '@/renderer/core/layout/utils/layoutMath' +import { makeLinkSegmentKey } from '@/renderer/core/layout/utils/layoutUtils' +import { + type NodeLayoutMap, + layoutToYNode, + yNodeToLayout +} from '@/renderer/core/layout/utils/mappers' import { SpatialIndexManager } from '@/renderer/core/spatial/SpatialIndex' type YEventChange = { @@ -48,9 +58,6 @@ type YEventChange = { const logger = log.getLogger('LayoutStore') -// Constants -const REROUTE_RADIUS = 8 - // Utility functions function asRerouteId(id: string | number): RerouteId { return Number(id) @@ -60,15 +67,6 @@ function asLinkId(id: string | number): LinkId { return Number(id) } -interface NodeLayoutData { - id: NodeId - position: Point - size: Size - zIndex: number - visible: boolean - bounds: Bounds -} - interface LinkData { id: LinkId sourceNodeId: NodeId @@ -91,15 +89,6 @@ interface TypedYMap { } class LayoutStoreImpl implements LayoutStore { - private static readonly NODE_DEFAULTS: NodeLayoutData = { - id: 'unknown-node', - position: { x: 0, y: 0 }, - size: { width: 100, height: 50 }, - zIndex: 0, - visible: true, - bounds: { x: 0, y: 0, width: 100, height: 50 } - } - private static readonly REROUTE_DEFAULTS: RerouteData = { id: 0, position: { x: 0, y: 0 }, @@ -109,7 +98,7 @@ class LayoutStoreImpl implements LayoutStore { // Yjs document and shared data structures private ydoc = new Y.Doc() - private ynodes: Y.Map> // Maps nodeId -> Y.Map containing NodeLayout data + private ynodes: Y.Map // Maps nodeId -> NodeLayoutMap containing NodeLayout data private ylinks: Y.Map> // Maps linkId -> Y.Map containing link data private yreroutes: Y.Map> // Maps rerouteId -> Y.Map containing reroute data private yoperations: Y.Array // Operation log @@ -155,7 +144,7 @@ class LayoutStoreImpl implements LayoutStore { this.rerouteSpatialIndex = new SpatialIndexManager() // Listen for Yjs changes and trigger Vue reactivity - this.ynodes.observe((event: Y.YMapEvent>) => { + this.ynodes.observe((event: Y.YMapEvent) => { this.version++ // Trigger all affected node refs @@ -184,16 +173,6 @@ class LayoutStoreImpl implements LayoutStore { }) } - private getNodeField( - ynode: Y.Map, - field: K, - defaultValue: NodeLayoutData[K] = LayoutStoreImpl.NODE_DEFAULTS[field] - ): NodeLayoutData[K] { - const typedNode = ynode as TypedYMap - const value = typedNode.get(field) - return value ?? defaultValue - } - private getLinkField( ylink: Y.Map, field: K @@ -227,7 +206,7 @@ class LayoutStoreImpl implements LayoutStore { get: () => { track() const ynode = this.ynodes.get(nodeId) - const layout = ynode ? this.yNodeToLayout(ynode) : null + const layout = ynode ? yNodeToLayout(ynode) : null return layout }, set: (newLayout: NodeLayout | null) => { @@ -242,7 +221,7 @@ class LayoutStoreImpl implements LayoutStore { timestamp: Date.now(), source: this.currentSource, actor: this.currentActor, - previousLayout: this.yNodeToLayout(existing) + previousLayout: yNodeToLayout(existing) }) } } else { @@ -260,7 +239,7 @@ class LayoutStoreImpl implements LayoutStore { actor: this.currentActor }) } else { - const existingLayout = this.yNodeToLayout(existing) + const existingLayout = yNodeToLayout(existing) // Check what properties changed if ( @@ -330,8 +309,8 @@ class LayoutStoreImpl implements LayoutStore { for (const [nodeId] of this.ynodes) { const ynode = this.ynodes.get(nodeId) if (ynode) { - const layout = this.yNodeToLayout(ynode) - if (layout && this.boundsIntersect(layout.bounds, bounds)) { + const layout = yNodeToLayout(ynode) + if (layout && boundsIntersect(layout.bounds, bounds)) { result.push(nodeId) } } @@ -352,7 +331,7 @@ class LayoutStoreImpl implements LayoutStore { for (const [nodeId] of this.ynodes) { const ynode = this.ynodes.get(nodeId) if (ynode) { - const layout = this.yNodeToLayout(ynode) + const layout = yNodeToLayout(ynode) if (layout) { result.set(nodeId, layout) } @@ -378,7 +357,7 @@ class LayoutStoreImpl implements LayoutStore { for (const [nodeId] of this.ynodes) { const ynode = this.ynodes.get(nodeId) if (ynode) { - const layout = this.yNodeToLayout(ynode) + const layout = yNodeToLayout(ynode) if (layout) { nodes.push([nodeId, layout]) } @@ -389,7 +368,7 @@ class LayoutStoreImpl implements LayoutStore { nodes.sort(([, a], [, b]) => b.zIndex - a.zIndex) for (const [nodeId, layout] of nodes) { - if (this.pointInBounds(point, layout.bounds)) { + if (pointInBounds(point, layout.bounds)) { return nodeId } } @@ -561,16 +540,6 @@ class LayoutStoreImpl implements LayoutStore { return this.rerouteLayouts.get(rerouteId) || null } - /** - * Helper to create internal key for link segment - */ - private makeLinkSegmentKey( - linkId: LinkId, - rerouteId: RerouteId | null - ): string { - return `${linkId}:${rerouteId ?? 'final'}` - } - /** * Update link segment layout data */ @@ -579,7 +548,7 @@ class LayoutStoreImpl implements LayoutStore { rerouteId: RerouteId | null, layout: Omit ): void { - const key = this.makeLinkSegmentKey(linkId, rerouteId) + const key = makeLinkSegmentKey(linkId, rerouteId) const existing = this.linkSegmentLayouts.get(key) // Short-circuit if bounds and centerPos unchanged (prevents spatial index churn) @@ -629,7 +598,7 @@ class LayoutStoreImpl implements LayoutStore { * Delete link segment layout data */ deleteLinkSegmentLayout(linkId: LinkId, rerouteId: RerouteId | null): void { - const key = this.makeLinkSegmentKey(linkId, rerouteId) + const key = makeLinkSegmentKey(linkId, rerouteId) const deleted = this.linkSegmentLayouts.delete(key) if (deleted) { // Remove from spatial index @@ -693,7 +662,7 @@ class LayoutStoreImpl implements LayoutStore { rerouteId: segmentLayout.rerouteId } } - } else if (this.pointInBounds(point, segmentLayout.bounds)) { + } else if (pointInBounds(point, segmentLayout.bounds)) { // Fallback to bounding box test return { linkId: segmentLayout.linkId, @@ -733,7 +702,7 @@ class LayoutStoreImpl implements LayoutStore { // Check precise bounds for candidates for (const key of candidateSlotKeys) { const slotLayout = this.slotLayouts.get(key) - if (slotLayout && this.pointInBounds(point, slotLayout.bounds)) { + if (slotLayout && pointInBounds(point, slotLayout.bounds)) { return slotLayout } } @@ -969,7 +938,7 @@ class LayoutStoreImpl implements LayoutStore { } } - this.ynodes.set(layout.id, this.layoutToYNode(layout)) + this.ynodes.set(layout.id, layoutToYNode(layout)) // Add to spatial index this.spatialIndex.insert(layout.id, layout.bounds) @@ -987,7 +956,7 @@ class LayoutStoreImpl implements LayoutStore { return } - const size = this.getNodeField(ynode, 'size') + const size = yNodeToLayout(ynode).size const newBounds = { x: operation.position.x, y: operation.position.y, @@ -1016,7 +985,7 @@ class LayoutStoreImpl implements LayoutStore { const ynode = this.ynodes.get(operation.nodeId) if (!ynode) return - const position = this.getNodeField(ynode, 'position') + const position = yNodeToLayout(ynode).position const newBounds = { x: position.x, y: position.y, @@ -1053,7 +1022,7 @@ class LayoutStoreImpl implements LayoutStore { operation: CreateNodeOperation, change: LayoutChange ): void { - const ynode = this.layoutToYNode(operation.layout) + const ynode = layoutToYNode(operation.layout) this.ynodes.set(operation.nodeId, ynode) // Add to spatial index @@ -1187,7 +1156,7 @@ class LayoutStoreImpl implements LayoutStore { * Update node bounds helper */ private updateNodeBounds( - ynode: Y.Map, + ynode: NodeLayoutMap, position: Point, size: { width: number; height: number } ): void { @@ -1335,27 +1304,6 @@ class LayoutStoreImpl implements LayoutStore { } // Helper methods - private layoutToYNode(layout: NodeLayout): Y.Map { - const ynode = new Y.Map() - ynode.set('id', layout.id) - ynode.set('position', layout.position) - ynode.set('size', layout.size) - ynode.set('zIndex', layout.zIndex) - ynode.set('visible', layout.visible) - ynode.set('bounds', layout.bounds) - return ynode - } - - private yNodeToLayout(ynode: Y.Map): NodeLayout { - return { - id: this.getNodeField(ynode, 'id'), - position: this.getNodeField(ynode, 'position'), - size: this.getNodeField(ynode, 'size'), - zIndex: this.getNodeField(ynode, 'zIndex'), - visible: this.getNodeField(ynode, 'visible'), - bounds: this.getNodeField(ynode, 'bounds') - } - } private notifyChange(change: LayoutChange): void { this.changeListeners.forEach((listener) => { @@ -1367,24 +1315,6 @@ class LayoutStoreImpl implements LayoutStore { }) } - private pointInBounds(point: Point, bounds: Bounds): boolean { - return ( - point.x >= bounds.x && - point.x <= bounds.x + bounds.width && - point.y >= bounds.y && - point.y <= bounds.y + bounds.height - ) - } - - private boundsIntersect(a: Bounds, b: Bounds): boolean { - return !( - a.x + a.width < b.x || - b.x + b.width < a.x || - a.y + a.height < b.y || - b.y + b.height < a.y - ) - } - // CRDT-specific methods getOperationsSince(timestamp: number): LayoutOperation[] { const operations: LayoutOperation[] = [] diff --git a/src/renderer/core/layout/utils/layoutMath.ts b/src/renderer/core/layout/utils/layoutMath.ts new file mode 100644 index 000000000..786e261dd --- /dev/null +++ b/src/renderer/core/layout/utils/layoutMath.ts @@ -0,0 +1,21 @@ +import type { Bounds, Point } from '@/renderer/core/layout/types' + +export const REROUTE_RADIUS = 8 + +export function pointInBounds(point: Point, bounds: Bounds): boolean { + return ( + point.x >= bounds.x && + point.x <= bounds.x + bounds.width && + point.y >= bounds.y && + point.y <= bounds.y + bounds.height + ) +} + +export function boundsIntersect(a: Bounds, b: Bounds): boolean { + return !( + a.x + a.width < b.x || + b.x + b.width < a.x || + a.y + a.height < b.y || + b.y + b.height < a.y + ) +} diff --git a/src/renderer/core/layout/utils/layoutUtils.ts b/src/renderer/core/layout/utils/layoutUtils.ts new file mode 100644 index 000000000..408e2718d --- /dev/null +++ b/src/renderer/core/layout/utils/layoutUtils.ts @@ -0,0 +1,11 @@ +import type { LinkId, RerouteId } from '@/renderer/core/layout/types' + +/** + * Creates a unique key for identifying link segments in spatial indexes + */ +export function makeLinkSegmentKey( + linkId: LinkId, + rerouteId: RerouteId | null +): string { + return `${linkId}:${rerouteId ?? 'final'}` +} diff --git a/src/renderer/core/layout/utils/mappers.ts b/src/renderer/core/layout/utils/mappers.ts new file mode 100644 index 000000000..48b85dc53 --- /dev/null +++ b/src/renderer/core/layout/utils/mappers.ts @@ -0,0 +1,45 @@ +import * as Y from 'yjs' + +import type { NodeLayout } from '@/renderer/core/layout/types' + +export type NodeLayoutMap = Y.Map + +export const NODE_LAYOUT_DEFAULTS: NodeLayout = { + id: 'unknown-node', + position: { x: 0, y: 0 }, + size: { width: 100, height: 50 }, + zIndex: 0, + visible: true, + bounds: { x: 0, y: 0, width: 100, height: 50 } +} + +export function layoutToYNode(layout: NodeLayout): NodeLayoutMap { + const ynode = new Y.Map() as NodeLayoutMap + ynode.set('id', layout.id) + ynode.set('position', layout.position) + ynode.set('size', layout.size) + ynode.set('zIndex', layout.zIndex) + ynode.set('visible', layout.visible) + ynode.set('bounds', layout.bounds) + return ynode +} + +function getOr( + map: NodeLayoutMap, + key: K, + fallback: NodeLayout[K] +): NodeLayout[K] { + const v = map.get(key) + return (v ?? fallback) as NodeLayout[K] +} + +export function yNodeToLayout(ynode: NodeLayoutMap): NodeLayout { + return { + id: getOr(ynode, 'id', NODE_LAYOUT_DEFAULTS.id), + position: getOr(ynode, 'position', NODE_LAYOUT_DEFAULTS.position), + size: getOr(ynode, 'size', NODE_LAYOUT_DEFAULTS.size), + zIndex: getOr(ynode, 'zIndex', NODE_LAYOUT_DEFAULTS.zIndex), + visible: getOr(ynode, 'visible', NODE_LAYOUT_DEFAULTS.visible), + bounds: getOr(ynode, 'bounds', NODE_LAYOUT_DEFAULTS.bounds) + } +} diff --git a/src/renderer/extensions/vueNodes/components/InputSlot.vue b/src/renderer/extensions/vueNodes/components/InputSlot.vue index 124597d6b..ea6d7db4e 100644 --- a/src/renderer/extensions/vueNodes/components/InputSlot.vue +++ b/src/renderer/extensions/vueNodes/components/InputSlot.vue @@ -21,7 +21,7 @@ {{ slotData.localized_name || slotData.name || `Input ${index}` }} diff --git a/src/renderer/extensions/vueNodes/components/LGraphNode.vue b/src/renderer/extensions/vueNodes/components/LGraphNode.vue index a95904837..aeb0e9a23 100644 --- a/src/renderer/extensions/vueNodes/components/LGraphNode.vue +++ b/src/renderer/extensions/vueNodes/components/LGraphNode.vue @@ -13,8 +13,7 @@ // border 'border border-solid border-sand-100 dark-theme:border-charcoal-300', !!executing && 'border-blue-500 dark-theme:border-blue-500', - !!(error || nodeData.hasErrors) && - 'border-error dark-theme:border-error', + !!(error || nodeData.hasErrors) && 'border-error', // hover 'hover:ring-7 ring-gray-500/50 dark-theme:ring-gray-500/20', // Selected @@ -22,8 +21,7 @@ !!isSelected && 'outline-black dark-theme:outline-white', !!(isSelected && executing) && 'outline-blue-500 dark-theme:outline-blue-500', - !!(isSelected && (error || nodeData.hasErrors)) && - 'outline-error dark-theme:outline-error', + !!(isSelected && (error || nodeData.hasErrors)) && 'outline-error', { 'animate-pulse': executing, 'opacity-50': nodeData.mode === 4, diff --git a/src/renderer/extensions/vueNodes/components/NodeHeader.vue b/src/renderer/extensions/vueNodes/components/NodeHeader.vue index 3ae528733..2af065317 100644 --- a/src/renderer/extensions/vueNodes/components/NodeHeader.vue +++ b/src/renderer/extensions/vueNodes/components/NodeHeader.vue @@ -18,7 +18,7 @@ > diff --git a/src/renderer/extensions/vueNodes/components/OutputSlot.vue b/src/renderer/extensions/vueNodes/components/OutputSlot.vue index e83019aa9..51dcc1d22 100644 --- a/src/renderer/extensions/vueNodes/components/OutputSlot.vue +++ b/src/renderer/extensions/vueNodes/components/OutputSlot.vue @@ -15,7 +15,7 @@ {{ slotData.name || `Output ${index}` }} diff --git a/src/renderer/extensions/vueNodes/widgets/components/WidgetButton.test.ts b/src/renderer/extensions/vueNodes/widgets/components/WidgetButton.test.ts new file mode 100644 index 000000000..72d6726a5 --- /dev/null +++ b/src/renderer/extensions/vueNodes/widgets/components/WidgetButton.test.ts @@ -0,0 +1,248 @@ +import { mount } from '@vue/test-utils' +import Button from 'primevue/button' +import type { ButtonProps } from 'primevue/button' +import PrimeVue from 'primevue/config' +import { describe, expect, it, vi } from 'vitest' + +import WidgetButton from '@/renderer/extensions/vueNodes/widgets/components/WidgetButton.vue' +import type { SimplifiedWidget } from '@/types/simplifiedWidget' + +describe('WidgetButton Interactions', () => { + const createMockWidget = ( + options: Partial = {}, + callback?: () => void, + name: string = 'test_button' + ): SimplifiedWidget => ({ + name, + type: 'button', + value: undefined, + options, + callback + }) + + const mountComponent = (widget: SimplifiedWidget, readonly = false) => { + return mount(WidgetButton, { + global: { + plugins: [PrimeVue], + components: { Button } + }, + props: { + widget, + readonly + } + }) + } + + const clickButton = async (wrapper: ReturnType) => { + const button = wrapper.findComponent({ name: 'Button' }) + await button.trigger('click') + return button + } + + describe('Click Handling', () => { + it('calls callback when button is clicked', async () => { + const mockCallback = vi.fn() + const widget = createMockWidget({}, mockCallback) + const wrapper = mountComponent(widget) + + await clickButton(wrapper) + + expect(mockCallback).toHaveBeenCalledTimes(1) + }) + + it('does not call callback when button is readonly', async () => { + const mockCallback = vi.fn() + const widget = createMockWidget({}, mockCallback) + const wrapper = mountComponent(widget, true) + + await clickButton(wrapper) + + expect(mockCallback).not.toHaveBeenCalled() + }) + + it('handles missing callback gracefully', async () => { + const widget = createMockWidget({}, undefined) + const wrapper = mountComponent(widget) + + // Should not throw error when clicking without callback + await expect(clickButton(wrapper)).resolves.toBeDefined() + }) + + it('calls callback multiple times when clicked multiple times', async () => { + const mockCallback = vi.fn() + const widget = createMockWidget({}, mockCallback) + const wrapper = mountComponent(widget) + + const numClicks = 8 + + await clickButton(wrapper) + for (let i = 0; i < numClicks; i++) { + await clickButton(wrapper) + } + + expect(mockCallback).toHaveBeenCalledTimes(numClicks) + }) + }) + + describe('Component Rendering', () => { + it('renders button component', () => { + const widget = createMockWidget() + const wrapper = mountComponent(widget) + + const button = wrapper.findComponent({ name: 'Button' }) + expect(button.exists()).toBe(true) + }) + + it('renders widget label when name is provided', () => { + const widget = createMockWidget() + const wrapper = mountComponent(widget) + + const label = wrapper.find('label') + expect(label.exists()).toBe(true) + expect(label.text()).toBe('test_button') + }) + + it('does not render label when widget name is empty', () => { + const widget = createMockWidget({}, undefined, '') + const wrapper = mountComponent(widget) + + const label = wrapper.find('label') + expect(label.exists()).toBe(false) + }) + + it('sets button size to small', () => { + const widget = createMockWidget() + const wrapper = mountComponent(widget) + + const button = wrapper.findComponent({ name: 'Button' }) + expect(button.props('size')).toBe('small') + }) + + it('passes widget options to button component', () => { + const buttonOptions = { + label: 'Custom Label', + icon: 'pi pi-check', + severity: 'success' as const + } + const widget = createMockWidget(buttonOptions) + const wrapper = mountComponent(widget) + + const button = wrapper.findComponent({ name: 'Button' }) + expect(button.props('label')).toBe('Custom Label') + expect(button.props('icon')).toBe('pi pi-check') + expect(button.props('severity')).toBe('success') + }) + }) + + describe('Readonly Mode', () => { + it('disables button when readonly', () => { + const widget = createMockWidget() + const wrapper = mountComponent(widget, true) + + // Test the actual DOM button element instead of the Vue component props + const buttonElement = wrapper.find('button') + expect(buttonElement.element.disabled).toBe(true) + }) + + it('enables button when not readonly', () => { + const widget = createMockWidget() + const wrapper = mountComponent(widget, false) + + // Test the actual DOM button element instead of the Vue component props + const buttonElement = wrapper.find('button') + expect(buttonElement.element.disabled).toBe(false) + }) + }) + + describe('Widget Options', () => { + it('handles button with text only', () => { + const widget = createMockWidget({ label: 'Click Me' }) + const wrapper = mountComponent(widget) + + const button = wrapper.findComponent({ name: 'Button' }) + expect(button.props('label')).toBe('Click Me') + expect(button.props('icon')).toBeNull() + }) + + it('handles button with icon only', () => { + const widget = createMockWidget({ icon: 'pi pi-star' }) + const wrapper = mountComponent(widget) + + const button = wrapper.findComponent({ name: 'Button' }) + expect(button.props('icon')).toBe('pi pi-star') + }) + + it('handles button with both text and icon', () => { + const widget = createMockWidget({ + label: 'Save', + icon: 'pi pi-save' + }) + const wrapper = mountComponent(widget) + + const button = wrapper.findComponent({ name: 'Button' }) + expect(button.props('label')).toBe('Save') + expect(button.props('icon')).toBe('pi pi-save') + }) + + it.for([ + 'secondary', + 'success', + 'info', + 'warning', + 'danger', + 'help', + 'contrast' + ] as const)('handles button severity: %s', (severity) => { + const widget = createMockWidget({ severity }) + const wrapper = mountComponent(widget) + const button = wrapper.findComponent({ name: 'Button' }) + expect(button.props('severity')).toBe(severity) + }) + + it.for(['outlined', 'text'] as const)( + 'handles button variant: %s', + (variant) => { + const widget = createMockWidget({ variant }) + const wrapper = mountComponent(widget) + const button = wrapper.findComponent({ name: 'Button' }) + expect(button.props('variant')).toBe(variant) + } + ) + }) + + describe('Edge Cases', () => { + it('handles widget with no options', () => { + const widget = createMockWidget() + const wrapper = mountComponent(widget) + + const button = wrapper.findComponent({ name: 'Button' }) + expect(button.exists()).toBe(true) + }) + + it('handles callback that throws error', async () => { + const mockCallback = vi.fn(() => { + throw new Error('Callback error') + }) + const widget = createMockWidget({}, mockCallback) + const wrapper = mountComponent(widget) + + // Should not break the component when callback throws + await expect(clickButton(wrapper)).rejects.toThrow('Callback error') + expect(mockCallback).toHaveBeenCalledTimes(1) + }) + + it('handles rapid consecutive clicks', async () => { + const mockCallback = vi.fn() + const widget = createMockWidget({}, mockCallback) + const wrapper = mountComponent(widget) + + // Simulate rapid clicks + const clickPromises = Array.from({ length: 16 }, () => + clickButton(wrapper) + ) + await Promise.all(clickPromises) + + expect(mockCallback).toHaveBeenCalledTimes(16) + }) + }) +}) diff --git a/src/renderer/extensions/vueNodes/widgets/components/WidgetColorPicker.test.ts b/src/renderer/extensions/vueNodes/widgets/components/WidgetColorPicker.test.ts new file mode 100644 index 000000000..96bb4ed3e --- /dev/null +++ b/src/renderer/extensions/vueNodes/widgets/components/WidgetColorPicker.test.ts @@ -0,0 +1,304 @@ +import { mount } from '@vue/test-utils' +import ColorPicker from 'primevue/colorpicker' +import type { ColorPickerProps } from 'primevue/colorpicker' +import PrimeVue from 'primevue/config' +import { describe, expect, it } from 'vitest' + +import type { SimplifiedWidget } from '@/types/simplifiedWidget' + +import WidgetColorPicker from './WidgetColorPicker.vue' +import WidgetLayoutField from './layout/WidgetLayoutField.vue' + +describe('WidgetColorPicker Value Binding', () => { + const createMockWidget = ( + value: string = '#000000', + options: Partial = {}, + callback?: (value: string) => void + ): SimplifiedWidget => ({ + name: 'test_color_picker', + type: 'color', + value, + options, + callback + }) + + const mountComponent = ( + widget: SimplifiedWidget, + modelValue: string, + readonly = false + ) => { + return mount(WidgetColorPicker, { + global: { + plugins: [PrimeVue], + components: { + ColorPicker, + WidgetLayoutField + } + }, + props: { + widget, + modelValue, + readonly + } + }) + } + + const setColorPickerValue = async ( + wrapper: ReturnType, + value: unknown + ) => { + const colorPicker = wrapper.findComponent({ name: 'ColorPicker' }) + await colorPicker.setValue(value) + return wrapper.emitted('update:modelValue') + } + + describe('Vue Event Emission', () => { + it('emits Vue event when color changes', async () => { + const widget = createMockWidget('#ff0000') + const wrapper = mountComponent(widget, '#ff0000') + + const emitted = await setColorPickerValue(wrapper, '#00ff00') + + expect(emitted).toBeDefined() + expect(emitted![0]).toContain('#00ff00') + }) + + it('handles different color formats', async () => { + const widget = createMockWidget('#ffffff') + const wrapper = mountComponent(widget, '#ffffff') + + const emitted = await setColorPickerValue(wrapper, '#123abc') + + expect(emitted).toBeDefined() + expect(emitted![0]).toContain('#123abc') + }) + + it('handles missing callback gracefully', async () => { + const widget = createMockWidget('#000000', {}, undefined) + const wrapper = mountComponent(widget, '#000000') + + const emitted = await setColorPickerValue(wrapper, '#ff00ff') + + // Should still emit Vue event + expect(emitted).toBeDefined() + expect(emitted![0]).toContain('#ff00ff') + }) + + it('normalizes bare hex without # to #hex on emit', async () => { + const widget = createMockWidget('ff0000') + const wrapper = mountComponent(widget, 'ff0000') + + const emitted = await setColorPickerValue(wrapper, '00ff00') + expect(emitted).toBeDefined() + expect(emitted![0]).toContain('#00ff00') + }) + + it('normalizes rgb() strings to #hex on emit', async () => { + const widget = createMockWidget('#000000') + const wrapper = mountComponent(widget, '#000000') + + const emitted = await setColorPickerValue(wrapper, 'rgb(255, 0, 0)') + expect(emitted).toBeDefined() + expect(emitted![0]).toContain('#ff0000') + }) + + it('normalizes hsb() strings to #hex on emit', async () => { + const widget = createMockWidget('#000000', { format: 'hsb' }) + const wrapper = mountComponent(widget, '#000000') + + const emitted = await setColorPickerValue(wrapper, 'hsb(120, 100, 100)') + expect(emitted).toBeDefined() + expect(emitted![0]).toContain('#00ff00') + }) + + it('normalizes HSB object values to #hex on emit', async () => { + const widget = createMockWidget('#000000', { format: 'hsb' }) + const wrapper = mountComponent(widget, '#000000') + + const emitted = await setColorPickerValue(wrapper, { + h: 240, + s: 100, + b: 100 + }) + expect(emitted).toBeDefined() + expect(emitted![0]).toContain('#0000ff') + }) + }) + + describe('Component Rendering', () => { + it('renders color picker component', () => { + const widget = createMockWidget('#ff0000') + const wrapper = mountComponent(widget, '#ff0000') + + const colorPicker = wrapper.findComponent({ name: 'ColorPicker' }) + expect(colorPicker.exists()).toBe(true) + }) + + it('normalizes display to a single leading #', () => { + // Case 1: model value already includes '#' + let widget = createMockWidget('#ff0000') + let wrapper = mountComponent(widget, '#ff0000') + let colorText = wrapper.find('[data-testid="widget-color-text"]') + expect.soft(colorText.text()).toBe('#ff0000') + + // Case 2: model value missing '#' + widget = createMockWidget('ff0000') + wrapper = mountComponent(widget, 'ff0000') + colorText = wrapper.find('[data-testid="widget-color-text"]') + expect.soft(colorText.text()).toBe('#ff0000') + }) + + it('renders layout field wrapper', () => { + const widget = createMockWidget('#ff0000') + const wrapper = mountComponent(widget, '#ff0000') + + const layoutField = wrapper.findComponent({ name: 'WidgetLayoutField' }) + expect(layoutField.exists()).toBe(true) + }) + + it('displays current color value as text', () => { + const widget = createMockWidget('#ff0000') + const wrapper = mountComponent(widget, '#ff0000') + + const colorText = wrapper.find('[data-testid="widget-color-text"]') + expect(colorText.text()).toBe('#ff0000') + }) + + it('updates color text when value changes', async () => { + const widget = createMockWidget('#ff0000') + const wrapper = mountComponent(widget, '#ff0000') + + await setColorPickerValue(wrapper, '#00ff00') + + // Need to check the local state update + const colorText = wrapper.find('[data-testid="widget-color-text"]') + // Be specific about the displayed value including the leading '#' + expect.soft(colorText.text()).toBe('#00ff00') + }) + + it('uses default color when no value provided', () => { + const widget = createMockWidget('') + const wrapper = mountComponent(widget, '') + + const colorPicker = wrapper.findComponent({ name: 'ColorPicker' }) + // Should use the default value from the composable + expect(colorPicker.exists()).toBe(true) + }) + }) + + describe('Readonly Mode', () => { + it('disables color picker when readonly', () => { + const widget = createMockWidget('#ff0000') + const wrapper = mountComponent(widget, '#ff0000', true) + + const colorPicker = wrapper.findComponent({ name: 'ColorPicker' }) + expect(colorPicker.props('disabled')).toBe(true) + }) + + it('enables color picker when not readonly', () => { + const widget = createMockWidget('#ff0000') + const wrapper = mountComponent(widget, '#ff0000', false) + + const colorPicker = wrapper.findComponent({ name: 'ColorPicker' }) + expect(colorPicker.props('disabled')).toBe(false) + }) + }) + + describe('Color Formats', () => { + it('handles valid hex colors', async () => { + const validHexColors = [ + '#000000', + '#ffffff', + '#ff0000', + '#00ff00', + '#0000ff', + '#123abc' + ] + + for (const color of validHexColors) { + const widget = createMockWidget(color) + const wrapper = mountComponent(widget, color) + + const colorText = wrapper.find('[data-testid="widget-color-text"]') + expect.soft(colorText.text()).toBe(color) + } + }) + + it('handles short hex colors', () => { + const widget = createMockWidget('#fff') + const wrapper = mountComponent(widget, '#fff') + + const colorText = wrapper.find('[data-testid="widget-color-text"]') + expect(colorText.text()).toBe('#fff') + }) + + it('passes widget options to color picker', () => { + const colorOptions = { + format: 'hex' as const, + inline: true + } + const widget = createMockWidget('#ff0000', colorOptions) + const wrapper = mountComponent(widget, '#ff0000') + + const colorPicker = wrapper.findComponent({ name: 'ColorPicker' }) + expect(colorPicker.props('format')).toBe('hex') + expect(colorPicker.props('inline')).toBe(true) + }) + }) + + describe('Widget Layout Integration', () => { + it('passes widget to layout field', () => { + const widget = createMockWidget('#ff0000') + const wrapper = mountComponent(widget, '#ff0000') + + const layoutField = wrapper.findComponent({ name: 'WidgetLayoutField' }) + expect(layoutField.props('widget')).toEqual(widget) + }) + + it('maintains proper component structure', () => { + const widget = createMockWidget('#ff0000') + const wrapper = mountComponent(widget, '#ff0000') + + // Should have layout field containing label with color picker and text + const layoutField = wrapper.findComponent({ name: 'WidgetLayoutField' }) + const label = wrapper.find('label') + const colorPicker = wrapper.findComponent({ name: 'ColorPicker' }) + const colorText = wrapper.find('span') + + expect(layoutField.exists()).toBe(true) + expect(label.exists()).toBe(true) + expect(colorPicker.exists()).toBe(true) + expect(colorText.exists()).toBe(true) + }) + }) + + describe('Edge Cases', () => { + it('handles empty color value', () => { + const widget = createMockWidget('') + const wrapper = mountComponent(widget, '') + + const colorPicker = wrapper.findComponent({ name: 'ColorPicker' }) + expect(colorPicker.exists()).toBe(true) + }) + + it('handles invalid color formats gracefully', async () => { + const widget = createMockWidget('invalid-color') + const wrapper = mountComponent(widget, 'invalid-color') + + const colorText = wrapper.find('[data-testid="widget-color-text"]') + expect(colorText.text()).toBe('#000000') + + const emitted = await setColorPickerValue(wrapper, 'invalid-color') + expect(emitted).toBeDefined() + expect(emitted![0]).toContain('#000000') + }) + + it('handles widget with no options', () => { + const widget = createMockWidget('#ff0000') + const wrapper = mountComponent(widget, '#ff0000') + + const colorPicker = wrapper.findComponent({ name: 'ColorPicker' }) + expect(colorPicker.exists()).toBe(true) + }) + }) +}) diff --git a/src/renderer/extensions/vueNodes/widgets/components/WidgetColorPicker.vue b/src/renderer/extensions/vueNodes/widgets/components/WidgetColorPicker.vue index ed5f2b0ec..a3fcb1725 100644 --- a/src/renderer/extensions/vueNodes/widgets/components/WidgetColorPicker.vue +++ b/src/renderer/extensions/vueNodes/widgets/components/WidgetColorPicker.vue @@ -14,19 +14,26 @@ :pt="{ preview: '!w-full !h-full !border-none' }" - @update:model-value="onChange" + @update:model-value="onPickerUpdate" /> - #{{ localValue }} + {{ + toHexFromFormat(localValue, format) + }}