From b5b502755fcedec12b8aabbcc221c4279fd8ee70 Mon Sep 17 00:00:00 2001 From: Terry Jia Date: Mon, 4 May 2026 18:44:52 -0400 Subject: [PATCH 001/172] fix(load3d): parse [output]/[input]/[temp] annotation when loading (#11805) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary The Vue node model picker mixes output assets into the dropdown with a trailing ' [output]' suffix on the value. Forwarding that string to loadModel as a literal filename under the configured input folder caused a 404 and the model never rendered. Strip the trailing folder annotation per call and use the matching folder so picking an output asset loads correctly while plain values keep the configured folder. Output assets stored under a subfolder (e.g. 3d/) were exposed in the Vue node dropdown as just ' [output]', so selection produced an /api/view URL with empty subfolder and a 404. Read the subfolder from the asset's OutputAssetMetadata and prefix it onto the annotated path so the downstream load handler can split it back out and target the correct folder. ## Screenshots (if applicable) https://github.com/user-attachments/assets/463d1071-efdc-46a4-b101-8e1c012d19c7 ┆Issue is synchronized with this [Notion page](https://app.notion.com/p/PR-11805-fix-load3d-parse-output-input-temp-annotation-when-loading-3536d73d365081a8ac9cf75d14ae29e6) by [Unito](https://www.unito.io) --- .../core/load3d/Load3DConfiguration.test.ts | 48 +++++++++- .../core/load3d/Load3DConfiguration.ts | 21 ++++- .../composables/useWidgetSelectItems.test.ts | 89 +++++++++++++++++++ .../composables/useWidgetSelectItems.ts | 9 +- 4 files changed, 163 insertions(+), 4 deletions(-) diff --git a/src/extensions/core/load3d/Load3DConfiguration.test.ts b/src/extensions/core/load3d/Load3DConfiguration.test.ts index 0aa445673f..7d79aede48 100644 --- a/src/extensions/core/load3d/Load3DConfiguration.test.ts +++ b/src/extensions/core/load3d/Load3DConfiguration.test.ts @@ -1,7 +1,9 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' import type Load3d from '@/extensions/core/load3d/Load3d' -import Load3DConfiguration from '@/extensions/core/load3d/Load3DConfiguration' +import Load3DConfiguration, { + parseAnnotatedFilename +} from '@/extensions/core/load3d/Load3DConfiguration' import Load3dUtils from '@/extensions/core/load3d/Load3dUtils' import type { GizmoConfig, @@ -249,3 +251,47 @@ describe('Load3DConfiguration.silentOnNotFound propagation', () => { }) }) }) + +describe('parseAnnotatedFilename', () => { + it('strips a [output] suffix and switches to the output folder', () => { + expect(parseAnnotatedFilename('foo.glb [output]', 'input')).toEqual({ + filename: 'foo.glb', + folder: 'output' + }) + }) + + it('strips a [input] suffix and switches to the input folder', () => { + expect(parseAnnotatedFilename('sub/foo.glb [input]', 'output')).toEqual({ + filename: 'sub/foo.glb', + folder: 'input' + }) + }) + + it('strips a [temp] suffix and switches to the temp folder', () => { + expect(parseAnnotatedFilename('foo.glb [temp]', 'input')).toEqual({ + filename: 'foo.glb', + folder: 'temp' + }) + }) + + it('returns the value unchanged with the fallback folder when unannotated', () => { + expect(parseAnnotatedFilename('foo.glb', 'input')).toEqual({ + filename: 'foo.glb', + folder: 'input' + }) + }) + + it('does not strip a non-folder annotation', () => { + expect(parseAnnotatedFilename('foo.glb [draft]', 'input')).toEqual({ + filename: 'foo.glb [draft]', + folder: 'input' + }) + }) + + it('only matches a trailing annotation, not one in the middle', () => { + expect(parseAnnotatedFilename('foo [output] bar.glb', 'input')).toEqual({ + filename: 'foo [output] bar.glb', + folder: 'input' + }) + }) +}) diff --git a/src/extensions/core/load3d/Load3DConfiguration.ts b/src/extensions/core/load3d/Load3DConfiguration.ts index 2486e8472a..426df0c785 100644 --- a/src/extensions/core/load3d/Load3DConfiguration.ts +++ b/src/extensions/core/load3d/Load3DConfiguration.ts @@ -24,6 +24,20 @@ type Load3DConfigurationSettings = { silentOnNotFound?: boolean } +const ANNOTATED_FILENAME_PATTERN = / \[(input|output|temp)\]$/ + +export function parseAnnotatedFilename( + rawValue: string, + fallbackFolder: string +): { filename: string; folder: string } { + const match = ANNOTATED_FILENAME_PATTERN.exec(rawValue) + if (!match) return { filename: rawValue, folder: fallbackFolder } + return { + filename: rawValue.slice(0, match.index), + folder: match[1] + } +} + class Load3DConfiguration { constructor( private load3d: Load3d, @@ -268,14 +282,17 @@ class Load3DConfiguration { return async (value: string | number | boolean | object) => { if (!value) return - const filename = value as string + const { filename, folder } = parseAnnotatedFilename( + value as string, + loadFolder + ) this.setResourceFolder(filename) const modelUrl = api.apiURL( Load3dUtils.getResourceURL( ...Load3dUtils.splitFilePath(filename), - loadFolder + folder ) ) diff --git a/src/renderer/extensions/vueNodes/widgets/composables/useWidgetSelectItems.test.ts b/src/renderer/extensions/vueNodes/widgets/composables/useWidgetSelectItems.test.ts index 7dcac57f0a..924a3a6f65 100644 --- a/src/renderer/extensions/vueNodes/widgets/composables/useWidgetSelectItems.test.ts +++ b/src/renderer/extensions/vueNodes/widgets/composables/useWidgetSelectItems.test.ts @@ -683,6 +683,95 @@ describe('useWidgetSelectItems', () => { }) }) + describe('output asset subfolder', () => { + it('prefixes the subfolder onto the annotated path so the load URL targets the right folder', async () => { + mockMediaAssets.media.value = [ + { + id: 'asset-mesh-1', + name: 'ComfyUI_00105_.glb', + preview_url: '', + tags: ['output'], + user_metadata: { + jobId: 'job-mesh', + nodeId: '7', + subfolder: '3d' + } + } + ] + + const { dropdownItems, filterSelected } = useWidgetSelectItems( + createDefaultOptions({ + values: () => [], + modelValue: ref(undefined), + assetKind: () => 'mesh' as const + }) + ) + filterSelected.value = 'outputs' + await nextTick() + + expect(dropdownItems.value).toHaveLength(1) + expect(dropdownItems.value[0].name).toBe('3d/ComfyUI_00105_.glb [output]') + }) + + it('omits the subfolder prefix when the asset has none', async () => { + mockMediaAssets.media.value = [ + { + id: 'asset-mesh-2', + name: 'plain.glb', + preview_url: '', + tags: ['output'], + user_metadata: { + jobId: 'job-plain', + nodeId: '8', + subfolder: '' + } + } + ] + + const { dropdownItems, filterSelected } = useWidgetSelectItems( + createDefaultOptions({ + values: () => [], + modelValue: ref(undefined), + assetKind: () => 'mesh' as const + }) + ) + filterSelected.value = 'outputs' + await nextTick() + + expect(dropdownItems.value).toHaveLength(1) + expect(dropdownItems.value[0].name).toBe('plain.glb [output]') + }) + + it('does not prefix the subfolder for non-mesh kinds even when present', async () => { + mockMediaAssets.media.value = [ + { + id: 'asset-image-1', + name: 'photo.png', + preview_url: '', + tags: ['output'], + user_metadata: { + jobId: 'job-image', + nodeId: '9', + subfolder: 'sub' + } + } + ] + + const { dropdownItems, filterSelected } = useWidgetSelectItems( + createDefaultOptions({ + values: () => [], + modelValue: ref(undefined), + assetKind: () => 'image' as const + }) + ) + filterSelected.value = 'outputs' + await nextTick() + + expect(dropdownItems.value).toHaveLength(1) + expect(dropdownItems.value[0].name).toBe('photo.png [output]') + }) + }) + describe('FE-228: output dropdown label uses human-readable filename', () => { it('renders metadata.filename in label when asset.name is a hash', async () => { mockMediaAssets.media.value = [ diff --git a/src/renderer/extensions/vueNodes/widgets/composables/useWidgetSelectItems.ts b/src/renderer/extensions/vueNodes/widgets/composables/useWidgetSelectItems.ts index 55d27acd1b..86a11321aa 100644 --- a/src/renderer/extensions/vueNodes/widgets/composables/useWidgetSelectItems.ts +++ b/src/renderer/extensions/vueNodes/widgets/composables/useWidgetSelectItems.ts @@ -180,7 +180,14 @@ export function useWidgetSelectItems(options: UseWidgetSelectItemsOptions) { if (getMediaTypeFromFilename(asset.name) !== targetMediaType) continue if (seen.has(asset.id)) continue seen.add(asset.id) - const annotatedPath = `${asset.name} [output]` + const subfolder = + kind === 'mesh' + ? getOutputAssetMetadata(asset.user_metadata)?.subfolder + : undefined + const pathWithSubfolder = subfolder + ? `${subfolder}/${asset.name}` + : asset.name + const annotatedPath = `${pathWithSubfolder} [output]` const displayLabel = `${getAssetDisplayFilename(asset)} [output]` items.push({ id: `output-${asset.id}`, From 5e3266e0c222553d5b09948607a8267c5ec62161 Mon Sep 17 00:00:00 2001 From: Christian Byrne Date: Mon, 4 May 2026 15:52:33 -0700 Subject: [PATCH 002/172] test: add e2e tests for node replacement flows (#11242) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary Add Playwright e2e tests for the node replacement feature (swap nodes UI in the errors tab). ## Changes - **What**: 6 e2e test cases across two describe blocks covering single and multi-type node replacement flows. Tests verify swap nodes group visibility, in-place replacement, widget value preservation, Replace All across multiple types, output connection preservation, and success toast display. Includes typed mock data for `/api/node_replacements` and two workflow fixture files with fake missing node types mapped to real core nodes. ## Review Focus - Mock setup pattern in `setupNodeReplacement` — enables feature flag via `page.evaluate` and routes the API endpoint - Workflow fixture design — uses fake node types (E2E_OldSampler, E2E_OldUpscaler) that map to real registered types (KSampler, ImageScaleBy) - Assertion coverage for link preservation after replacement ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-11242-test-add-e2e-tests-for-node-replacement-flows-3426d73d3650811e87d7f0d96fd66433) by [Unito](https://www.unito.io) --------- Co-authored-by: Connor Byrne --- .../missing/node_replacement_multi.json | 68 +++++++ .../missing/node_replacement_simple.json | 59 ++++++ .../fixtures/data/nodeReplacements.ts | 47 +++++ .../fixtures/helpers/NodeReplacementHelper.ts | 93 ++++++++++ browser_tests/fixtures/selectors.ts | 1 + browser_tests/tests/nodeReplacement.spec.ts | 168 ++++++++++++++++++ 6 files changed, 436 insertions(+) create mode 100644 browser_tests/assets/missing/node_replacement_multi.json create mode 100644 browser_tests/assets/missing/node_replacement_simple.json create mode 100644 browser_tests/fixtures/data/nodeReplacements.ts create mode 100644 browser_tests/fixtures/helpers/NodeReplacementHelper.ts create mode 100644 browser_tests/tests/nodeReplacement.spec.ts diff --git a/browser_tests/assets/missing/node_replacement_multi.json b/browser_tests/assets/missing/node_replacement_multi.json new file mode 100644 index 0000000000..c4473f216a --- /dev/null +++ b/browser_tests/assets/missing/node_replacement_multi.json @@ -0,0 +1,68 @@ +{ + "last_node_id": 4, + "last_link_id": 2, + "nodes": [ + { + "id": 1, + "type": "E2E_OldSampler", + "pos": [100, 100], + "size": [400, 262], + "flags": {}, + "order": 0, + "mode": 0, + "inputs": [ + { "name": "model", "type": "MODEL", "link": null }, + { "name": "positive", "type": "CONDITIONING", "link": null }, + { "name": "negative", "type": "CONDITIONING", "link": null }, + { "name": "latent_image", "type": "LATENT", "link": null } + ], + "outputs": [ + { + "name": "LATENT", + "type": "LATENT", + "links": [], + "slot_index": 0 + } + ], + "properties": { "Node name for S&R": "E2E_OldSampler" }, + "widgets_values": [42, 20, 7, "euler", "normal"] + }, + { + "id": 2, + "type": "E2E_OldUpscaler", + "pos": [500, 100], + "size": [400, 200], + "flags": {}, + "order": 1, + "mode": 0, + "inputs": [{ "name": "image", "type": "IMAGE", "link": null }], + "outputs": [ + { + "name": "IMAGE", + "type": "IMAGE", + "links": [2], + "slot_index": 0 + } + ], + "properties": { "Node name for S&R": "E2E_OldUpscaler" }, + "widgets_values": ["lanczos", 1.5] + }, + { + "id": 3, + "type": "SaveImage", + "pos": [900, 100], + "size": [400, 200], + "flags": {}, + "order": 2, + "mode": 0, + "inputs": [{ "name": "images", "type": "IMAGE", "link": 2 }], + "properties": { "Node name for S&R": "SaveImage" }, + "widgets_values": ["ComfyUI"] + } + ], + "links": [[2, 2, 0, 3, 0, "IMAGE"]], + "groups": [], + "config": {}, + "extra": { "ds": { "scale": 1, "offset": [0, 0] } }, + "version": 0.4 +} diff --git a/browser_tests/assets/missing/node_replacement_simple.json b/browser_tests/assets/missing/node_replacement_simple.json new file mode 100644 index 0000000000..ce0de205b7 --- /dev/null +++ b/browser_tests/assets/missing/node_replacement_simple.json @@ -0,0 +1,59 @@ +{ + "last_node_id": 3, + "last_link_id": 1, + "nodes": [ + { + "id": 1, + "type": "E2E_OldSampler", + "pos": [100, 100], + "size": [400, 262], + "flags": {}, + "order": 0, + "mode": 0, + "inputs": [ + { "name": "model", "type": "MODEL", "link": null }, + { "name": "positive", "type": "CONDITIONING", "link": null }, + { "name": "negative", "type": "CONDITIONING", "link": null }, + { "name": "latent_image", "type": "LATENT", "link": null } + ], + "outputs": [ + { + "name": "LATENT", + "type": "LATENT", + "links": [1], + "slot_index": 0 + } + ], + "properties": { "Node name for S&R": "E2E_OldSampler" }, + "widgets_values": [42, 20, 7, "euler", "normal"] + }, + { + "id": 2, + "type": "VAEDecode", + "pos": [500, 100], + "size": [400, 200], + "flags": {}, + "order": 1, + "mode": 0, + "inputs": [ + { "name": "samples", "type": "LATENT", "link": 1 }, + { "name": "vae", "type": "VAE", "link": null } + ], + "outputs": [ + { + "name": "IMAGE", + "type": "IMAGE", + "links": [], + "slot_index": 0 + } + ], + "properties": { "Node name for S&R": "VAEDecode" }, + "widgets_values": [] + } + ], + "links": [[1, 1, 0, 2, 0, "LATENT"]], + "groups": [], + "config": {}, + "extra": { "ds": { "scale": 1, "offset": [0, 0] } }, + "version": 0.4 +} diff --git a/browser_tests/fixtures/data/nodeReplacements.ts b/browser_tests/fixtures/data/nodeReplacements.ts new file mode 100644 index 0000000000..31c9cd2af2 --- /dev/null +++ b/browser_tests/fixtures/data/nodeReplacements.ts @@ -0,0 +1,47 @@ +import type { NodeReplacementResponse } from '@/platform/nodeReplacement/types' + +/** + * Mock node replacement mappings for e2e tests. + * + * Maps fake "missing" node types (E2E_OldSampler, E2E_OldUpscaler) to real + * core node types that are always available in the test server. + */ +export const mockNodeReplacements: NodeReplacementResponse = { + E2E_OldSampler: [ + { + new_node_id: 'KSampler', + old_node_id: 'E2E_OldSampler', + old_widget_ids: ['seed', 'steps', 'cfg', 'sampler_name', 'scheduler'], + input_mapping: [ + { new_id: 'model', old_id: 'model' }, + { new_id: 'positive', old_id: 'positive' }, + { new_id: 'negative', old_id: 'negative' }, + { new_id: 'latent_image', old_id: 'latent_image' }, + { new_id: 'seed', old_id: 'seed' }, + { new_id: 'steps', old_id: 'steps' }, + { new_id: 'cfg', old_id: 'cfg' }, + { new_id: 'sampler_name', old_id: 'sampler_name' }, + { new_id: 'scheduler', old_id: 'scheduler' } + ], + output_mapping: [{ new_idx: 0, old_idx: 0 }] + } + ], + E2E_OldUpscaler: [ + { + new_node_id: 'ImageScaleBy', + old_node_id: 'E2E_OldUpscaler', + old_widget_ids: ['upscale_method', 'scale_by'], + input_mapping: [ + { new_id: 'image', old_id: 'image' }, + { new_id: 'upscale_method', old_id: 'upscale_method' }, + { new_id: 'scale_by', old_id: 'scale_by' } + ], + output_mapping: [{ new_idx: 0, old_idx: 0 }] + } + ] +} + +/** Subset containing only the E2E_OldSampler replacement. */ +export const mockNodeReplacementsSingle: NodeReplacementResponse = { + E2E_OldSampler: mockNodeReplacements.E2E_OldSampler +} diff --git a/browser_tests/fixtures/helpers/NodeReplacementHelper.ts b/browser_tests/fixtures/helpers/NodeReplacementHelper.ts new file mode 100644 index 0000000000..31584e7bef --- /dev/null +++ b/browser_tests/fixtures/helpers/NodeReplacementHelper.ts @@ -0,0 +1,93 @@ +import type { Locator, Page } from '@playwright/test' + +import type { ComfyPage } from '@e2e/fixtures/ComfyPage' +import { TestIds } from '@e2e/fixtures/selectors' +import type { NodeReplacementResponse } from '@/platform/nodeReplacement/types' + +/** + * Mock `/api/node_replacements` and enable the node replacement feature. + * + * Unlike features that only consult settings (e.g. shareWorkflowDialog, + * managerDialog), node replacement gates on `api.serverFeatureFlags`. The + * server sends a `feature_flags` WS message that wholesale replaces + * `serverFeatureFlags`, racing with any test-side override done via + * `page.evaluate`. To make the flow deterministic across CI shards, this + * helper patches `WebSocket.prototype` so every incoming `feature_flags` + * message has `node_replacements: true` injected before the api's WS + * handler sees it. Reload the page so the patched WebSocket and persisted + * settings apply to a fresh app boot, then wait for the resulting + * `/api/node_replacements` fetch before returning. + */ +export async function setupNodeReplacement( + comfyPage: ComfyPage, + replacements: NodeReplacementResponse +): Promise { + await comfyPage.page.route('**/api/node_replacements', (route) => + route.fulfill({ json: replacements }) + ) + + await comfyPage.settings.setSetting( + 'Comfy.RightSidePanel.ShowErrorsTab', + true + ) + await comfyPage.settings.setSetting('Comfy.NodeReplacement.Enabled', true) + + await comfyPage.page.addInitScript(() => { + const proto = window.WebSocket.prototype + const originalAdd = proto.addEventListener + proto.addEventListener = function patchedAdd( + this: WebSocket, + type: string, + listener: EventListenerOrEventListenerObject | null, + options?: AddEventListenerOptions | boolean + ) { + if (type === 'message' && typeof listener === 'function') { + const wrapped = function (this: WebSocket, event: Event) { + const msgEvent = event as MessageEvent + if (typeof msgEvent.data === 'string') { + try { + const msg = JSON.parse(msgEvent.data) + if ( + msg && + msg.type === 'feature_flags' && + msg.data && + typeof msg.data === 'object' + ) { + msg.data.node_replacements = true + const patched = new MessageEvent('message', { + data: JSON.stringify(msg), + origin: msgEvent.origin, + lastEventId: msgEvent.lastEventId + }) + return (listener as EventListener).call(this, patched) + } + } catch { + // not JSON or not a feature_flags message - pass through + } + } + return (listener as EventListener).call(this, event) + } + return originalAdd.call(this, type, wrapped as EventListener, options) + } + return originalAdd.call( + this, + type, + listener as EventListenerOrEventListenerObject, + options + ) + } + }) + + const fetchPromise = comfyPage.page.waitForResponse( + (response) => + response.url().includes('/api/node_replacements') && response.ok(), + { timeout: 10000 } + ) + + await comfyPage.workflow.reloadAndWaitForApp() + await fetchPromise +} + +export function getSwapNodesGroup(page: Page): Locator { + return page.getByTestId(TestIds.dialogs.swapNodesGroup) +} diff --git a/browser_tests/fixtures/selectors.ts b/browser_tests/fixtures/selectors.ts index 57124fdcaa..0d86f9cb79 100644 --- a/browser_tests/fixtures/selectors.ts +++ b/browser_tests/fixtures/selectors.ts @@ -64,6 +64,7 @@ export const TestIds = { missingModelRefresh: 'missing-model-refresh', missingModelImportUnsupported: 'missing-model-import-unsupported', missingMediaGroup: 'error-group-missing-media', + swapNodesGroup: 'error-group-swap-nodes', missingMediaRow: 'missing-media-row', missingMediaUploadDropzone: 'missing-media-upload-dropzone', missingMediaLibrarySelect: 'missing-media-library-select', diff --git a/browser_tests/tests/nodeReplacement.spec.ts b/browser_tests/tests/nodeReplacement.spec.ts new file mode 100644 index 0000000000..89ca766343 --- /dev/null +++ b/browser_tests/tests/nodeReplacement.spec.ts @@ -0,0 +1,168 @@ +import { + comfyPageFixture as test, + comfyExpect as expect +} from '@e2e/fixtures/ComfyPage' +import { + mockNodeReplacements, + mockNodeReplacementsSingle +} from '@e2e/fixtures/data/nodeReplacements' +import { loadWorkflowAndOpenErrorsTab } from '@e2e/fixtures/helpers/ErrorsTabHelper' +import { + getSwapNodesGroup, + setupNodeReplacement +} from '@e2e/fixtures/helpers/NodeReplacementHelper' + +const renderModes = [ + { name: 'vue nodes', vueNodesEnabled: true }, + { name: 'litegraph', vueNodesEnabled: false } +] as const + +test.describe('Node replacement', { tag: ['@node', '@ui'] }, () => { + for (const mode of renderModes) { + test.describe(`(${mode.name})`, () => { + test.describe('Single replacement', () => { + test.beforeEach(async ({ comfyPage }) => { + await comfyPage.settings.setSetting( + 'Comfy.VueNodes.Enabled', + mode.vueNodesEnabled + ) + await setupNodeReplacement(comfyPage, mockNodeReplacementsSingle) + await loadWorkflowAndOpenErrorsTab( + comfyPage, + 'missing/node_replacement_simple' + ) + }) + + test('Swap Nodes group appears in errors tab for replaceable nodes', async ({ + comfyPage + }) => { + const swapGroup = getSwapNodesGroup(comfyPage.page) + await expect(swapGroup).toBeVisible() + await expect(swapGroup).toContainText('E2E_OldSampler') + await expect( + swapGroup.getByRole('button', { name: 'Replace All', exact: true }) + ).toBeVisible() + }) + + test('Replace Node replaces a single group in-place', async ({ + comfyPage + }) => { + const swapGroup = getSwapNodesGroup(comfyPage.page) + await swapGroup.getByRole('button', { name: /replace node/i }).click() + await expect(swapGroup).toBeHidden() + + const workflow = await comfyPage.workflow.getExportedWorkflow() + expect( + workflow.nodes, + 'Node count should be unchanged after in-place replacement' + ).toHaveLength(2) + + const nodeTypes = workflow.nodes.map((n) => n.type) + expect(nodeTypes).not.toContain('E2E_OldSampler') + expect(nodeTypes).toContain('KSampler') + + const ksampler = workflow.nodes.find((n) => n.type === 'KSampler') + expect( + ksampler?.id, + 'Replaced node should keep the original id' + ).toBe(1) + + const linkFromReplacedToDecode = workflow.links?.find( + (l) => l[1] === 1 && l[3] === 2 + ) + expect( + linkFromReplacedToDecode, + 'Output link from replaced node to VAEDecode should be preserved' + ).toBeDefined() + }) + + test('Widget values are preserved after replacement', async ({ + comfyPage + }) => { + await getSwapNodesGroup(comfyPage.page) + .getByRole('button', { name: /replace node/i }) + .click() + + const workflow = await comfyPage.workflow.getExportedWorkflow() + const ksampler = workflow.nodes.find((n) => n.type === 'KSampler') + + expect(ksampler?.widgets_values).toBeDefined() + const widgetValues = ksampler!.widgets_values as unknown[] + expect(widgetValues).toEqual([ + 42, + 'randomize', + 20, + 7, + 'euler', + 'normal', + 1 + ]) + }) + + test('Success toast is shown after replacement', async ({ + comfyPage + }) => { + await getSwapNodesGroup(comfyPage.page) + .getByRole('button', { name: /replace node/i }) + .click() + + await expect(comfyPage.visibleToasts.first()).toContainText( + /replaced|swapped/i + ) + }) + }) + + test.describe('Multi-type replacement', () => { + test.beforeEach(async ({ comfyPage }) => { + await comfyPage.settings.setSetting( + 'Comfy.VueNodes.Enabled', + mode.vueNodesEnabled + ) + await setupNodeReplacement(comfyPage, mockNodeReplacements) + await loadWorkflowAndOpenErrorsTab( + comfyPage, + 'missing/node_replacement_multi' + ) + }) + + test('Replace All replaces all groups across multiple types', async ({ + comfyPage + }) => { + const swapGroup = getSwapNodesGroup(comfyPage.page) + await expect(swapGroup).toBeVisible() + await expect(swapGroup).toContainText('E2E_OldSampler') + await expect(swapGroup).toContainText('E2E_OldUpscaler') + + await swapGroup + .getByRole('button', { name: 'Replace All', exact: true }) + .click() + await expect(swapGroup).toBeHidden() + + const workflow = await comfyPage.workflow.getExportedWorkflow() + const nodeTypes = workflow.nodes.map((n) => n.type) + expect(nodeTypes).not.toContain('E2E_OldSampler') + expect(nodeTypes).not.toContain('E2E_OldUpscaler') + expect(nodeTypes).toContain('KSampler') + expect(nodeTypes).toContain('ImageScaleBy') + }) + + test('Output connections are preserved across replacement with output mapping', async ({ + comfyPage + }) => { + await getSwapNodesGroup(comfyPage.page) + .getByRole('button', { name: 'Replace All', exact: true }) + .click() + + const replacedNodeOutputLinkCount = await comfyPage.page.evaluate( + () => + window.app!.graph!.getNodeById(2)?.outputs[0]?.links?.length ?? 0 + ) + expect( + replacedNodeOutputLinkCount, + 'Replaced upscaler should still drive its downstream consumer' + ).toBeGreaterThan(0) + }) + }) + }) + } +}) From 1f60f7cfcc5b8e7e7549744fa84d5698511dcc2b Mon Sep 17 00:00:00 2001 From: Christian Byrne Date: Mon, 4 May 2026 16:55:06 -0700 Subject: [PATCH 003/172] test: add unit tests for useImageCrop composable (#11138) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary Add 40 unit tests for `useImageCrop` composable (previously 0% coverage, 277 missed lines). ## Changes - **What**: New test file `src/composables/useImageCrop.test.ts` covering: - Crop computed properties (read/write/defaults) - `cropBoxStyle` computation - `selectedRatio` / `isLockEnabled` aspect ratio locking - `applyLockedRatio` with boundary clamping - `resizeHandles` filtering (8 handles unlocked, 4 corners locked) - `handleImageLoad` / `handleImageError` - Drag start/move/end with boundary clamping - Resize from all 4 edges + MIN_CROP_SIZE enforcement - Constrained resize with locked aspect ratio (corner handles) - `getInputImageUrl` with subgraph node resolution - `updateDisplayedDimensions` for landscape/portrait/zero dimensions - `initialize` with `resolveNode` lookup ## Review Focus Test-only change. Mocks `resolveNode`, `useNodeOutputStore`, and `useResizeObserver`. No production code changes. ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-11138-test-add-unit-tests-for-useImageCrop-composable-33e6d73d365081e6aa06e98b66feb585) by [Unito](https://www.unito.io) --------- Co-authored-by: GitHub Action --- src/composables/useImageCrop.test.ts | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/src/composables/useImageCrop.test.ts b/src/composables/useImageCrop.test.ts index e7ccd65b32..9132344e3b 100644 --- a/src/composables/useImageCrop.test.ts +++ b/src/composables/useImageCrop.test.ts @@ -481,6 +481,30 @@ describe('useImageCrop', () => { expect(vm.modelValue.x).toBe(50) }) + it('resizes from the top edge, moving y and shrinking height', async () => { + const vm = await mountHarness() + setupImageLayout(vm, 500, 500) + vm.modelValue = { x: 50, y: 100, width: 120, height: 200 } + + const captureEl = document.createElement('div') + captureEl.setPointerCapture = vi.fn() + captureEl.releasePointerCapture = vi.fn() + + const resizeStart = vm.handleResizeStart as ( + e: PointerEvent, + dir: string + ) => void + const resizeMove = vm.handleResizeMove as (e: PointerEvent) => void + const resizeEnd = vm.handleResizeEnd as (e: PointerEvent) => void + + resizeStart(makePointerEvent('pointerdown', captureEl, 100, 100), 'top') + resizeMove(makePointerEvent('pointermove', captureEl, 100, 150)) + resizeEnd(makePointerEvent('pointerup', captureEl, 100, 150)) + + expect(vm.modelValue.y).toBeGreaterThan(100) + expect(vm.modelValue.height).toBeLessThan(200) + }) + it('applies a preset aspect ratio and clamps height to the image', async () => { const vm = await mountHarness() setupImageLayout(vm, 800, 500) From ea2e8e59f2156093566f85b7f103621be6f3ab97 Mon Sep 17 00:00:00 2001 From: Christian Byrne Date: Mon, 4 May 2026 17:02:15 -0700 Subject: [PATCH 004/172] test: add MembersPanelContent unit tests (#11402) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary Add 27 unit tests for MembersPanelContent component covering workspace views, member management, and billing states. ## Changes - **What**: New test file for MembersPanelContent with 27 tests across 8 describe blocks (personal workspace, owner view, member view, sorting, search filtering, pending invites, single seat plan, member count display) ## Review Focus - Uses `@testing-library/vue` + `@testing-library/user-event` per project conventions - Component stubs (Button, SearchInput, Menu, UserAvatar) for isolation - Reactive mock refs in `vi.hoisted()` shared across `vi.mock()` calls ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-11402-test-add-MembersPanelContent-unit-tests-3476d73d36508107abcbce95b72b3fb7) by [Unito](https://www.unito.io) --- .../settings/MembersPanelContent.test.ts | 349 ++++++++++++ .../dialogs/settings/MembersPanelContent.vue | 189 +------ .../composables/useMembersPanel.test.ts | 501 ++++++++++++++++++ .../workspace/composables/useMembersPanel.ts | 219 ++++++++ 4 files changed, 1092 insertions(+), 166 deletions(-) create mode 100644 src/platform/workspace/components/dialogs/settings/MembersPanelContent.test.ts create mode 100644 src/platform/workspace/composables/useMembersPanel.test.ts create mode 100644 src/platform/workspace/composables/useMembersPanel.ts diff --git a/src/platform/workspace/components/dialogs/settings/MembersPanelContent.test.ts b/src/platform/workspace/components/dialogs/settings/MembersPanelContent.test.ts new file mode 100644 index 0000000000..9d49cbfd59 --- /dev/null +++ b/src/platform/workspace/components/dialogs/settings/MembersPanelContent.test.ts @@ -0,0 +1,349 @@ +import { render, screen } from '@testing-library/vue' +import userEvent from '@testing-library/user-event' +import { beforeEach, describe, expect, it, vi } from 'vitest' +import { computed, ref } from 'vue' +import { createI18n } from 'vue-i18n' + +import MembersPanelContent from './MembersPanelContent.vue' + +import type { + PendingInvite, + WorkspaceMember +} from '../../../stores/teamWorkspaceStore' + +const mockHandleCopyInviteLink = vi.fn() +const mockHandleRevokeInvite = vi.fn() +const mockHandleCreateWorkspace = vi.fn() +const mockShowSubscriptionDialog = vi.fn() +const mockSelectMember = vi.fn() +const mockToggleSort = vi.fn() + +const { + mockMembers, + mockPendingInvites, + mockFilteredMembers, + mockFilteredPendingInvites, + mockIsPersonalWorkspace, + mockIsSingleSeatPlan, + mockIsActiveSubscription, + mockActiveView, + mockSearchQuery, + mockPermissions, + mockUiConfig +} = vi.hoisted(() => { + // eslint-disable-next-line @typescript-eslint/no-require-imports, @typescript-eslint/consistent-type-imports + const { ref } = require('vue') as typeof import('vue') + + return { + mockMembers: ref([]), + mockPendingInvites: ref([]), + mockFilteredMembers: ref([]), + mockFilteredPendingInvites: ref([]), + mockIsPersonalWorkspace: ref(false), + mockIsSingleSeatPlan: ref(false), + mockIsActiveSubscription: ref(true), + mockActiveView: ref<'active' | 'pending'>('active'), + mockSearchQuery: ref(''), + mockPermissions: ref({ + canViewOtherMembers: true, + canViewPendingInvites: true, + canInviteMembers: true, + canManageInvites: true, + canRemoveMembers: true, + canLeaveWorkspace: true, + canAccessWorkspaceMenu: true, + canManageSubscription: true, + canTopUp: true + }), + mockUiConfig: ref({ + showMembersList: true, + showPendingTab: true, + showSearch: true, + showDateColumn: true, + showRoleBadge: true, + membersGridCols: 'grid-cols-[50%_40%_10%]', + pendingGridCols: 'grid-cols-[50%_20%_20%_10%]', + headerGridCols: 'grid-cols-[50%_40%_10%]', + showEditWorkspaceMenuItem: true, + workspaceMenuAction: 'delete' as 'leave' | 'delete' | null, + workspaceMenuDisabledTooltip: null as string | null + }) + } +}) + +vi.mock('@/platform/workspace/composables/useMembersPanel', () => ({ + useMembersPanel: () => ({ + searchQuery: mockSearchQuery, + activeView: mockActiveView, + maxSeats: computed(() => 20), + isSingleSeatPlan: mockIsSingleSeatPlan, + personalWorkspaceMember: computed(() => ({ + id: 'self', + name: 'Owner User', + email: 'owner@example.com', + role: 'owner' as const, + joinDate: new Date(0) + })), + filteredMembers: mockFilteredMembers, + filteredPendingInvites: mockFilteredPendingInvites, + memberMenuItems: computed(() => []), + isPersonalWorkspace: mockIsPersonalWorkspace, + members: mockMembers, + pendingInvites: mockPendingInvites, + permissions: mockPermissions, + uiConfig: mockUiConfig, + isActiveSubscription: mockIsActiveSubscription, + userPhotoUrl: ref(null), + isCurrentUser: (m: WorkspaceMember) => + m.email.toLowerCase() === 'owner@example.com', + selectMember: mockSelectMember, + toggleSort: mockToggleSort, + showSubscriptionDialog: mockShowSubscriptionDialog, + handleCopyInviteLink: mockHandleCopyInviteLink, + handleRevokeInvite: mockHandleRevokeInvite, + handleCreateWorkspace: mockHandleCreateWorkspace, + handleRemoveMember: vi.fn() + }) +})) + +const i18n = createI18n({ + legacy: false, + locale: 'en', + messages: { en: {} }, + missingWarn: false, + fallbackWarn: false +}) + +const ButtonStub = { + name: 'Button', + template: + '', + props: ['disabled', 'loading', 'variant', 'size', 'ariaLabel'] +} + +const SearchInputStub = { + name: 'SearchInput', + template: + '', + props: ['modelValue', 'placeholder', 'size'], + emits: ['update:modelValue'] +} + +function renderComponent() { + return render(MembersPanelContent, { + global: { + plugins: [i18n], + stubs: { + Button: ButtonStub, + SearchInput: SearchInputStub, + UserAvatar: true, + Menu: { template: '
', props: ['model', 'popup'] } + }, + directives: { tooltip: () => {} } + } + }) +} + +function createMember( + overrides: Partial = {} +): WorkspaceMember { + return { + id: 'member-1', + name: 'Member One', + email: 'member1@example.com', + joinDate: new Date('2025-01-15'), + role: 'member', + ...overrides + } +} + +function createInvite(overrides: Partial = {}): PendingInvite { + return { + id: 'invite-1', + email: 'invitee@example.com', + token: 'token-abc', + inviteDate: new Date('2025-03-01'), + expiryDate: new Date('2025-04-01'), + ...overrides + } +} + +describe('MembersPanelContent', () => { + beforeEach(() => { + vi.clearAllMocks() + mockMembers.value = [] + mockPendingInvites.value = [] + mockFilteredMembers.value = [] + mockFilteredPendingInvites.value = [] + mockIsPersonalWorkspace.value = false + mockIsSingleSeatPlan.value = false + mockIsActiveSubscription.value = true + mockActiveView.value = 'active' + mockSearchQuery.value = '' + mockPermissions.value = { + canViewOtherMembers: true, + canViewPendingInvites: true, + canInviteMembers: true, + canManageInvites: true, + canRemoveMembers: true, + canLeaveWorkspace: true, + canAccessWorkspaceMenu: true, + canManageSubscription: true, + canTopUp: true + } + mockUiConfig.value = { + showMembersList: true, + showPendingTab: true, + showSearch: true, + showDateColumn: true, + showRoleBadge: true, + membersGridCols: 'grid-cols-[50%_40%_10%]', + pendingGridCols: 'grid-cols-[50%_20%_20%_10%]', + headerGridCols: 'grid-cols-[50%_40%_10%]', + showEditWorkspaceMenuItem: true, + workspaceMenuAction: 'delete', + workspaceMenuDisabledTooltip: null + } + }) + + describe('personal workspace', () => { + beforeEach(() => { + mockIsPersonalWorkspace.value = true + mockUiConfig.value.showMembersList = false + mockUiConfig.value.showSearch = false + mockUiConfig.value.showPendingTab = false + }) + + it('shows personal workspace message and create workspace button', () => { + renderComponent() + expect( + screen.getByText('workspacePanel.members.personalWorkspaceMessage') + ).toBeTruthy() + expect( + screen.getByText('workspacePanel.members.createNewWorkspace') + ).toBeTruthy() + }) + + it('calls handleCreateWorkspace when create button is clicked', async () => { + renderComponent() + await userEvent.click( + screen.getByText('workspacePanel.members.createNewWorkspace') + ) + expect(mockHandleCreateWorkspace).toHaveBeenCalled() + }) + + it('does not show search input', () => { + renderComponent() + expect(screen.queryByRole('textbox')).toBeNull() + }) + }) + + describe('team workspace - member list', () => { + it('renders filtered members', () => { + mockFilteredMembers.value = [ + createMember({ name: 'Alice', email: 'alice@test.com' }), + createMember({ + id: '2', + name: 'Bob', + email: 'bob@test.com' + }) + ] + renderComponent() + expect(screen.getByText('Alice')).toBeTruthy() + expect(screen.getByText('Bob')).toBeTruthy() + }) + + it('shows more options button for non-current members', () => { + mockFilteredMembers.value = [ + createMember({ name: 'Other', email: 'other@test.com' }) + ] + renderComponent() + expect( + screen.queryAllByRole('button', { name: 'g.moreOptions' }) + ).toHaveLength(1) + }) + + it('does not show more options for current user', () => { + mockFilteredMembers.value = [ + createMember({ name: 'Owner User', email: 'owner@example.com' }) + ] + renderComponent() + expect( + screen.queryAllByRole('button', { name: 'g.moreOptions' }) + ).toHaveLength(0) + }) + }) + + describe('pending invites tab', () => { + it('shows pending tab button when configured', () => { + mockPendingInvites.value = [createInvite()] + renderComponent() + expect( + screen.getByText(/workspacePanel\.members\.tabs\.pendingCount/) + ).toBeTruthy() + }) + + it('triggers handleRevokeInvite on revoke click', async () => { + mockActiveView.value = 'pending' + mockFilteredPendingInvites.value = [createInvite({ id: 'inv-42' })] + renderComponent() + const revokeBtn = screen.getByRole('button', { + name: 'workspacePanel.members.actions.revokeInvite' + }) + await userEvent.click(revokeBtn) + expect(mockHandleRevokeInvite).toHaveBeenCalled() + }) + + it('triggers handleCopyInviteLink on copy click', async () => { + mockActiveView.value = 'pending' + mockFilteredPendingInvites.value = [createInvite({ id: 'inv-42' })] + renderComponent() + const copyBtn = screen.getByRole('button', { + name: 'workspacePanel.members.actions.copyLink' + }) + await userEvent.click(copyBtn) + expect(mockHandleCopyInviteLink).toHaveBeenCalled() + }) + }) + + describe('single seat plan', () => { + beforeEach(() => { + mockIsSingleSeatPlan.value = true + }) + + it('shows upsell banner', () => { + renderComponent() + expect( + screen.getByText('workspacePanel.members.upsellBannerUpgrade') + ).toBeTruthy() + }) + + it('opens subscription dialog on view plans click', async () => { + renderComponent() + const viewPlansBtn = screen.getByRole('button', { + name: /workspacePanel\.members\.viewPlans/ + }) + await userEvent.click(viewPlansBtn) + expect(mockShowSubscriptionDialog).toHaveBeenCalled() + }) + + it('hides search input', () => { + renderComponent() + expect(screen.queryByRole('textbox')).toBeNull() + }) + }) + + describe('member count display', () => { + it('shows member count header for team workspace', () => { + mockFilteredMembers.value = [ + createMember({ id: '1' }), + createMember({ id: '2' }) + ] + mockMembers.value = mockFilteredMembers.value + renderComponent() + expect( + screen.getByText(/workspacePanel\.members\.membersCount/) + ).toBeTruthy() + }) + }) +}) diff --git a/src/platform/workspace/components/dialogs/settings/MembersPanelContent.vue b/src/platform/workspace/components/dialogs/settings/MembersPanelContent.vue index c47c638b09..b62993c9cb 100644 --- a/src/platform/workspace/components/dialogs/settings/MembersPanelContent.vue +++ b/src/platform/workspace/components/dialogs/settings/MembersPanelContent.vue @@ -198,190 +198,47 @@ diff --git a/src/platform/workspace/composables/useMembersPanel.test.ts b/src/platform/workspace/composables/useMembersPanel.test.ts new file mode 100644 index 0000000000..7e9c71b29d --- /dev/null +++ b/src/platform/workspace/composables/useMembersPanel.test.ts @@ -0,0 +1,501 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest' +import { ref } from 'vue' + +import type { + PendingInvite, + WorkspaceMember +} from '@/platform/workspace/stores/teamWorkspaceStore' + +import { + filterBySearch, + sortMembers, + sortPendingInvites +} from './useMembersPanel' + +function createMember( + overrides: Partial = {} +): WorkspaceMember { + return { + id: 'member-1', + name: 'Member One', + email: 'member1@example.com', + joinDate: new Date('2025-01-15'), + role: 'member', + ...overrides + } +} + +function createInvite(overrides: Partial = {}): PendingInvite { + return { + id: 'invite-1', + email: 'invitee@example.com', + token: 'token-abc', + inviteDate: new Date('2025-03-01'), + expiryDate: new Date('2025-04-01'), + ...overrides + } +} + +describe('sortMembers', () => { + it('places owners before members', () => { + const owner = createMember({ id: 'o', role: 'owner', name: 'Owner' }) + const member = createMember({ id: 'm', role: 'member', name: 'Member' }) + const result = sortMembers([member, owner], null, 'desc') + expect(result[0].id).toBe('o') + expect(result[1].id).toBe('m') + }) + + it('places current user after owners but before others', () => { + const owner = createMember({ + id: 'o', + role: 'owner', + email: 'boss@test.com', + joinDate: new Date('2025-03-01') + }) + const current = createMember({ + id: 'me', + role: 'member', + email: 'me@test.com', + joinDate: new Date('2025-02-01') + }) + const other = createMember({ + id: 'other', + role: 'member', + email: 'other@test.com', + joinDate: new Date('2025-01-01') + }) + + const result = sortMembers([other, current, owner], 'me@test.com', 'desc') + expect(result.map((m) => m.id)).toEqual(['o', 'me', 'other']) + }) + + it('sorts remaining members by joinDate descending', () => { + const early = createMember({ + id: 'early', + joinDate: new Date('2025-01-01') + }) + const late = createMember({ + id: 'late', + joinDate: new Date('2025-06-01') + }) + const result = sortMembers([early, late], null, 'desc') + expect(result[0].id).toBe('late') + expect(result[1].id).toBe('early') + }) + + it('sorts remaining members by joinDate ascending', () => { + const early = createMember({ + id: 'early', + joinDate: new Date('2025-01-01') + }) + const late = createMember({ + id: 'late', + joinDate: new Date('2025-06-01') + }) + const result = sortMembers([early, late], null, 'asc') + expect(result[0].id).toBe('early') + expect(result[1].id).toBe('late') + }) + + it('does not mutate the input array', () => { + const members = [ + createMember({ id: 'b', joinDate: new Date('2025-06-01') }), + createMember({ id: 'a', joinDate: new Date('2025-01-01') }) + ] + const original = [...members] + sortMembers(members, null, 'desc') + expect(members).toEqual(original) + }) +}) + +describe('filterBySearch', () => { + const alice = createMember({ + id: '1', + name: 'Alice', + email: 'alice@example.com' + }) + const bob = createMember({ + id: '2', + name: 'Bob', + email: 'bob@example.com' + }) + + it('returns all items when query is empty', () => { + expect(filterBySearch([alice, bob], '')).toEqual([alice, bob]) + }) + + it('filters by name (case-insensitive)', () => { + const result = filterBySearch([alice, bob], 'alice') + expect(result).toEqual([alice]) + }) + + it('filters by email', () => { + const result = filterBySearch([alice, bob], 'bob@') + expect(result).toEqual([bob]) + }) + + it('filters pending invites by email only', () => { + const inv1 = createInvite({ id: 'i1', email: 'alice@test.com' }) + const inv2 = createInvite({ id: 'i2', email: 'bob@test.com' }) + const result = filterBySearch([inv1, inv2], 'alice') + expect(result).toEqual([inv1]) + }) +}) + +describe('sortPendingInvites', () => { + it('sorts by inviteDate descending by default', () => { + const early = createInvite({ + id: 'e', + inviteDate: new Date('2025-01-01') + }) + const late = createInvite({ + id: 'l', + inviteDate: new Date('2025-06-01') + }) + const result = sortPendingInvites([early, late], 'inviteDate', 'desc') + expect(result[0].id).toBe('l') + expect(result[1].id).toBe('e') + }) + + it('sorts by expiryDate ascending', () => { + const early = createInvite({ + id: 'e', + expiryDate: new Date('2025-01-01') + }) + const late = createInvite({ + id: 'l', + expiryDate: new Date('2025-06-01') + }) + const result = sortPendingInvites([early, late], 'expiryDate', 'asc') + expect(result[0].id).toBe('e') + expect(result[1].id).toBe('l') + }) + + it('falls back to inviteDate when sortField is joinDate', () => { + const early = createInvite({ + id: 'e', + inviteDate: new Date('2025-01-01') + }) + const late = createInvite({ + id: 'l', + inviteDate: new Date('2025-06-01') + }) + const result = sortPendingInvites([early, late], 'joinDate', 'desc') + expect(result[0].id).toBe('l') + }) + + it('does not mutate the input array', () => { + const invites = [ + createInvite({ + id: 'b', + inviteDate: new Date('2025-06-01') + }), + createInvite({ + id: 'a', + inviteDate: new Date('2025-01-01') + }) + ] + const original = [...invites] + sortPendingInvites(invites, 'inviteDate', 'desc') + expect(invites).toEqual(original) + }) +}) + +const mockToastAdd = vi.fn() +const mockCopyInviteLink = vi.fn() +const mockShowRemoveMemberDialog = vi.fn() +const mockShowRevokeInviteDialog = vi.fn() +const mockShowCreateWorkspaceDialog = vi.fn() +const mockShowSubscriptionDialog = vi.fn() + +const { + mockMembers, + mockPendingInvites, + mockIsInPersonalWorkspace, + mockPermissions, + mockUiConfig, + mockIsActiveSubscription, + mockSubscription +} = vi.hoisted(() => { + // eslint-disable-next-line @typescript-eslint/no-require-imports, @typescript-eslint/consistent-type-imports + const { ref } = require('vue') as typeof import('vue') + + return { + mockMembers: ref([]), + mockPendingInvites: ref([]), + mockIsInPersonalWorkspace: ref(false), + mockPermissions: ref({ + canViewOtherMembers: true, + canViewPendingInvites: true, + canInviteMembers: true, + canManageInvites: true, + canRemoveMembers: true, + canLeaveWorkspace: true, + canAccessWorkspaceMenu: true, + canManageSubscription: true, + canTopUp: true + }), + mockUiConfig: ref({ + showMembersList: true, + showPendingTab: true, + showSearch: true, + showDateColumn: true, + showRoleBadge: true, + membersGridCols: 'grid-cols-[50%_40%_10%]', + pendingGridCols: 'grid-cols-[50%_20%_20%_10%]', + headerGridCols: 'grid-cols-[50%_40%_10%]', + showEditWorkspaceMenuItem: true, + workspaceMenuAction: 'delete' as 'leave' | 'delete' | null, + workspaceMenuDisabledTooltip: null as string | null + }), + mockIsActiveSubscription: ref(true), + mockSubscription: ref<{ tier: string } | null>({ tier: 'PRO' }) + } +}) + +vi.mock('primevue/usetoast', () => ({ + useToast: () => ({ add: mockToastAdd }) +})) + +vi.mock('vue-i18n', () => ({ + useI18n: () => ({ t: (key: string) => key }) +})) + +vi.mock('pinia', async (importOriginal) => { + const actual = await importOriginal() + return { + ...(actual as object), + storeToRefs: (store: Record) => store + } +}) + +vi.mock('@/platform/workspace/stores/teamWorkspaceStore', () => ({ + useTeamWorkspaceStore: () => ({ + members: mockMembers, + pendingInvites: mockPendingInvites, + isInPersonalWorkspace: mockIsInPersonalWorkspace, + copyInviteLink: mockCopyInviteLink + }) +})) + +vi.mock('@/platform/workspace/composables/useWorkspaceUI', () => ({ + useWorkspaceUI: () => ({ + permissions: mockPermissions, + uiConfig: mockUiConfig + }) +})) + +vi.mock('@/composables/auth/useCurrentUser', () => ({ + useCurrentUser: () => ({ + userPhotoUrl: ref(null), + userEmail: ref('owner@example.com'), + userDisplayName: ref('Owner User') + }) +})) + +vi.mock('@/composables/billing/useBillingContext', () => ({ + useBillingContext: () => ({ + isActiveSubscription: mockIsActiveSubscription, + subscription: mockSubscription, + showSubscriptionDialog: mockShowSubscriptionDialog, + getMaxSeats: (tierKey: string) => { + const seats: Record = { + free: 1, + standard: 1, + creator: 5, + pro: 20 + } + return seats[tierKey] ?? 1 + } + }) +})) + +vi.mock('@/platform/cloud/subscription/constants/tierPricing', () => ({ + TIER_TO_KEY: { + FREE: 'free', + STANDARD: 'standard', + CREATOR: 'creator', + PRO: 'pro', + FOUNDERS_EDITION: 'founder' + } +})) + +vi.mock('@/services/dialogService', () => ({ + useDialogService: () => ({ + showRemoveMemberDialog: mockShowRemoveMemberDialog, + showRevokeInviteDialog: mockShowRevokeInviteDialog, + showCreateWorkspaceDialog: mockShowCreateWorkspaceDialog + }) +})) + +describe('useMembersPanel', () => { + beforeEach(() => { + vi.clearAllMocks() + mockMembers.value = [] + mockPendingInvites.value = [] + mockIsInPersonalWorkspace.value = false + mockIsActiveSubscription.value = true + mockSubscription.value = { tier: 'PRO' } + }) + + // Lazy import so mocks are in place + async function setup() { + const { useMembersPanel } = await import('./useMembersPanel') + return useMembersPanel() + } + + describe('isSingleSeatPlan', () => { + it('is false for personal workspace', async () => { + mockIsInPersonalWorkspace.value = true + const panel = await setup() + expect(panel.isSingleSeatPlan.value).toBe(false) + }) + + it('is true when no active subscription', async () => { + mockIsActiveSubscription.value = false + const panel = await setup() + expect(panel.isSingleSeatPlan.value).toBe(true) + }) + + it('is true for standard tier (1 seat)', async () => { + mockSubscription.value = { tier: 'STANDARD' } + const panel = await setup() + expect(panel.isSingleSeatPlan.value).toBe(true) + }) + + it('is false for pro tier (20 seats)', async () => { + mockSubscription.value = { tier: 'PRO' } + const panel = await setup() + expect(panel.isSingleSeatPlan.value).toBe(false) + }) + }) + + describe('maxSeats', () => { + it('returns 1 for personal workspace', async () => { + mockIsInPersonalWorkspace.value = true + const panel = await setup() + expect(panel.maxSeats.value).toBe(1) + }) + + it('returns tier-appropriate seats', async () => { + mockSubscription.value = { tier: 'CREATOR' } + const panel = await setup() + expect(panel.maxSeats.value).toBe(5) + }) + + it('returns 1 when no subscription', async () => { + mockSubscription.value = null + const panel = await setup() + expect(panel.maxSeats.value).toBe(1) + }) + }) + + describe('personalWorkspaceMember', () => { + it('uses current user info', async () => { + const panel = await setup() + expect(panel.personalWorkspaceMember.value).toMatchObject({ + id: 'self', + name: 'Owner User', + email: 'owner@example.com', + role: 'owner' + }) + }) + }) + + describe('isCurrentUser', () => { + it('matches by email (case-insensitive)', async () => { + const panel = await setup() + const member = createMember({ email: 'OWNER@EXAMPLE.COM' }) + expect(panel.isCurrentUser(member)).toBe(true) + }) + + it('returns false for other users', async () => { + const panel = await setup() + const member = createMember({ email: 'other@example.com' }) + expect(panel.isCurrentUser(member)).toBe(false) + }) + }) + + describe('filteredMembers', () => { + it('filters and sorts members based on searchQuery', async () => { + const alice = createMember({ + id: '1', + name: 'Alice', + email: 'alice@example.com' + }) + const bob = createMember({ + id: '2', + name: 'Bob', + email: 'bob@example.com' + }) + mockMembers.value = [alice, bob] + const panel = await setup() + + panel.searchQuery.value = 'alice' + expect(panel.filteredMembers.value).toHaveLength(1) + expect(panel.filteredMembers.value[0].name).toBe('Alice') + }) + }) + + describe('toggleSort', () => { + it('changes sortField and resets to desc', async () => { + const panel = await setup() + panel.toggleSort('expiryDate') + expect(panel.sortField.value).toBe('expiryDate') + expect(panel.sortDirection.value).toBe('desc') + }) + + it('toggles direction when same field', async () => { + const panel = await setup() + panel.toggleSort('inviteDate') + expect(panel.sortDirection.value).toBe('asc') + panel.toggleSort('inviteDate') + expect(panel.sortDirection.value).toBe('desc') + }) + }) + + describe('handleCopyInviteLink', () => { + it('shows success toast on success', async () => { + mockCopyInviteLink.mockResolvedValue(undefined) + const panel = await setup() + await panel.handleCopyInviteLink(createInvite({ id: 'inv-1' })) + expect(mockCopyInviteLink).toHaveBeenCalledWith('inv-1') + expect(mockToastAdd).toHaveBeenCalledWith( + expect.objectContaining({ severity: 'success' }) + ) + }) + + it('shows error toast on failure', async () => { + mockCopyInviteLink.mockRejectedValue(new Error('fail')) + const panel = await setup() + await panel.handleCopyInviteLink(createInvite({ id: 'inv-1' })) + expect(mockToastAdd).toHaveBeenCalledWith( + expect.objectContaining({ severity: 'error' }) + ) + }) + }) + + describe('handleRevokeInvite', () => { + it('calls showRevokeInviteDialog', async () => { + const panel = await setup() + panel.handleRevokeInvite(createInvite({ id: 'inv-42' })) + expect(mockShowRevokeInviteDialog).toHaveBeenCalledWith('inv-42') + }) + }) + + describe('handleCreateWorkspace', () => { + it('calls showCreateWorkspaceDialog', async () => { + const panel = await setup() + panel.handleCreateWorkspace() + expect(mockShowCreateWorkspaceDialog).toHaveBeenCalled() + }) + }) + + describe('handleRemoveMember', () => { + it('calls showRemoveMemberDialog', async () => { + const panel = await setup() + panel.handleRemoveMember(createMember({ id: 'mem-7' })) + expect(mockShowRemoveMemberDialog).toHaveBeenCalledWith('mem-7') + }) + }) +}) diff --git a/src/platform/workspace/composables/useMembersPanel.ts b/src/platform/workspace/composables/useMembersPanel.ts new file mode 100644 index 0000000000..502c0d699b --- /dev/null +++ b/src/platform/workspace/composables/useMembersPanel.ts @@ -0,0 +1,219 @@ +import { storeToRefs } from 'pinia' +import { useToast } from 'primevue/usetoast' +import { computed, ref } from 'vue' +import { useI18n } from 'vue-i18n' + +import { useCurrentUser } from '@/composables/auth/useCurrentUser' +import { useBillingContext } from '@/composables/billing/useBillingContext' +import { TIER_TO_KEY } from '@/platform/cloud/subscription/constants/tierPricing' +import { useWorkspaceUI } from '@/platform/workspace/composables/useWorkspaceUI' +import type { + PendingInvite, + WorkspaceMember +} from '@/platform/workspace/stores/teamWorkspaceStore' +import { useTeamWorkspaceStore } from '@/platform/workspace/stores/teamWorkspaceStore' +import { useDialogService } from '@/services/dialogService' + +type ActiveView = 'active' | 'pending' +type SortField = 'inviteDate' | 'expiryDate' | 'joinDate' +type SortDirection = 'asc' | 'desc' + +export function sortMembers( + members: WorkspaceMember[], + currentUserEmail: string | null, + sortDirection: SortDirection +): WorkspaceMember[] { + return [...members].sort((a, b) => { + if (a.role === 'owner' && b.role !== 'owner') return -1 + if (a.role !== 'owner' && b.role === 'owner') return 1 + + const aIsCurrent = a.email.toLowerCase() === currentUserEmail?.toLowerCase() + const bIsCurrent = b.email.toLowerCase() === currentUserEmail?.toLowerCase() + if (aIsCurrent && !bIsCurrent) return -1 + if (!aIsCurrent && bIsCurrent) return 1 + + const aValue = a.joinDate.getTime() + const bValue = b.joinDate.getTime() + return sortDirection === 'asc' ? aValue - bValue : bValue - aValue + }) +} + +export function filterBySearch( + items: T[], + query: string +): T[] { + if (!query) return items + const q = query.toLowerCase() + return items.filter( + (item) => + item.email.toLowerCase().includes(q) || + ('name' in item && item.name?.toLowerCase().includes(q)) + ) +} + +export function sortPendingInvites( + invites: PendingInvite[], + sortField: SortField, + sortDirection: SortDirection +): PendingInvite[] { + const field = sortField === 'joinDate' ? 'inviteDate' : sortField + return [...invites].sort((a, b) => { + const aDate = a[field] + const bDate = b[field] + if (!aDate || !bDate) return 0 + const aValue = aDate.getTime() + const bValue = bDate.getTime() + return sortDirection === 'asc' ? aValue - bValue : bValue - aValue + }) +} + +export function useMembersPanel() { + const { t } = useI18n() + const toast = useToast() + const { userPhotoUrl, userEmail, userDisplayName } = useCurrentUser() + const { + showRemoveMemberDialog, + showRevokeInviteDialog, + showCreateWorkspaceDialog + } = useDialogService() + const workspaceStore = useTeamWorkspaceStore() + const { + members, + pendingInvites, + isInPersonalWorkspace: isPersonalWorkspace + } = storeToRefs(workspaceStore) + const { copyInviteLink } = workspaceStore + const { permissions, uiConfig } = useWorkspaceUI() + const { + isActiveSubscription, + subscription, + showSubscriptionDialog, + getMaxSeats + } = useBillingContext() + + const maxSeats = computed(() => { + if (isPersonalWorkspace.value) return 1 + const tier = subscription.value?.tier + if (!tier) return 1 + const tierKey = TIER_TO_KEY[tier] + if (!tierKey) return 1 + return getMaxSeats(tierKey) + }) + + const isSingleSeatPlan = computed(() => { + if (isPersonalWorkspace.value) return false + if (!isActiveSubscription.value) return true + return maxSeats.value <= 1 + }) + + const personalWorkspaceMember = computed(() => ({ + id: 'self', + name: userDisplayName.value ?? '', + email: userEmail.value ?? '', + role: 'owner' as const, + joinDate: new Date(0) + })) + + const searchQuery = ref('') + const activeView = ref('active') + const sortField = ref('inviteDate') + const sortDirection = ref('desc') + + const selectedMember = ref(null) + + const memberMenuItems = computed(() => [ + { + label: t('workspacePanel.members.actions.removeMember'), + icon: 'pi pi-user-minus', + command: () => { + if (selectedMember.value) { + handleRemoveMember(selectedMember.value) + } + } + } + ]) + + function selectMember(member: WorkspaceMember) { + selectedMember.value = member + } + + function isCurrentUser(member: WorkspaceMember): boolean { + return member.email.toLowerCase() === userEmail.value?.toLowerCase() + } + + const filteredMembers = computed(() => { + const searched = filterBySearch(members.value, searchQuery.value) + return sortMembers(searched, userEmail.value ?? null, sortDirection.value) + }) + + const filteredPendingInvites = computed(() => { + const searched = filterBySearch(pendingInvites.value, searchQuery.value) + return sortPendingInvites(searched, sortField.value, sortDirection.value) + }) + + function toggleSort(field: SortField) { + if (sortField.value === field) { + sortDirection.value = sortDirection.value === 'asc' ? 'desc' : 'asc' + } else { + sortField.value = field + sortDirection.value = 'desc' + } + } + + async function handleCopyInviteLink(invite: PendingInvite) { + try { + await copyInviteLink(invite.id) + toast.add({ + severity: 'success', + summary: t('g.copied'), + life: 2000 + }) + } catch { + toast.add({ + severity: 'error', + summary: t('g.error') + }) + } + } + + function handleRevokeInvite(invite: PendingInvite) { + void showRevokeInviteDialog(invite.id) + } + + function handleCreateWorkspace() { + void showCreateWorkspaceDialog() + } + + function handleRemoveMember(member: WorkspaceMember) { + void showRemoveMemberDialog(member.id) + } + + return { + searchQuery, + activeView, + sortField, + sortDirection, + selectedMember, + maxSeats, + isSingleSeatPlan, + personalWorkspaceMember, + filteredMembers, + filteredPendingInvites, + memberMenuItems, + isPersonalWorkspace, + members, + pendingInvites, + permissions, + uiConfig, + isActiveSubscription, + userPhotoUrl, + isCurrentUser, + selectMember, + toggleSort, + showSubscriptionDialog, + handleCopyInviteLink, + handleRevokeInvite, + handleCreateWorkspace, + handleRemoveMember + } +} From 8b1d564729a9b8de395a3ba1aedb9620c60e32f1 Mon Sep 17 00:00:00 2001 From: LifetimeVip <275470805+LifetimeVip@users.noreply.github.com> Date: Tue, 5 May 2026 08:05:51 +0800 Subject: [PATCH 005/172] chore(i18n): correct zh and zh-TW translations (#11930) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary Fixes several issues in both Simplified Chinese (zh) and Traditional Chinese (zh-TW) locale files that were identified through systematic comparison against the English source. ### Changes **nodeDefs.json (zh + zh-TW):** - **CLIPLoader.description**: Added missing model recipes (lumina2, wan, hidream, omnigen2) to match English source - **ByteDanceSeedreamNode.display_name**: Updated version from "Seedream 4" to "Seedream 4.5 and 5.0" to match English - **BriaImageEditNode.display_name**: Added missing "FIBO" model name **nodeDefs.json (zh only):** - **APG.inputs.eta.name**: Fixed incorrect translation "预计到达时间" (ETA) -> kept as "eta" (technical parameter name) **commands.json (zh + zh-TW):** - **Comfy_ToggleLinear**: Updated label to match English "Toggle App Mode" - **Experimental_ToggleVueNodes**: Rebranded "Vue 节点"/"Vue 節點" to "Nodes 2.0" to match English **settings.json (zh + zh-TW):** - **Comfy_VueNodes_Enabled / Comfy_VueNodes_AutoScaleLayout**: Rebranded "Vue 节点"/"Vue 節點" to "Nodes 2.0" **main.json (zh + zh-TW):** - **errorDialog.accessRestrictedMessage / accessRestrictedTitle**: Added missing Chinese translations ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-11930-fix-i18n-correct-zh-and-zh-TW-translations-3566d73d365081ff9b0beb1c1fc7ef1a) by [Unito](https://www.unito.io) --------- Co-authored-by: LifetimeVip --- src/locales/zh-TW/commands.json | 4 ++-- src/locales/zh-TW/main.json | 4 ++-- src/locales/zh-TW/nodeDefs.json | 6 +++--- src/locales/zh-TW/settings.json | 4 ++-- src/locales/zh/commands.json | 4 ++-- src/locales/zh/main.json | 4 ++-- src/locales/zh/nodeDefs.json | 8 ++++---- src/locales/zh/settings.json | 4 ++-- 8 files changed, 19 insertions(+), 19 deletions(-) diff --git a/src/locales/zh-TW/commands.json b/src/locales/zh-TW/commands.json index 8e7f9a8dad..0ccb227fba 100644 --- a/src/locales/zh-TW/commands.json +++ b/src/locales/zh-TW/commands.json @@ -294,7 +294,7 @@ "label": "說明中心" }, "Comfy_ToggleLinear": { - "label": "切換線性模式" + "label": "切換應用模式" }, "Comfy_ToggleQPOV2": { "label": "切換佇列面板 V2" @@ -312,7 +312,7 @@ "label": "登出" }, "Experimental_ToggleVueNodes": { - "label": "實驗性:啟用 Vue 節點" + "label": "啟用 Nodes 2.0" }, "Workspace_CloseWorkflow": { "label": "關閉當前工作流程" diff --git a/src/locales/zh-TW/main.json b/src/locales/zh-TW/main.json index 1028cf7685..fc5a2036eb 100644 --- a/src/locales/zh-TW/main.json +++ b/src/locales/zh-TW/main.json @@ -903,8 +903,8 @@ "resume": "繼續下載" }, "errorDialog": { - "accessRestrictedMessage": "Your account is not authorized for this feature.", - "accessRestrictedTitle": "Access Restricted", + "accessRestrictedMessage": "您的帳戶無權使用此功能。", + "accessRestrictedTitle": "存取受限", "defaultTitle": "發生錯誤", "extensionFileHint": "這可能是由於以下指令碼所致", "loadWorkflowTitle": "由於重新載入工作流程資料時發生錯誤,已中止載入", diff --git a/src/locales/zh-TW/nodeDefs.json b/src/locales/zh-TW/nodeDefs.json index d7a16d4c04..425733ccaf 100644 --- a/src/locales/zh-TW/nodeDefs.json +++ b/src/locales/zh-TW/nodeDefs.json @@ -371,7 +371,7 @@ }, "BriaImageEditNode": { "description": "使用 Bria 最新模型編輯圖像", - "display_name": "Bria 圖像編輯", + "display_name": "Bria FIBO 圖像編輯", "inputs": { "control_after_generate": { "name": "生成後控制" @@ -864,7 +864,7 @@ }, "ByteDanceSeedreamNode": { "description": "統一文字生成圖片和精確單句編輯,最高支援 4K 解析度。", - "display_name": "字節跳動 Seedream 4", + "display_name": "ByteDance Seedream 4.5 & 5.0", "inputs": { "control_after_generate": { "name": "生成後控制" @@ -1047,7 +1047,7 @@ } }, "CLIPLoader": { - "description": "[配方]\n\nstable_diffusion: clip-l\nstable_cascade: clip-g\nsd3: t5 xxl/ clip-g / clip-l\nstable_audio: t5 base\nmochi: t5 xxl\ncosmos: 舊 t5 xxl\nlumina2: gemma 2 2B\nwan: umt5 xxl\nhidream: llama-3.1(推薦)或 t5", + "description": "[配方]\n\nstable_diffusion: clip-l\nstable_cascade: clip-g\nsd3: t5 xxl/ clip-g / clip-l\nstable_audio: t5 base\nmochi: t5 xxl\ncosmos: 舊 t5 xxl\nlumina2: gemma 2 2B\nwan: umt5 xxl\nhidream: llama-3.1(推薦)或 t5\nomnigen2: qwen vl 2.5 3B", "display_name": "載入 CLIP", "inputs": { "clip_name": { diff --git a/src/locales/zh-TW/settings.json b/src/locales/zh-TW/settings.json index eb734abe60..21b4567f2f 100644 --- a/src/locales/zh-TW/settings.json +++ b/src/locales/zh-TW/settings.json @@ -428,11 +428,11 @@ "name": "驗證工作流程" }, "Comfy_VueNodes_AutoScaleLayout": { - "name": "自動縮放佈局 (Vue 節點)", + "name": "自動縮放佈局 (Nodes 2.0)", "tooltip": "切換至 Vue 渲染時自動縮放節點位置以防止重疊" }, "Comfy_VueNodes_Enabled": { - "name": "現代節點設計 (Vue 節點)", + "name": "現代節點設計 (Nodes 2.0)", "tooltip": "現代:基於 DOM 的渲染,具備增強的互動性、原生瀏覽器功能和更新的視覺設計。經典:傳統畫布渲染。" }, "Comfy_WidgetControlMode": { diff --git a/src/locales/zh/commands.json b/src/locales/zh/commands.json index 2b35705ed6..d9e71ed898 100644 --- a/src/locales/zh/commands.json +++ b/src/locales/zh/commands.json @@ -294,7 +294,7 @@ "label": "说明中心" }, "Comfy_ToggleLinear": { - "label": "切换线性模式" + "label": "切换应用模式" }, "Comfy_ToggleQPOV2": { "label": "切换队列面板V2" @@ -312,7 +312,7 @@ "label": "退出登录" }, "Experimental_ToggleVueNodes": { - "label": "实验性:启用 Vue 节点" + "label": "启用 Nodes 2.0" }, "Workspace_CloseWorkflow": { "label": "关闭当前工作流" diff --git a/src/locales/zh/main.json b/src/locales/zh/main.json index 9b52a78af5..e8a08e38d6 100644 --- a/src/locales/zh/main.json +++ b/src/locales/zh/main.json @@ -903,8 +903,8 @@ "resume": "恢复下载" }, "errorDialog": { - "accessRestrictedMessage": "Your account is not authorized for this feature.", - "accessRestrictedTitle": "Access Restricted", + "accessRestrictedMessage": "您的账户无权使用此功能。", + "accessRestrictedTitle": "访问受限", "defaultTitle": "发生错误", "extensionFileHint": "这可能是由于以下脚本", "loadWorkflowTitle": "由于重新加载工作流数据出错,加载被中止", diff --git a/src/locales/zh/nodeDefs.json b/src/locales/zh/nodeDefs.json index 97060bb91d..58c1e33023 100644 --- a/src/locales/zh/nodeDefs.json +++ b/src/locales/zh/nodeDefs.json @@ -3,7 +3,7 @@ "display_name": "自适应投影引导", "inputs": { "eta": { - "name": "预计到达时间", + "name": "eta", "tooltip": "控制平行引导向量的缩放比例。默认 CFG 行为设置为 1。" }, "model": { @@ -371,7 +371,7 @@ }, "BriaImageEditNode": { "description": "使用 Bria 最新模型编辑图像", - "display_name": "Bria 图像编辑", + "display_name": "Bria FIBO 图像编辑", "inputs": { "control_after_generate": { "name": "control after generate" @@ -864,7 +864,7 @@ }, "ByteDanceSeedreamNode": { "description": "统一文本到图像生成,支持高达4K分辨率的精确单句编辑。", - "display_name": "字节跳动Seedream 4", + "display_name": "ByteDance Seedream 4.5 & 5.0", "inputs": { "control_after_generate": { "name": "生成后控制" @@ -1047,7 +1047,7 @@ } }, "CLIPLoader": { - "description": "[配方]\n\nStable Diffusion:clip-l\nStable Cascade:clip-g\nSD3:t5 / clip-g / clip-l\nStable Audio:t5\nMochi:t5\ncosmos:old t5 xxl", + "description": "[配方]\n\nstable_diffusion: clip-l\nstable_cascade: clip-g\nsd3: t5 xxl/ clip-g / clip-l\nstable_audio: t5 base\nmochi: t5 xxl\ncosmos: 旧 t5 xxl\nlumina2: gemma 2 2B\nwan: umt5 xxl\nhidream: llama-3.1(推荐)或 t5\nomnigen2: qwen vl 2.5 3B", "display_name": "加载CLIP", "inputs": { "clip_name": { diff --git a/src/locales/zh/settings.json b/src/locales/zh/settings.json index 5d64012ecf..1f77b665bb 100644 --- a/src/locales/zh/settings.json +++ b/src/locales/zh/settings.json @@ -428,11 +428,11 @@ "name": "校验工作流" }, "Comfy_VueNodes_AutoScaleLayout": { - "name": "自动缩放布局(Vue节点)", + "name": "自动缩放布局(Nodes 2.0)", "tooltip": "切换到Vue渲染时自动缩放节点位置以防止重叠" }, "Comfy_VueNodes_Enabled": { - "name": "现代节点设计(Vue节点)", + "name": "现代节点设计(Nodes 2.0)", "tooltip": "现代:基于DOM的渲染,具有增强的交互性、原生浏览器功能和更新的视觉设计。经典:传统的画布渲染。" }, "Comfy_WidgetControlMode": { From 7b59c561fff577745f610aed36fac5e2977cf943 Mon Sep 17 00:00:00 2001 From: Kelly Yang <124ykl@gmail.com> Date: Mon, 4 May 2026 17:25:55 -0700 Subject: [PATCH 006/172] fix(load3d): update renderer pixel ratio on canvas zoom to fix LOD resolution (#11734) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary Preview 3D and Animation nodes were stuck at the LOD from initial page load because CSS `scale3d` transforms don't affect `clientWidth`/`clientHeight` — `handleResize()` always received layout-space dimensions regardless of zoom level. This fix passes `ds.scale` as the renderer pixel ratio so the 3D scene renders at the correct visual resolution when the graph is zoomed in or out. ## Changes - **What**: In `Load3d.handleResize()`, call `renderer.setPixelRatio(ds.scale)` before `setSize` so pixel density scales with canvas zoom. A `getZoomScale` callback is threaded through `Load3DOptions` → `Load3d` constructor → `handleResize`. In `useLoad3d`, a watcher on `canvasStore.appScalePercentage` triggers `handleResize` whenever the zoom level changes. - **What**: Fix `SceneManager.captureScene()` to save and restore the renderer's logical size and pixel ratio around capture, so exact-pixel output is unaffected by the current zoom state. ## Review Focus - `handleResize` now calls `setPixelRatio` before `setSize`. Three.js renders at `logicalWidth × pixelRatio` physical pixels while CSS displays it at `logicalWidth` CSS pixels — this is the standard pattern for HiDPI but here used to match the visual zoom level. - `captureScene` must reset `pixelRatio` to 1 so `setSize(w, h)` produces exactly `w×h` pixel output. It saves and restores both logical size and pixel ratio via `renderer.getSize()` / `renderer.getPixelRatio()`. - The zoom watcher is guarded with `getActivePinia()` to avoid errors in unit tests and non-Pinia contexts. ## Test before https://github.com/user-attachments/assets/9778ad54-7cb2-4fdc-b200-65a683ee8e4d after https://github.com/user-attachments/assets/acfaaf7a-43c7-495f-b352-5dd2cdaa94db ## Analysis Report https://linear.app/comfyorg/issue/FE-401/bug-preview-3d-and-animation-nodes-lod-stuck-at-initial-page-load ## More - Add `debounce` and pixel ratio limit --- > [!NOTE] > **Medium Risk** > Medium risk because it changes core `Load3d.handleResize()` rendering behavior (pixel ratio/LOD) and adds a debounced zoom-driven resize watcher, which could affect performance or visual output across all Load3D nodes. Capture logic is also refactored to manipulate renderer size/pixel ratio and camera params, so regressions would show up in thumbnails/exports. > > **Overview** > Fixes Load3D LOD/render sharpness when the graph canvas is zoomed by threading a new `getZoomScale` option from `useLoad3d` into `Load3d` and using it to call `renderer.setPixelRatio()` (clamped) during `handleResize()`. > > Adds a debounced watcher on `canvasStore.appScalePercentage` to trigger `handleResize()` on zoom changes, and updates `SceneManager.captureScene()` to temporarily force pixel ratio 1 and restore renderer size/pixel ratio and camera settings after capture. Coverage is expanded with new Playwright smoke coverage plus unit tests for zoom propagation, debouncing, pixel ratio behavior, and capture state restoration. > > Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit 261940d111057e71e19a3ab214676c24e1ab6d36. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot). ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-11734-fix-load3d-update-renderer-pixel-ratio-on-canvas-zoom-to-fix-LOD-resolution-3516d73d365081e6b3d4cdd05f516489) by [Unito](https://www.unito.io) --- browser_tests/tests/load3d/load3dLod.spec.ts | 32 +++ src/composables/useLoad3d.test.ts | 85 ++++++- src/composables/useLoad3d.ts | 13 +- src/extensions/core/load3d/Load3d.test.ts | 66 +++++- src/extensions/core/load3d/Load3d.ts | 7 + .../core/load3d/SceneManager.test.ts | 124 ++++++++++ src/extensions/core/load3d/SceneManager.ts | 222 ++++++++++-------- src/extensions/core/load3d/interfaces.ts | 5 + 8 files changed, 449 insertions(+), 105 deletions(-) create mode 100644 browser_tests/tests/load3d/load3dLod.spec.ts diff --git a/browser_tests/tests/load3d/load3dLod.spec.ts b/browser_tests/tests/load3d/load3dLod.spec.ts new file mode 100644 index 0000000000..ef51612fd7 --- /dev/null +++ b/browser_tests/tests/load3d/load3dLod.spec.ts @@ -0,0 +1,32 @@ +import { expect } from '@playwright/test' + +import { load3dTest as test } from '@e2e/fixtures/helpers/Load3DFixtures' + +test.describe('Load3D LOD', () => { + test( + 'canvas pixel dimensions scale with ComfyUI canvas zoom level', + { tag: '@smoke' }, + async ({ comfyPage, load3d }) => { + await expect(load3d.canvas).toBeVisible() + + await expect + .poll(() => load3d.canvas.evaluate((el: HTMLCanvasElement) => el.width)) + .toBeGreaterThan(0) + + const initialWidth = await load3d.canvas.evaluate( + (el: HTMLCanvasElement) => el.width + ) + + await comfyPage.page.evaluate(() => { + const node = window.app!.graph!.nodes[0] + window.app!.canvas.ds.scale = 2.0 + node.onResize?.(node.size) + }) + await comfyPage.nextFrame() + + await expect + .poll(() => load3d.canvas.evaluate((el: HTMLCanvasElement) => el.width)) + .toBeGreaterThan(initialWidth) + } + ) +}) diff --git a/src/composables/useLoad3d.test.ts b/src/composables/useLoad3d.test.ts index 9c0027d21b..d969cf2356 100644 --- a/src/composables/useLoad3d.test.ts +++ b/src/composables/useLoad3d.test.ts @@ -1,5 +1,7 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' -import { nextTick, ref, shallowRef } from 'vue' +import { nextTick, reactive, ref, shallowRef } from 'vue' +import type { Pinia } from 'pinia' +import { getActivePinia } from 'pinia' import { nodeToLoad3dMap, useLoad3d } from '@/composables/useLoad3d' import Load3d from '@/extensions/core/load3d/Load3d' @@ -9,6 +11,7 @@ import type { Size } from '@/lib/litegraph/src/interfaces' import type { LGraph } from '@/lib/litegraph/src/LGraph' import type { LGraphNode } from '@/lib/litegraph/src/LGraphNode' import type { IWidget } from '@/lib/litegraph/src/types/widgets' +import { useCanvasStore } from '@/renderer/core/canvas/canvasStore' import { useToastStore } from '@/platform/updates/common/toastStore' import { api } from '@/scripts/api' import { @@ -59,6 +62,18 @@ vi.mock('@/i18n', () => ({ t: vi.fn((key) => key) })) +vi.mock('pinia', async (importOriginal) => { + const actual = await importOriginal() + return { + ...(actual as Record), + getActivePinia: vi.fn(() => null) + } +}) + +vi.mock('@/renderer/core/canvas/canvasStore', () => ({ + useCanvasStore: vi.fn() +})) + describe('useLoad3d', () => { let mockLoad3d: Partial let mockNode: LGraphNode @@ -67,6 +82,7 @@ describe('useLoad3d', () => { beforeEach(() => { vi.clearAllMocks() nodeToLoad3dMap.clear() + vi.mocked(getActivePinia).mockReturnValue(null as unknown as Pinia) mockNode = createMockLGraphNode({ properties: { @@ -334,6 +350,73 @@ describe('useLoad3d', () => { expect(composable.sceneConfig.value.backgroundColor).toBe('#000000') }) + + it('passes getZoomScale callback to createLoad3d', async () => { + const composable = useLoad3d(mockNode) + const containerRef = document.createElement('div') + + await composable.initializeLoad3d(containerRef) + + expect(createLoad3d).toHaveBeenCalledWith( + containerRef, + expect.objectContaining({ getZoomScale: expect.any(Function) }) + ) + }) + }) + + describe('zoom watcher', () => { + it('calls load3d.handleResize after debounce when canvas appScalePercentage changes', async () => { + vi.useFakeTimers() + + const canvasStore = reactive({ appScalePercentage: 100 }) + vi.mocked(getActivePinia).mockReturnValue({} as unknown as Pinia) + vi.mocked(useCanvasStore).mockReturnValue( + canvasStore as unknown as ReturnType + ) + + const composable = useLoad3d(mockNode) + const containerRef = document.createElement('div') + await composable.initializeLoad3d(containerRef) + + vi.mocked(mockLoad3d.handleResize!).mockClear() + + canvasStore.appScalePercentage = 200 + await nextTick() + expect(mockLoad3d.handleResize).not.toHaveBeenCalled() + + vi.advanceTimersByTime(150) + expect(mockLoad3d.handleResize).toHaveBeenCalledOnce() + + vi.useRealTimers() + }) + + it('debounces rapid zoom changes into a single handleResize call', async () => { + vi.useFakeTimers() + + const canvasStore = reactive({ appScalePercentage: 100 }) + vi.mocked(getActivePinia).mockReturnValue({} as unknown as Pinia) + vi.mocked(useCanvasStore).mockReturnValue( + canvasStore as unknown as ReturnType + ) + + const composable = useLoad3d(mockNode) + const containerRef = document.createElement('div') + await composable.initializeLoad3d(containerRef) + + vi.mocked(mockLoad3d.handleResize!).mockClear() + + canvasStore.appScalePercentage = 150 + await nextTick() + canvasStore.appScalePercentage = 200 + await nextTick() + canvasStore.appScalePercentage = 250 + await nextTick() + + vi.advanceTimersByTime(150) + expect(mockLoad3d.handleResize).toHaveBeenCalledOnce() + + vi.useRealTimers() + }) }) describe('preserves existing node callbacks through initializeLoad3d', () => { diff --git a/src/composables/useLoad3d.ts b/src/composables/useLoad3d.ts index 619aa89f41..d61d61b3a4 100644 --- a/src/composables/useLoad3d.ts +++ b/src/composables/useLoad3d.ts @@ -1,6 +1,6 @@ import type { MaybeRef } from 'vue' -import { toRef } from '@vueuse/core' +import { toRef, useDebounceFn } from '@vueuse/core' import { getActivePinia } from 'pinia' import { ref, toRaw, watch } from 'vue' @@ -31,6 +31,7 @@ import type { LGraphNode } from '@/lib/litegraph/src/LGraphNode' import { LiteGraph } from '@/lib/litegraph/src/litegraph' import { useSettingStore } from '@/platform/settings/settingStore' import { useToastStore } from '@/platform/updates/common/toastStore' +import { useCanvasStore } from '@/renderer/core/canvas/canvasStore' import { api } from '@/scripts/api' import { app } from '@/scripts/app' import { useLoad3dService } from '@/services/load3dService' @@ -44,6 +45,15 @@ export const useLoad3d = (nodeOrRef: MaybeRef) => { let load3d: Load3d | null = null let isFirstModelLoad = true + const debouncedHandleResize = useDebounceFn(() => { + load3d?.handleResize() + }, 150) + + watch( + () => (getActivePinia() ? useCanvasStore().appScalePercentage : 0), + debouncedHandleResize + ) + const sceneConfig = ref({ showGrid: true, backgroundColor: '#000000', @@ -132,6 +142,7 @@ export const useLoad3d = (nodeOrRef: MaybeRef) => { height: heightWidget.value as number }) : undefined, + getZoomScale: () => app.canvas?.ds?.scale ?? 1, onContextMenu: (event) => { const menuOptions = app.canvas.getNodeMenuOptions(node) new LiteGraph.ContextMenu(menuOptions, { diff --git a/src/extensions/core/load3d/Load3d.test.ts b/src/extensions/core/load3d/Load3d.test.ts index 7cf76122e0..af1d9d2df4 100644 --- a/src/extensions/core/load3d/Load3d.test.ts +++ b/src/extensions/core/load3d/Load3d.test.ts @@ -280,7 +280,7 @@ describe('Load3d', () => { const sceneResize = vi.fn() Object.assign(ctx.load3d, { - renderer: { domElement: canvas, setSize }, + renderer: { domElement: canvas, setSize, setPixelRatio: vi.fn() }, targetWidth: 400, targetHeight: 200, targetAspectRatio: 2, @@ -383,6 +383,70 @@ describe('Load3d', () => { expect(args[2]).toBe(800) expect(args[3]).toBe(400) }) + + it('handleResize calls setPixelRatio with the value returned by getZoomScaleCallback', () => { + delete (ctx.load3d as { handleResize?: unknown }).handleResize + + const parent = document.createElement('div') + Object.defineProperty(parent, 'clientWidth', { + value: 400, + configurable: true + }) + Object.defineProperty(parent, 'clientHeight', { + value: 400, + configurable: true + }) + const canvas = document.createElement('canvas') + parent.appendChild(canvas) + + const setPixelRatio = vi.fn() + + Object.assign(ctx.load3d, { + renderer: { domElement: canvas, setSize: vi.fn(), setPixelRatio }, + getZoomScaleCallback: () => 2.5, + targetWidth: 0, + targetHeight: 0, + isViewerMode: false, + cameraManager: { ...ctx.cameraManager, handleResize: vi.fn() }, + sceneManager: { ...ctx.sceneManager, handleResize: vi.fn() } + }) + + ctx.load3d.handleResize() + + expect(setPixelRatio).toHaveBeenCalledWith(2.5) + }) + + it('handleResize defaults to pixelRatio 1 when no getZoomScaleCallback is provided', () => { + delete (ctx.load3d as { handleResize?: unknown }).handleResize + + const parent = document.createElement('div') + Object.defineProperty(parent, 'clientWidth', { + value: 400, + configurable: true + }) + Object.defineProperty(parent, 'clientHeight', { + value: 400, + configurable: true + }) + const canvas = document.createElement('canvas') + parent.appendChild(canvas) + + const setPixelRatio = vi.fn() + + Object.assign(ctx.load3d, { + renderer: { domElement: canvas, setSize: vi.fn(), setPixelRatio }, + getZoomScaleCallback: undefined, + targetWidth: 0, + targetHeight: 0, + isViewerMode: false, + cameraManager: { ...ctx.cameraManager, handleResize: vi.fn() }, + sceneManager: { ...ctx.sceneManager, handleResize: vi.fn() } + }) + + ctx.load3d.handleResize() + + expect(setPixelRatio).toHaveBeenCalledWith(1) + }) }) describe('render loop wiring', () => { diff --git a/src/extensions/core/load3d/Load3d.ts b/src/extensions/core/load3d/Load3d.ts index 28579297ed..dd6cc2c039 100644 --- a/src/extensions/core/load3d/Load3d.ts +++ b/src/extensions/core/load3d/Load3d.ts @@ -102,6 +102,7 @@ class Load3d { private disposeContextMenuGuard: (() => void) | null = null private resizeObserver: ResizeObserver | null = null + private getZoomScaleCallback: (() => number) | undefined constructor( container: Element | HTMLElement, @@ -112,6 +113,7 @@ class Load3d { this.isViewerMode = options.isViewerMode || false this.onContextMenuCallback = options.onContextMenu this.getDimensionsCallback = options.getDimensions + this.getZoomScaleCallback = options.getZoomScale if (options.width && options.height) { this.targetWidth = options.width @@ -645,6 +647,11 @@ class Load3d { const containerWidth = parentElement.clientWidth const containerHeight = parentElement.clientHeight + // Scale pixel density to match the graph zoom level so the 3D scene + // renders at the correct resolution when the canvas is zoomed in or out. + const zoomScale = this.getZoomScaleCallback?.() ?? 1 + this.renderer.setPixelRatio(Math.min(zoomScale, 3)) + if (this.getDimensionsCallback) { const dims = this.getDimensionsCallback() if (dims) { diff --git a/src/extensions/core/load3d/SceneManager.test.ts b/src/extensions/core/load3d/SceneManager.test.ts index c087f27681..6a70c49025 100644 --- a/src/extensions/core/load3d/SceneManager.test.ts +++ b/src/extensions/core/load3d/SceneManager.test.ts @@ -25,6 +25,37 @@ vi.mock('three', async (importOriginal) => { return { ...actual, TextureLoader: StubTextureLoader } }) +vi.mock('three/examples/jsm/controls/OrbitControls', () => { + class OrbitControls {} + return { OrbitControls } +}) + +function makeMockRenderer(pixelRatio = 1): THREE.WebGLRenderer { + const domElement = { + toDataURL: vi.fn().mockReturnValue('data:image/png;base64,abc'), + clientWidth: 400, + clientHeight: 300 + } + return { + domElement, + outputColorSpace: THREE.SRGBColorSpace, + toneMapping: THREE.ACESFilmicToneMapping, + toneMappingExposure: 1, + getSize: vi.fn((v: THREE.Vector2) => { + v.set(400, 300) + return v + }), + getPixelRatio: vi.fn().mockReturnValue(pixelRatio), + getClearColor: vi.fn((c: THREE.Color) => c), + getClearAlpha: vi.fn().mockReturnValue(0), + setPixelRatio: vi.fn(), + setSize: vi.fn(), + setClearColor: vi.fn(), + clear: vi.fn(), + render: vi.fn() + } as unknown as THREE.WebGLRenderer +} + function makeMockEventManager() { return { addEventListener: vi.fn(), @@ -50,6 +81,12 @@ function makeRenderer() { domElement: canvas, setClearColor: vi.fn(), setSize: vi.fn(), + getSize: vi.fn((v: THREE.Vector2) => { + v.set(800, 600) + return v + }), + getPixelRatio: vi.fn().mockReturnValue(1), + setPixelRatio: vi.fn(), render: vi.fn(), clear: vi.fn(), getClearColor: vi.fn().mockReturnValue(new THREE.Color(0xffffff)), @@ -544,3 +581,90 @@ describe('SceneManager', () => { }) }) }) + +function makeSceneManager( + pixelRatio = 1, + cameraOverride?: THREE.PerspectiveCamera | THREE.OrthographicCamera +) { + const renderer = makeMockRenderer(pixelRatio) + const camera = cameraOverride ?? new THREE.PerspectiveCamera() + const eventManager = makeMockEventManager() + const manager = new SceneManager( + renderer, + () => camera, + vi.fn() as unknown as () => InstanceType< + typeof import('three/examples/jsm/controls/OrbitControls').OrbitControls + >, + eventManager + ) + return { manager, renderer, camera, eventManager } +} + +describe('SceneManager.captureScene', () => { + beforeEach(() => { + vi.clearAllMocks() + }) + + it('resolves with scene, mask, and normal data URLs', async () => { + const { manager } = makeSceneManager() + const result = await manager.captureScene(800, 600) + expect(result.scene).toContain('data:image/png') + expect(result.mask).toContain('data:image/png') + expect(result.normal).toContain('data:image/png') + }) + + it('forces pixel ratio to 1 before rendering regardless of original value', async () => { + const { manager, renderer } = makeSceneManager(2) + await manager.captureScene(800, 600) + expect(vi.mocked(renderer.setPixelRatio).mock.calls[0]).toEqual([1]) + }) + + it('restores original pixel ratio after capture completes', async () => { + const originalPixelRatio = 3 + const { manager, renderer } = makeSceneManager(originalPixelRatio) + await manager.captureScene(800, 600) + const calls = vi.mocked(renderer.setPixelRatio).mock.calls + expect(calls.at(-1)).toEqual([originalPixelRatio]) + }) + + it('renders at requested capture dimensions', async () => { + const { manager, renderer } = makeSceneManager() + await manager.captureScene(1920, 1080) + expect(vi.mocked(renderer.setSize).mock.calls[0]).toEqual([1920, 1080]) + }) + + it('restores original renderer size after capture', async () => { + const { manager, renderer } = makeSceneManager() + await manager.captureScene(1920, 1080) + const calls = vi.mocked(renderer.setSize).mock.calls + expect(calls.at(-1)).toEqual([400, 300]) + }) + + it('restores perspective camera aspect after capture', async () => { + const camera = new THREE.PerspectiveCamera(75, 1, 0.1, 1000) + const { manager } = makeSceneManager(1, camera) + const originalAspect = camera.aspect + await manager.captureScene(800, 600) + expect(camera.aspect).toBe(originalAspect) + }) + + it('restores orthographic camera bounds after capture', async () => { + const camera = new THREE.OrthographicCamera(-5, 5, 5, -5, 0.1, 1000) + const { manager } = makeSceneManager(1, camera) + await manager.captureScene(800, 600) + expect(camera.left).toBe(-5) + expect(camera.right).toBe(5) + expect(camera.top).toBe(5) + expect(camera.bottom).toBe(-5) + }) + + it('disposes each temporary MeshNormalMaterial after the normal pass', async () => { + const { manager } = makeSceneManager() + manager.scene.add( + new THREE.Mesh(new THREE.BoxGeometry(), new THREE.MeshBasicMaterial()) + ) + const disposeSpy = vi.spyOn(THREE.MeshNormalMaterial.prototype, 'dispose') + await manager.captureScene(800, 600) + expect(disposeSpy).toHaveBeenCalledOnce() + }) +}) diff --git a/src/extensions/core/load3d/SceneManager.ts b/src/extensions/core/load3d/SceneManager.ts index a9600b1f86..86c30200d2 100644 --- a/src/extensions/core/load3d/SceneManager.ts +++ b/src/extensions/core/load3d/SceneManager.ts @@ -332,120 +332,138 @@ export class SceneManager implements SceneManagerInterface { } } - captureScene( + async captureScene( width: number, height: number ): Promise<{ scene: string; mask: string; normal: string }> { - return new Promise(async (resolve, reject) => { - try { - const originalWidth = this.renderer.domElement.width - const originalHeight = this.renderer.domElement.height - const originalClearColor = this.renderer.getClearColor( - new THREE.Color() - ) - const originalClearAlpha = this.renderer.getClearAlpha() - const originalOutputColorSpace = this.renderer.outputColorSpace + const originalSize = new THREE.Vector2() + this.renderer.getSize(originalSize) + const originalPixelRatio = this.renderer.getPixelRatio() + const originalClearColor = this.renderer.getClearColor(new THREE.Color()) + const originalClearAlpha = this.renderer.getClearAlpha() + const originalOutputColorSpace = this.renderer.outputColorSpace - this.renderer.setSize(width, height) - - if (this.getActiveCamera() instanceof THREE.PerspectiveCamera) { - const perspectiveCamera = - this.getActiveCamera() as THREE.PerspectiveCamera - - perspectiveCamera.aspect = width / height - perspectiveCamera.updateProjectionMatrix() - } else { - const orthographicCamera = - this.getActiveCamera() as THREE.OrthographicCamera - - const frustumSize = 10 - const aspect = width / height - - orthographicCamera.left = (-frustumSize * aspect) / 2 - orthographicCamera.right = (frustumSize * aspect) / 2 - orthographicCamera.top = frustumSize / 2 - orthographicCamera.bottom = -frustumSize / 2 - - orthographicCamera.updateProjectionMatrix() - } - - if ( - this.backgroundTexture && - this.backgroundMesh && - this.currentBackgroundType === 'image' - ) { - this.updateBackgroundSize( - this.backgroundTexture, - this.backgroundMesh, - width, - height - ) - } - - const originalMaterials = new Map< - THREE.Mesh, - THREE.Material | THREE.Material[] - >() - - this.renderer.clear() - this.renderBackground() - this.renderer.render(this.scene, this.getActiveCamera()) - const sceneData = this.renderer.domElement.toDataURL('image/png') - - this.renderer.setClearColor(0x000000, 0) - this.renderer.clear() - this.renderer.render(this.scene, this.getActiveCamera()) - const maskData = this.renderer.domElement.toDataURL('image/png') - - this.scene.traverse((child) => { - if (child instanceof THREE.Mesh) { - originalMaterials.set(child, child.material) - - child.material = new THREE.MeshNormalMaterial({ - flatShading: false, - side: THREE.DoubleSide, - normalScale: new THREE.Vector2(1, 1) - }) + const activeCamera = this.getActiveCamera() + const savedCameraParams = + activeCamera instanceof THREE.PerspectiveCamera + ? { type: 'perspective' as const, aspect: activeCamera.aspect } + : { + type: 'orthographic' as const, + left: (activeCamera as THREE.OrthographicCamera).left, + right: (activeCamera as THREE.OrthographicCamera).right, + top: (activeCamera as THREE.OrthographicCamera).top, + bottom: (activeCamera as THREE.OrthographicCamera).bottom } - }) - const gridVisible = this.gridHelper.visible - this.gridHelper.visible = false + const originalMaterials = new Map< + THREE.Mesh, + THREE.Material | THREE.Material[] + >() + const tempMaterials: THREE.MeshNormalMaterial[] = [] + const gridVisible = this.gridHelper.visible - this.renderer.setClearColor(0x000000, 1) - this.renderer.clear() - this.renderer.render(this.scene, this.getActiveCamera()) - const normalData = this.renderer.domElement.toDataURL('image/png') + try { + // Capture at exactly the requested pixel dimensions, independent of + // the current zoom-driven pixel ratio. + this.renderer.setPixelRatio(1) + this.renderer.setSize(width, height) - this.scene.traverse((child) => { - if (child instanceof THREE.Mesh) { - const originalMaterial = originalMaterials.get(child) - if (originalMaterial) { - child.material = originalMaterial - } - } - }) + if (activeCamera instanceof THREE.PerspectiveCamera) { + activeCamera.aspect = width / height + activeCamera.updateProjectionMatrix() + } else { + const orthographicCamera = activeCamera as THREE.OrthographicCamera - this.renderer.setClearColor(0xffffff, 1) - this.renderer.clear() + const frustumSize = 10 + const aspect = width / height - this.gridHelper.visible = gridVisible + orthographicCamera.left = (-frustumSize * aspect) / 2 + orthographicCamera.right = (frustumSize * aspect) / 2 + orthographicCamera.top = frustumSize / 2 + orthographicCamera.bottom = -frustumSize / 2 - this.renderer.setClearColor(originalClearColor, originalClearAlpha) - this.renderer.setSize(originalWidth, originalHeight) - this.renderer.outputColorSpace = originalOutputColorSpace - - this.handleResize(originalWidth, originalHeight) - - resolve({ - scene: sceneData, - mask: maskData, - normal: normalData - }) - } catch (error) { - reject(error) + orthographicCamera.updateProjectionMatrix() } - }) + + if ( + this.backgroundTexture && + this.backgroundMesh && + this.currentBackgroundType === 'image' + ) { + this.updateBackgroundSize( + this.backgroundTexture, + this.backgroundMesh, + width, + height + ) + } + + this.renderer.clear() + this.renderBackground() + this.renderer.render(this.scene, activeCamera) + const sceneData = this.renderer.domElement.toDataURL('image/png') + + this.renderer.setClearColor(0x000000, 0) + this.renderer.clear() + this.renderer.render(this.scene, activeCamera) + const maskData = this.renderer.domElement.toDataURL('image/png') + + this.scene.traverse((child) => { + if (child instanceof THREE.Mesh) { + originalMaterials.set(child, child.material) + + const tempMaterial = new THREE.MeshNormalMaterial({ + flatShading: false, + side: THREE.DoubleSide, + normalScale: new THREE.Vector2(1, 1) + }) + tempMaterials.push(tempMaterial) + child.material = tempMaterial + } + }) + + this.gridHelper.visible = false + + this.renderer.setClearColor(0x000000, 1) + this.renderer.clear() + this.renderer.render(this.scene, activeCamera) + const normalData = this.renderer.domElement.toDataURL('image/png') + + this.renderer.setClearColor(0xffffff, 1) + this.renderer.clear() + + return { scene: sceneData, mask: maskData, normal: normalData } + } finally { + this.scene.traverse((child) => { + if (child instanceof THREE.Mesh) { + const originalMaterial = originalMaterials.get(child) + if (originalMaterial) { + child.material = originalMaterial + } + } + }) + for (const mat of tempMaterials) { + mat.dispose() + } + this.gridHelper.visible = gridVisible + if (savedCameraParams.type === 'perspective') { + const persp = activeCamera as THREE.PerspectiveCamera + persp.aspect = savedCameraParams.aspect + persp.updateProjectionMatrix() + } else { + const ortho = activeCamera as THREE.OrthographicCamera + ortho.left = savedCameraParams.left + ortho.right = savedCameraParams.right + ortho.top = savedCameraParams.top + ortho.bottom = savedCameraParams.bottom + ortho.updateProjectionMatrix() + } + this.renderer.setClearColor(originalClearColor, originalClearAlpha) + this.renderer.setPixelRatio(originalPixelRatio) + this.renderer.setSize(originalSize.x, originalSize.y) + this.renderer.outputColorSpace = originalOutputColorSpace + this.handleResize(originalSize.x, originalSize.y) + } } reset(): void {} diff --git a/src/extensions/core/load3d/interfaces.ts b/src/extensions/core/load3d/interfaces.ts index a5dc7c3b08..679e4fd7ee 100644 --- a/src/extensions/core/load3d/interfaces.ts +++ b/src/extensions/core/load3d/interfaces.ts @@ -77,6 +77,11 @@ export interface Load3DOptions { // Use this for reactive dimensions that change over time getDimensions?: () => { width: number; height: number } | null + // Returns the current canvas zoom scale (e.g. ds.scale from LiteGraph). + // Used to scale the renderer pixel ratio so the 3D scene renders at the + // correct resolution when the graph is zoomed in or out. + getZoomScale?: () => number + // Viewer mode flag (affects aspect ratio behavior) isViewerMode?: boolean From 2c8ecd82ec4b01683a899fce1980535352322330 Mon Sep 17 00:00:00 2001 From: Christian Byrne Date: Mon, 4 May 2026 17:28:09 -0700 Subject: [PATCH 007/172] fix(load3d): snapshot original materials before reapplying materialMode (#11825) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary Fixes a bug where models reloaded in wireframe/normal/depth modes would not restore to their original materials correctly, because the material snapshot was being taken *after* the mode was applied. ## Changes - Move `setupModelMaterials(model)` to immediately after `scene.add(model)` and before `setMaterialMode()` / `setUpDirection()` in `SceneModelManager.setupModel()` - Save `materialMode` into `pendingMaterialMode` before calling `setupModelMaterials()`, since the latter internally calls `setMaterialMode('original')` which resets `this.materialMode` — preserving the value ensures the subsequent reapplication guard works correctly - Update stale test assertion that reflected the old (incorrect) call order - Add regression test: verifies that `originalMaterials` captures the pre-mutation material and that restoring to `'original'` after a non-original load gives back the true original mesh material ## Testing ### Automated - `src/extensions/core/load3d/SceneModelManager.test.ts` — 44 tests, all pass - Full load3d test suite — 401 tests, all pass ### E2E Verification Steps 1. Open ComfyUI with a Load3D node 2. Load any GLB/OBJ model 3. Switch Material Mode to **Wireframe** (or Normal/Depth) 4. Load a different model (or reload the same one) 5. Switch Material Mode back to **Original** 6. Verify the model renders with its original diffuse/PBR materials (not wireframe) ## Review Focus The key invariant: `setupModelMaterials()` must snapshot mesh materials in their *unmodified* state. It must run before any `setMaterialMode()` call that mutates them. The `pendingMaterialMode` variable is needed because `setupModelMaterials()` internally calls `setMaterialMode('original')`, which updates `this.materialMode`, making the subsequent guard `if (this.materialMode !== 'original')` silently skip reapplication without it. Fixes #11344 ┆Issue is synchronized with this [Notion page](https://app.notion.com/p/PR-11825-fix-load3d-snapshot-original-materials-before-reapplying-materialMode-3546d73d3650818b9c35fa60c15f17a3) by [Unito](https://www.unito.io) --- .../core/load3d/SceneModelManager.test.ts | 32 +++++++++++++++---- .../core/load3d/SceneModelManager.ts | 7 ++-- 2 files changed, 30 insertions(+), 9 deletions(-) diff --git a/src/extensions/core/load3d/SceneModelManager.test.ts b/src/extensions/core/load3d/SceneModelManager.test.ts index cc9e122980..b9800378d1 100644 --- a/src/extensions/core/load3d/SceneModelManager.test.ts +++ b/src/extensions/core/load3d/SceneModelManager.test.ts @@ -211,20 +211,40 @@ describe('SceneModelManager', () => { expect(setupCamera).toHaveBeenCalled() }) - it('does not skip materialMode when it differs from original', async () => { + it('reapplies non-original materialMode after snapshotting', async () => { const { manager } = createManager() const model = createMeshModel() - // setupModel checks materialMode !== 'original' and calls - // setMaterialMode, but the guard `mode === this.materialMode` - // causes it to no-op. Then setupModelMaterials resets to 'original'. + // setupModel calls setupModelMaterials first (which internally calls + // setMaterialMode('original') to reset), then reapplies the stored mode. manager.materialMode = 'wireframe' const spy = vi.spyOn(manager, 'setMaterialMode') await manager.setupModel(model) - // setMaterialMode is called with the stored mode and then 'original' - expect(spy).toHaveBeenCalledWith('wireframe') expect(spy).toHaveBeenCalledWith('original') + expect(spy).toHaveBeenCalledWith('wireframe') + // The final material mode visible on the mesh should be wireframe. + const mesh = model.children[0] as THREE.Mesh + expect((mesh.material as THREE.MeshBasicMaterial).wireframe).toBe(true) + }) + + it('snapshots original materials before applying materialMode so restore is correct', async () => { + const { manager } = createManager() + const model = createMeshModel() + const mesh = model.children[0] as THREE.Mesh + const originalMat = mesh.material + + // Set a non-original mode before loading — this was the bug: + // originalMaterials would capture the wireframe material instead of the real one. + manager.materialMode = 'wireframe' + await manager.setupModel(model) + + // The snapshot must hold the *pre-mutation* material. + expect(manager.originalMaterials.get(mesh)).toBe(originalMat) + + // Restoring to 'original' must give back the true original, not wireframe. + manager.setMaterialMode('original') + expect(mesh.material).toBe(originalMat) }) it('applies current up direction if not original', async () => { diff --git a/src/extensions/core/load3d/SceneModelManager.ts b/src/extensions/core/load3d/SceneModelManager.ts index 5743a4e609..9ed75fed5a 100644 --- a/src/extensions/core/load3d/SceneModelManager.ts +++ b/src/extensions/core/load3d/SceneModelManager.ts @@ -447,15 +447,16 @@ export class SceneModelManager implements ModelManagerInterface { } this.scene.add(model) + const pendingMaterialMode = this.materialMode + this.setupModelMaterials(model) - if (this.materialMode !== 'original') { - this.setMaterialMode(this.materialMode) + if (pendingMaterialMode !== 'original') { + this.setMaterialMode(pendingMaterialMode) } if (this.currentUpDirection !== 'original') { this.setUpDirection(this.currentUpDirection) } - this.setupModelMaterials(model) const box = this.computeWorldBounds(model) const size = box.getSize(new THREE.Vector3()) From 551cf21fb117be022e25546a801b262a67bd4cf1 Mon Sep 17 00:00:00 2001 From: Christian Byrne Date: Mon, 4 May 2026 17:29:24 -0700 Subject: [PATCH 008/172] fix(load3d): reapply up-direction after fitToViewer() transform reset (#11826) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary `fitToViewer()` in `SceneModelManager` resets the model rotation to `(0,0,0)` as part of its normalize-and-scale flow, but does not reapply `currentUpDirection` afterward. This causes a state/view mismatch when the user has previously selected a non-default up-axis (e.g. `+x`, `-z`): the model snaps back to its raw orientation while the Up Direction control still shows the previously selected value. ## Changes - In `fitToViewer()`, clear `originalRotation` (to avoid compounding with the stale pre-fit base) then reapply `currentUpDirection` if it is not `'original'` - Add 5 unit tests covering: no-op when no model, reapplication of direction, no rotation compounding on repeated calls, zero rotation for `'original'` direction, and stale `originalRotation` guard ## Testing ### Automated - `src/extensions/core/load3d/SceneModelManager.test.ts` — 5 new tests in `describe('fitToViewer')` - All 48 unit tests pass ### E2E Verification Steps 1. Open the Load3D viewer with any 3D model 2. Change **Up Direction** to any non-default value (e.g. `+x` or `-z`) — observe model rotates correctly 3. Click **Fit to Viewer** — model should remain in the chosen up-direction orientation, not snap back to raw orientation 4. Click **Fit to Viewer** again — rotation should remain stable (no compounding) 5. Change Up Direction back to `original` then click **Fit to Viewer** — model should return to neutral orientation `(0,0,0)` ## Review Focus The key invariant: `fitToViewer()` resets `rotation.set(0,0,0)` explicitly, so we must clear `originalRotation = null` before calling `setUpDirection`. Otherwise `setUpDirection` restores the stale pre-fit rotation as a base and then adds the direction offset on top, compounding incorrectly. Fixes #11347 ┆Issue is synchronized with this [Notion page](https://app.notion.com/p/PR-11826-fix-load3d-reapply-up-direction-after-fitToViewer-transform-reset-3546d73d36508166b9bcecc9949c4952) by [Unito](https://www.unito.io) --- .../core/load3d/SceneModelManager.test.ts | 74 +++++++++++++++++++ .../core/load3d/SceneModelManager.ts | 7 ++ 2 files changed, 81 insertions(+) diff --git a/src/extensions/core/load3d/SceneModelManager.test.ts b/src/extensions/core/load3d/SceneModelManager.test.ts index b9800378d1..8db052d23f 100644 --- a/src/extensions/core/load3d/SceneModelManager.test.ts +++ b/src/extensions/core/load3d/SceneModelManager.test.ts @@ -699,6 +699,80 @@ describe('SceneModelManager', () => { }) }) + describe('fitToViewer', () => { + it('does nothing when no current model', () => { + const { manager, setupCamera, setupGizmo } = createManager() + + manager.fitToViewer() + + expect(setupCamera).not.toHaveBeenCalled() + expect(setupGizmo).not.toHaveBeenCalled() + }) + + it('reapplies currentUpDirection after fitting', async () => { + const { manager, eventManager } = createManager() + const model = createMeshModel() + await manager.setupModel(model) + + manager.setUpDirection('+z') + vi.mocked(eventManager.emitEvent).mockClear() + + manager.fitToViewer() + + // rotation.x should reflect +z direction (-PI/2) applied to the post-fit base (0,0,0) + expect(model.rotation.x).toBeCloseTo(-Math.PI / 2) + expect(eventManager.emitEvent).toHaveBeenCalledWith( + 'upDirectionChange', + '+z' + ) + }) + + it('does not compound rotations when fitToViewer is called multiple times', async () => { + const { manager } = createManager() + const model = createMeshModel() + await manager.setupModel(model) + + manager.setUpDirection('-x') + + manager.fitToViewer() + const rotationAfterFirst = model.rotation.z + + manager.fitToViewer() + expect(model.rotation.z).toBeCloseTo(rotationAfterFirst) + }) + + it('leaves rotation at zero when currentUpDirection is original', async () => { + const { manager } = createManager() + const model = createMeshModel() + await manager.setupModel(model) + + manager.fitToViewer() + + expect(model.rotation.x).toBeCloseTo(0) + expect(model.rotation.y).toBeCloseTo(0) + expect(model.rotation.z).toBeCloseTo(0) + }) + + it('does not compound rotation when fitToViewer is called after manual rotation override', async () => { + const { manager } = createManager() + const model = createMeshModel() + await manager.setupModel(model) + + // Set an up direction, then manually override originalRotation to simulate + // a prior state where the base rotation was non-zero before fit + manager.setUpDirection('+x') + // Simulate that originalRotation was captured at a non-zero rotation + manager.originalRotation = new THREE.Euler(0.5, 0.3, 0.1) + + manager.fitToViewer() + + // After fit, the rotation should be correct for +x direction applied to (0,0,0) base + // Not compounded with the stale originalRotation + expect(model.rotation.x).toBeCloseTo(0) + expect(model.rotation.z).toBeCloseTo(-Math.PI / 2) + }) + }) + describe('PLY mode switching', () => { function createPLYManager() { const ctx = createManager({ diff --git a/src/extensions/core/load3d/SceneModelManager.ts b/src/extensions/core/load3d/SceneModelManager.ts index 9ed75fed5a..766fd0b243 100644 --- a/src/extensions/core/load3d/SceneModelManager.ts +++ b/src/extensions/core/load3d/SceneModelManager.ts @@ -492,6 +492,13 @@ export class SceneModelManager implements ModelManagerInterface { model.position.set(-center.x, -scaledBox.min.y, -center.z) + // fitToViewer resets rotation to (0,0,0), so originalRotation must be cleared + // before reapplying currentUpDirection to avoid compounding rotations. + this.originalRotation = null + if (this.currentUpDirection !== 'original') { + this.setUpDirection(this.currentUpDirection) + } + const newBox = this.computeWorldBounds(model) const newSize = newBox.getSize(new THREE.Vector3()) const newCenter = newBox.getCenter(new THREE.Vector3()) From 90b3d8a5c63c71d5e6dfb31d0dd8e7e7512fd5bc Mon Sep 17 00:00:00 2001 From: Alexander Brown Date: Mon, 4 May 2026 17:31:15 -0700 Subject: [PATCH 009/172] test: add mask editor brush adjustment and layer management browser tests (#11368) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit *PR Created by the Glary-Bot Agent* --- ## Summary Adds `browser_tests/tests/maskEditorBrushLayers.spec.ts` covering untested brush settings interaction and tool/layer management in the mask editor. ### Coverage gaps filled - `useBrushDrawing.ts` — brush thickness/opacity/hardness slider interaction - `useToolManager.ts` — tool switching with independent mask data, data preservation across tool switches ### Test cases (5 tests, 2 groups) | Group | Tests | Behavior | |---|---|---| | Brush settings | 3 | Thickness slider changes size, opacity slider changes opacity, hardness slider changes hardness | | Layer management | 2 | Different tools produce independent mask data, switching tools preserves previous mask data | ### References - Reuses patterns from existing `maskEditor.spec.ts` (`loadImageOnNode`, `openMaskEditorDialog`, `drawStrokeOnPointerZone`, `getMaskCanvasPixelData`) - Follows `browser_tests/AGENTS.md` directory structure - Follows `browser_tests/FLAKE_PREVENTION_RULES.md` assertion patterns ### Verification - TypeScript: clean - ESLint: clean - oxlint: clean - oxfmt: formatted ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-11368-test-add-mask-editor-brush-adjustment-and-layer-management-browser-tests-3466d73d36508170ae24ebea2b73d60d) by [Unito](https://www.unito.io) --------- Co-authored-by: Glary-Bot Co-authored-by: Amp --- .../fixtures/helpers/MaskEditorHelper.ts | 136 ++++++++++++ browser_tests/tests/maskEditor.spec.ts | 210 +++++------------- .../tests/maskEditorBrushLayers.spec.ts | 100 +++++++++ .../maskeditor/BrushSettingsPanel.vue | 1 + 4 files changed, 292 insertions(+), 155 deletions(-) create mode 100644 browser_tests/fixtures/helpers/MaskEditorHelper.ts create mode 100644 browser_tests/tests/maskEditorBrushLayers.spec.ts diff --git a/browser_tests/fixtures/helpers/MaskEditorHelper.ts b/browser_tests/fixtures/helpers/MaskEditorHelper.ts new file mode 100644 index 0000000000..9d9a6a54df --- /dev/null +++ b/browser_tests/fixtures/helpers/MaskEditorHelper.ts @@ -0,0 +1,136 @@ +import type { Locator } from '@playwright/test' +import { expect } from '@playwright/test' + +import { comfyPageFixture } from '@e2e/fixtures/ComfyPage' +import type { ComfyPage } from '@e2e/fixtures/ComfyPage' + +const MASK_CANVAS_INDEX = 2 +const RGB_CANVAS_INDEX = 1 + +export type BrushSliderLabel = 'thickness' + +export class MaskEditorHelper { + constructor(private comfyPage: ComfyPage) {} + + private get page() { + return this.comfyPage.page + } + + async loadImageOnNode() { + await this.comfyPage.workflow.loadWorkflow('widgets/load_image_widget') + + const loadImageNode = ( + await this.comfyPage.nodeOps.getNodeRefsByType('LoadImage') + )[0] + const { x, y } = await loadImageNode.getPosition() + + await this.comfyPage.dragDrop.dragAndDropFile('image64x64.webp', { + dropPosition: { x, y } + }) + + const imagePreview = this.page.locator('.image-preview') + await expect(imagePreview).toBeVisible() + await expect(imagePreview.locator('img')).toBeVisible() + await expect(imagePreview).toContainText('x') + + return { + imagePreview, + nodeId: String(loadImageNode.id) + } + } + + async openDialog(): Promise { + const { imagePreview } = await this.loadImageOnNode() + + await imagePreview.getByRole('region').hover() + await this.page.getByLabel('Edit or mask image').click() + + const dialog = this.page.locator('.mask-editor-dialog') + await expect(dialog).toBeVisible() + await expect( + dialog.getByRole('heading', { name: 'Mask Editor' }) + ).toBeVisible() + + const canvasContainer = dialog.locator('#maskEditorCanvasContainer') + await expect(canvasContainer).toBeVisible() + await expect(canvasContainer.locator('canvas')).toHaveCount(4) + + return dialog + } + + async drawStrokeOnPointerZone(dialog: Locator) { + const pointerZone = dialog.getByTestId('pointer-zone') + await expect(pointerZone).toBeVisible() + + const box = await pointerZone.boundingBox() + if (!box) throw new Error('Pointer zone bounding box not found') + + const startX = box.x + box.width * 0.3 + const startY = box.y + box.height * 0.5 + const endX = box.x + box.width * 0.7 + const endY = box.y + box.height * 0.5 + + await this.page.mouse.move(startX, startY) + await this.page.mouse.down() + await this.page.mouse.move(endX, endY, { steps: 10 }) + await this.page.mouse.up() + + return { startX, startY, endX, endY, box } + } + + async drawStrokeAndExpectPixels(dialog: Locator) { + await this.drawStrokeOnPointerZone(dialog) + await expect.poll(() => this.pollMaskPixelCount()).toBeGreaterThan(0) + } + + getCanvasPixelData(canvasIndex: number) { + return this.page.evaluate((idx) => { + const canvases = document.querySelectorAll( + '#maskEditorCanvasContainer canvas' + ) + const canvas = canvases[idx] as HTMLCanvasElement | undefined + if (!canvas) return null + const ctx = canvas.getContext('2d') + if (!ctx) return null + const data = ctx.getImageData(0, 0, canvas.width, canvas.height) + let nonTransparentPixels = 0 + for (let i = 3; i < data.data.length; i += 4) { + if (data.data[i] > 0) nonTransparentPixels++ + } + return { nonTransparentPixels, totalPixels: data.data.length / 4 } + }, canvasIndex) + } + + pollMaskPixelCount(): Promise { + return this.getCanvasPixelData(MASK_CANVAS_INDEX).then( + (d) => d?.nonTransparentPixels ?? 0 + ) + } + + pollRgbPixelCount(): Promise { + return this.getCanvasPixelData(RGB_CANVAS_INDEX).then( + (d) => d?.nonTransparentPixels ?? 0 + ) + } + + getCanvasSnapshot(canvasIndex: number): Promise { + return this.page.evaluate((idx) => { + const canvas = document.querySelectorAll( + '#maskEditorCanvasContainer canvas' + )[idx] as HTMLCanvasElement | undefined + return canvas?.toDataURL() ?? '' + }, canvasIndex) + } + + brushInput(dialog: Locator, label: BrushSliderLabel): Locator { + return dialog.getByTestId(`brush-${label}-input`) + } +} + +export const maskEditorTest = comfyPageFixture.extend<{ + maskEditor: MaskEditorHelper +}>({ + maskEditor: async ({ comfyPage }, use) => { + await use(new MaskEditorHelper(comfyPage)) + } +}) diff --git a/browser_tests/tests/maskEditor.spec.ts b/browser_tests/tests/maskEditor.spec.ts index d9bce93dc3..badf78e621 100644 --- a/browser_tests/tests/maskEditor.spec.ts +++ b/browser_tests/tests/maskEditor.spec.ts @@ -1,117 +1,13 @@ -import type { Page } from '@playwright/test' import { expect } from '@playwright/test' -import type { ComfyPage } from '@e2e/fixtures/ComfyPage' -import { comfyPageFixture as test } from '@e2e/fixtures/ComfyPage' +import { maskEditorTest as test } from '@e2e/fixtures/helpers/MaskEditorHelper' test.describe('Mask Editor', { tag: '@vue-nodes' }, () => { - async function loadImageOnNode(comfyPage: ComfyPage) { - await comfyPage.workflow.loadWorkflow('widgets/load_image_widget') - - const loadImageNode = ( - await comfyPage.nodeOps.getNodeRefsByType('LoadImage') - )[0] - const { x, y } = await loadImageNode.getPosition() - - await comfyPage.dragDrop.dragAndDropFile('image64x64.webp', { - dropPosition: { x, y } - }) - - const imagePreview = comfyPage.page.locator('.image-preview') - await expect(imagePreview).toBeVisible() - await expect(imagePreview.locator('img')).toBeVisible() - await expect(imagePreview).toContainText('x') - - return { - imagePreview, - nodeId: String(loadImageNode.id) - } - } - - async function openMaskEditorDialog(comfyPage: ComfyPage) { - const { imagePreview } = await loadImageOnNode(comfyPage) - - await imagePreview.getByRole('region').hover() - await comfyPage.page.getByLabel('Edit or mask image').click() - - const dialog = comfyPage.page.locator('.mask-editor-dialog') - await expect(dialog).toBeVisible() - await expect( - dialog.getByRole('heading', { name: 'Mask Editor' }) - ).toBeVisible() - - const canvasContainer = dialog.locator('#maskEditorCanvasContainer') - await expect(canvasContainer).toBeVisible() - await expect(canvasContainer.locator('canvas')).toHaveCount(4) - - return dialog - } - - async function getMaskCanvasPixelData(page: Page) { - return page.evaluate(() => { - const canvases = document.querySelectorAll( - '#maskEditorCanvasContainer canvas' - ) - // The mask canvas is the 3rd canvas (index 2, z-30) - const maskCanvas = canvases[2] as HTMLCanvasElement - if (!maskCanvas) return null - const ctx = maskCanvas.getContext('2d') - if (!ctx) return null - const data = ctx.getImageData(0, 0, maskCanvas.width, maskCanvas.height) - let nonTransparentPixels = 0 - for (let i = 3; i < data.data.length; i += 4) { - if (data.data[i] > 0) nonTransparentPixels++ - } - return { nonTransparentPixels, totalPixels: data.data.length / 4 } - }) - } - - function pollMaskPixelCount(page: Page): Promise { - return getMaskCanvasPixelData(page).then( - (d) => d?.nonTransparentPixels ?? 0 - ) - } - - async function drawStrokeOnPointerZone( - page: Page, - dialog: ReturnType - ) { - const pointerZone = dialog.locator( - '.maskEditor-ui-container [class*="w-[calc"]' - ) - await expect(pointerZone).toBeVisible() - - const box = await pointerZone.boundingBox() - if (!box) throw new Error('Pointer zone bounding box not found') - - const startX = box.x + box.width * 0.3 - const startY = box.y + box.height * 0.5 - const endX = box.x + box.width * 0.7 - const endY = box.y + box.height * 0.5 - - await page.mouse.move(startX, startY) - await page.mouse.down() - await page.mouse.move(endX, endY, { steps: 10 }) - await page.mouse.up() - - return { startX, startY, endX, endY, box } - } - - async function drawStrokeAndExpectPixels( - comfyPage: ComfyPage, - dialog: ReturnType - ) { - await drawStrokeOnPointerZone(comfyPage.page, dialog) - await expect - .poll(() => pollMaskPixelCount(comfyPage.page)) - .toBeGreaterThan(0) - } - test( 'opens mask editor from image preview button', { tag: ['@smoke', '@screenshot'] }, - async ({ comfyPage }) => { - const { imagePreview } = await loadImageOnNode(comfyPage) + async ({ comfyPage, maskEditor }) => { + const { imagePreview } = await maskEditor.loadImageOnNode() // Hover over the image panel to reveal action buttons await imagePreview.getByRole('region').hover() @@ -139,8 +35,8 @@ test.describe('Mask Editor', { tag: '@vue-nodes' }, () => { test( 'opens mask editor from context menu', { tag: ['@smoke', '@screenshot'] }, - async ({ comfyPage }) => { - const { nodeId } = await loadImageOnNode(comfyPage) + async ({ comfyPage, maskEditor }) => { + const { nodeId } = await maskEditor.loadImageOnNode() const nodeHeader = comfyPage.vueNodes .getNodeLocator(nodeId) @@ -166,63 +62,61 @@ test.describe('Mask Editor', { tag: '@vue-nodes' }, () => { } ) - test('draws a brush stroke on the mask canvas', async ({ comfyPage }) => { - const dialog = await openMaskEditorDialog(comfyPage) + test('draws a brush stroke on the mask canvas', async ({ maskEditor }) => { + const dialog = await maskEditor.openDialog() - const dataBefore = await getMaskCanvasPixelData(comfyPage.page) + const dataBefore = await maskEditor.getCanvasPixelData(2) expect(dataBefore).not.toBeNull() expect(dataBefore!.nonTransparentPixels).toBe(0) - await drawStrokeAndExpectPixels(comfyPage, dialog) + await maskEditor.drawStrokeAndExpectPixels(dialog) }) - test('undo reverts a brush stroke', async ({ comfyPage }) => { - const dialog = await openMaskEditorDialog(comfyPage) + test('undo reverts a brush stroke', async ({ maskEditor }) => { + const dialog = await maskEditor.openDialog() - await drawStrokeAndExpectPixels(comfyPage, dialog) + await maskEditor.drawStrokeAndExpectPixels(dialog) const undoButton = dialog.locator('button[title="Undo"]') await expect(undoButton).toBeVisible() await undoButton.click() - await expect.poll(() => pollMaskPixelCount(comfyPage.page)).toBe(0) + await expect.poll(() => maskEditor.pollMaskPixelCount()).toBe(0) }) - test('redo restores an undone stroke', async ({ comfyPage }) => { - const dialog = await openMaskEditorDialog(comfyPage) + test('redo restores an undone stroke', async ({ maskEditor }) => { + const dialog = await maskEditor.openDialog() - await drawStrokeAndExpectPixels(comfyPage, dialog) + await maskEditor.drawStrokeAndExpectPixels(dialog) const undoButton = dialog.locator('button[title="Undo"]') await undoButton.click() - await expect.poll(() => pollMaskPixelCount(comfyPage.page)).toBe(0) + await expect.poll(() => maskEditor.pollMaskPixelCount()).toBe(0) const redoButton = dialog.locator('button[title="Redo"]') await expect(redoButton).toBeVisible() await redoButton.click() - await expect - .poll(() => pollMaskPixelCount(comfyPage.page)) - .toBeGreaterThan(0) + await expect.poll(() => maskEditor.pollMaskPixelCount()).toBeGreaterThan(0) }) - test('clear button removes all mask content', async ({ comfyPage }) => { - const dialog = await openMaskEditorDialog(comfyPage) + test('clear button removes all mask content', async ({ maskEditor }) => { + const dialog = await maskEditor.openDialog() - await drawStrokeAndExpectPixels(comfyPage, dialog) + await maskEditor.drawStrokeAndExpectPixels(dialog) const clearButton = dialog.getByRole('button', { name: 'Clear' }) await expect(clearButton).toBeVisible() await clearButton.click() - await expect.poll(() => pollMaskPixelCount(comfyPage.page)).toBe(0) + await expect.poll(() => maskEditor.pollMaskPixelCount()).toBe(0) }) - test('cancel closes the dialog without saving', async ({ comfyPage }) => { - const dialog = await openMaskEditorDialog(comfyPage) + test('cancel closes the dialog without saving', async ({ maskEditor }) => { + const dialog = await maskEditor.openDialog() - await drawStrokeAndExpectPixels(comfyPage, dialog) + await maskEditor.drawStrokeAndExpectPixels(dialog) const cancelButton = dialog.getByRole('button', { name: 'Cancel' }) await cancelButton.click() @@ -230,10 +124,10 @@ test.describe('Mask Editor', { tag: '@vue-nodes' }, () => { await expect(dialog).toBeHidden() }) - test('invert button inverts the mask', async ({ comfyPage }) => { - const dialog = await openMaskEditorDialog(comfyPage) + test('invert button inverts the mask', async ({ maskEditor }) => { + const dialog = await maskEditor.openDialog() - const dataBefore = await getMaskCanvasPixelData(comfyPage.page) + const dataBefore = await maskEditor.getCanvasPixelData(2) expect(dataBefore).not.toBeNull() const pixelsBefore = dataBefore!.nonTransparentPixels @@ -242,26 +136,29 @@ test.describe('Mask Editor', { tag: '@vue-nodes' }, () => { await invertButton.click() await expect - .poll(() => pollMaskPixelCount(comfyPage.page)) + .poll(() => maskEditor.pollMaskPixelCount()) .toBeGreaterThan(pixelsBefore) }) - test('keyboard shortcut Ctrl+Z triggers undo', async ({ comfyPage }) => { - const dialog = await openMaskEditorDialog(comfyPage) + test('keyboard shortcut Ctrl+Z triggers undo', async ({ + comfyPage, + maskEditor + }) => { + const dialog = await maskEditor.openDialog() - await drawStrokeAndExpectPixels(comfyPage, dialog) + await maskEditor.drawStrokeAndExpectPixels(dialog) const modifier = process.platform === 'darwin' ? 'Meta+z' : 'Control+z' await comfyPage.page.keyboard.press(modifier) - await expect.poll(() => pollMaskPixelCount(comfyPage.page)).toBe(0) + await expect.poll(() => maskEditor.pollMaskPixelCount()).toBe(0) }) test( 'tool panel shows all five tools', { tag: ['@smoke'] }, - async ({ comfyPage }) => { - const dialog = await openMaskEditorDialog(comfyPage) + async ({ maskEditor }) => { + const dialog = await maskEditor.openDialog() const toolPanel = dialog.locator('.maskEditor-ui-container') await expect(toolPanel).toBeVisible() @@ -279,9 +176,9 @@ test.describe('Mask Editor', { tag: '@vue-nodes' }, () => { ) test('switching tools updates the selected indicator', async ({ - comfyPage + maskEditor }) => { - const dialog = await openMaskEditorDialog(comfyPage) + const dialog = await maskEditor.openDialog() const toolEntries = dialog.locator('.maskEditor_toolPanelContainer') await expect(toolEntries).toHaveCount(5) @@ -300,9 +197,9 @@ test.describe('Mask Editor', { tag: '@vue-nodes' }, () => { }) test('brush settings panel is visible with thickness controls', async ({ - comfyPage + maskEditor }) => { - const dialog = await openMaskEditorDialog(comfyPage) + const dialog = await maskEditor.openDialog() // The side panel should show brush settings by default const thicknessLabel = dialog.getByText('Thickness') @@ -315,8 +212,11 @@ test.describe('Mask Editor', { tag: '@vue-nodes' }, () => { await expect(hardnessLabel).toBeVisible() }) - test('save uploads all layers and closes dialog', async ({ comfyPage }) => { - const dialog = await openMaskEditorDialog(comfyPage) + test('save uploads all layers and closes dialog', async ({ + comfyPage, + maskEditor + }) => { + const dialog = await maskEditor.openDialog() let maskUploadCount = 0 let imageUploadCount = 0 @@ -359,8 +259,8 @@ test.describe('Mask Editor', { tag: '@vue-nodes' }, () => { ).toBeGreaterThan(0) }) - test('save failure keeps dialog open', async ({ comfyPage }) => { - const dialog = await openMaskEditorDialog(comfyPage) + test('save failure keeps dialog open', async ({ comfyPage, maskEditor }) => { + const dialog = await maskEditor.openDialog() // Fail all upload routes await comfyPage.page.route('**/upload/mask', (route) => @@ -380,23 +280,23 @@ test.describe('Mask Editor', { tag: '@vue-nodes' }, () => { test( 'eraser tool removes mask content', { tag: ['@screenshot'] }, - async ({ comfyPage }) => { - const dialog = await openMaskEditorDialog(comfyPage) + async ({ maskEditor }) => { + const dialog = await maskEditor.openDialog() // Draw a stroke with the mask pen (default tool) - await drawStrokeAndExpectPixels(comfyPage, dialog) + await maskEditor.drawStrokeAndExpectPixels(dialog) - const pixelsAfterDraw = await getMaskCanvasPixelData(comfyPage.page) + const pixelsAfterDraw = await maskEditor.getCanvasPixelData(2) // Switch to eraser tool (3rd tool, index 2) const toolEntries = dialog.locator('.maskEditor_toolPanelContainer') await toolEntries.nth(2).click() // Draw over the same area with the eraser - await drawStrokeOnPointerZone(comfyPage.page, dialog) + await maskEditor.drawStrokeOnPointerZone(dialog) await expect - .poll(() => pollMaskPixelCount(comfyPage.page)) + .poll(() => maskEditor.pollMaskPixelCount()) .toBeLessThan(pixelsAfterDraw!.nonTransparentPixels) } ) diff --git a/browser_tests/tests/maskEditorBrushLayers.spec.ts b/browser_tests/tests/maskEditorBrushLayers.spec.ts new file mode 100644 index 0000000000..78c7ad96d8 --- /dev/null +++ b/browser_tests/tests/maskEditorBrushLayers.spec.ts @@ -0,0 +1,100 @@ +import { expect } from '@playwright/test' + +import { maskEditorTest as test } from '@e2e/fixtures/helpers/MaskEditorHelper' + +const RGB_PAINT_TOOL_INDEX = 1 // RGB / color paint tool +const ERASER_TOOL_INDEX = 2 // Eraser tool + +test.describe( + 'Mask Editor brush adjustment and layer management', + { tag: '@vue-nodes' }, + () => { + test.describe('Brush settings interaction', () => { + test('Adjusting brush thickness slider changes stroke output', async ({ + comfyPage, + maskEditor + }) => { + const dialog = await maskEditor.openDialog() + const thicknessInput = maskEditor.brushInput(dialog, 'thickness') + + // Thin brush + await thicknessInput.fill('2') + await expect(thicknessInput).toHaveValue('2') + + await maskEditor.drawStrokeOnPointerZone(dialog) + await expect + .poll(() => maskEditor.pollMaskPixelCount()) + .toBeGreaterThan(0) + const thinPixels = await maskEditor.pollMaskPixelCount() + + await comfyPage.page.keyboard.press('Control+z') + await expect.poll(() => maskEditor.pollMaskPixelCount()).toBe(0) + + // Thick brush + await thicknessInput.fill('200') + await expect(thicknessInput).toHaveValue('200') + + await maskEditor.drawStrokeOnPointerZone(dialog) + await expect + .poll(() => maskEditor.pollMaskPixelCount()) + .toBeGreaterThan(thinPixels) + }) + }) + + test.describe('Layer management', () => { + test('Drawing on different tools produces independent mask data', async ({ + maskEditor + }) => { + const dialog = await maskEditor.openDialog() + + await maskEditor.drawStrokeOnPointerZone(dialog) + await expect + .poll(() => maskEditor.pollMaskPixelCount()) + .toBeGreaterThan(0) + const maskSnapshotAfterPen = await maskEditor.getCanvasSnapshot(2) + + const toolEntries = dialog.locator('.maskEditor_toolPanelContainer') + await expect(toolEntries).toHaveCount(5) + await toolEntries.nth(RGB_PAINT_TOOL_INDEX).click() + await expect(toolEntries.nth(RGB_PAINT_TOOL_INDEX)).toHaveClass( + /Selected/ + ) + + await maskEditor.drawStrokeOnPointerZone(dialog) + await expect + .poll(() => maskEditor.pollRgbPixelCount()) + .toBeGreaterThan(0) + + await expect + .poll(() => maskEditor.getCanvasSnapshot(2)) + .toBe(maskSnapshotAfterPen) + }) + + test("Switching between tools preserves previous tool's mask data", async ({ + maskEditor + }) => { + const dialog = await maskEditor.openDialog() + + await maskEditor.drawStrokeOnPointerZone(dialog) + await expect + .poll(() => maskEditor.pollMaskPixelCount()) + .toBeGreaterThan(0) + + const maskSnapshot = await maskEditor.getCanvasSnapshot(2) + + const toolEntries = dialog.locator('.maskEditor_toolPanelContainer') + await expect(toolEntries).toHaveCount(5) + + await toolEntries.nth(ERASER_TOOL_INDEX).click() + await expect(toolEntries.nth(ERASER_TOOL_INDEX)).toHaveClass(/Selected/) + + await toolEntries.nth(0).click() + await expect(toolEntries.nth(0)).toHaveClass(/Selected/) + + await expect + .poll(() => maskEditor.getCanvasSnapshot(2)) + .toBe(maskSnapshot) + }) + }) + } +) diff --git a/src/components/maskeditor/BrushSettingsPanel.vue b/src/components/maskeditor/BrushSettingsPanel.vue index 99b9dd5412..31a4f476e0 100644 --- a/src/components/maskeditor/BrushSettingsPanel.vue +++ b/src/components/maskeditor/BrushSettingsPanel.vue @@ -64,6 +64,7 @@ Date: Mon, 4 May 2026 17:32:57 -0700 Subject: [PATCH 010/172] fix(load3d): preserve unknown Model Config fields with spread (#11838) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary Use spread pattern when writing `nodeValue.properties['Model Config']` so future ModelConfig fields are preserved across viewer dialog cancel/apply. ## Changes - **What**: Spread existing `Model Config` before applying known keys in both `restoreInitialState()` and `applyChanges()` in [useLoad3dViewer.ts](src/composables/useLoad3dViewer.ts). Removes the hard-coded `showSkeleton: false` override from `applyChanges()` so it falls through from the existing config. ## Review Focus The change is intentionally minimal and matches the suggestion in the upstream issue. Two regression tests added (one each for restore/apply) verify that an unknown future field on Model Config survives both code paths. Fixes #11346 ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-11838-fix-load3d-preserve-unknown-Model-Config-fields-with-spread-3546d73d3650819686efc4e1a9799ad9) by [Unito](https://www.unito.io) --- src/composables/useLoad3dViewer.test.ts | 34 +++++++++++++++++++++++++ src/composables/useLoad3dViewer.ts | 9 ++++++- 2 files changed, 42 insertions(+), 1 deletion(-) diff --git a/src/composables/useLoad3dViewer.test.ts b/src/composables/useLoad3dViewer.test.ts index a692871a3f..9b05e1b983 100644 --- a/src/composables/useLoad3dViewer.test.ts +++ b/src/composables/useLoad3dViewer.test.ts @@ -404,6 +404,23 @@ describe('useLoad3dViewer', () => { .intensity ).toBe(1) }) + + it('should preserve unknown fields on Model Config when restoring', async () => { + const viewer = useLoad3dViewer(mockNode) + const containerRef = document.createElement('div') + + await viewer.initializeViewer(containerRef, mockSourceLoad3d as Load3d) + ;( + mockNode.properties!['Model Config'] as Record + ).futureField = 'preserve-me' + + viewer.restoreInitialState() + + expect( + (mockNode.properties!['Model Config'] as Record) + .futureField + ).toBe('preserve-me') + }) }) describe('applyChanges', () => { @@ -457,6 +474,23 @@ describe('useLoad3dViewer', () => { expect(result).toBe(false) }) + + it('should preserve unknown fields on Model Config when applying', async () => { + const viewer = useLoad3dViewer(mockNode) + const containerRef = document.createElement('div') + + await viewer.initializeViewer(containerRef, mockSourceLoad3d as Load3d) + ;( + mockNode.properties!['Model Config'] as Record + ).futureField = 'preserve-me' + + await viewer.applyChanges() + + expect( + (mockNode.properties!['Model Config'] as Record) + .futureField + ).toBe('preserve-me') + }) }) describe('refreshViewport', () => { diff --git a/src/composables/useLoad3dViewer.ts b/src/composables/useLoad3dViewer.ts index 4de6ea8a54..b048c17bb4 100644 --- a/src/composables/useLoad3dViewer.ts +++ b/src/composables/useLoad3dViewer.ts @@ -619,7 +619,11 @@ export const useLoad3dViewer = (node?: LGraphNode) => { intensity: initialState.value.lightIntensity } + const existingModelConfig = nodeValue.properties['Model Config'] as + | ModelConfig + | undefined nodeValue.properties['Model Config'] = { + ...existingModelConfig, upDirection: initialState.value.upDirection, materialMode: initialState.value.materialMode, gizmo: { @@ -671,10 +675,13 @@ export const useLoad3dViewer = (node?: LGraphNode) => { } const gizmoTransform = load3d.getGizmoTransform() + const existingModelConfig = nodeValue.properties['Model Config'] as + | ModelConfig + | undefined nodeValue.properties['Model Config'] = { + ...existingModelConfig, upDirection: upDirection.value, materialMode: materialMode.value, - showSkeleton: false, gizmo: { enabled: gizmoEnabled.value, mode: gizmoMode.value, From 9013102db9afe21f639e192532ad8b5c1a08ca3d Mon Sep 17 00:00:00 2001 From: Dante Date: Tue, 5 May 2026 09:38:31 +0900 Subject: [PATCH 011/172] fix: use capitalize for keybinding badges (#11810) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary Render keybinding badges in sentence case (`Ctrl + Shift + A`) instead of UPPERCASE (`CTRL + SHIFT + A`) by swapping the `uppercase` Tailwind class for `capitalize` in `KeyComboDisplay.vue`. `KeyComboImpl.getKeySequences()` already returns labels in their canonical form (`Ctrl`, `Alt`, `Shift`, plus the raw key). The badge styling was forcing them all to UPPER, which is what FE-524 calls out. `text-transform: capitalize` cleanly handles every case: lower modifier, upper modifier, and single character keys. - Fixes FE-524 ## Before / After | Before (`uppercase`) | After (`capitalize`) | | --- | --- | | | | ## Test plan - [ ] Open Settings → Keybinding panel and confirm modifier badges render as `Ctrl`, `Alt`, `Shift` instead of `CTRL`, `ALT`, `SHIFT` - [ ] Confirm single-letter keys (e.g. `A`, `S`) still render uppercase - [ ] Edit a keybinding and verify the live preview badges in the dialog also render in sentence case --- .../dialog/content/setting/keybinding/KeyComboDisplay.vue | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/components/dialog/content/setting/keybinding/KeyComboDisplay.vue b/src/components/dialog/content/setting/keybinding/KeyComboDisplay.vue index 7fb0ee4ba3..9e5d81b52b 100644 --- a/src/components/dialog/content/setting/keybinding/KeyComboDisplay.vue +++ b/src/components/dialog/content/setting/keybinding/KeyComboDisplay.vue @@ -2,7 +2,7 @@ diff --git a/src/components/queue/job/JobDetailsHoverPopover.vue b/src/components/queue/job/JobDetailsHoverPopover.vue new file mode 100644 index 0000000000..41b2369848 --- /dev/null +++ b/src/components/queue/job/JobDetailsHoverPopover.vue @@ -0,0 +1,62 @@ + + + diff --git a/src/components/queue/job/getHoverPopoverPosition.test.ts b/src/components/queue/job/getHoverPopoverPosition.test.ts deleted file mode 100644 index 9cd8595ccf..0000000000 --- a/src/components/queue/job/getHoverPopoverPosition.test.ts +++ /dev/null @@ -1,61 +0,0 @@ -import { describe, expect, it } from 'vitest' - -import { getHoverPopoverPosition } from './getHoverPopoverPosition' - -describe('getHoverPopoverPosition', () => { - it('places the popover to the right when space is available', () => { - const position = getHoverPopoverPosition( - { top: 100, left: 40, right: 240 }, - 1280 - ) - expect(position).toEqual({ top: 100, left: 248 }) - }) - - it('places the popover to the left when right space is insufficient', () => { - const position = getHoverPopoverPosition( - { top: 100, left: 980, right: 1180 }, - 1280 - ) - expect(position).toEqual({ top: 100, left: 672 }) - }) - - it('clamps the top to viewport padding when rect.top is near the top edge', () => { - const position = getHoverPopoverPosition( - { top: 2, left: 40, right: 240 }, - 1280 - ) - expect(position).toEqual({ top: 8, left: 248 }) - }) - - it('clamps left to viewport padding when fallback would go off-screen', () => { - const position = getHoverPopoverPosition( - { top: 100, left: 100, right: 300 }, - 320 - ) - expect(position).toEqual({ top: 100, left: 8 }) - }) - - it('prefers right when both sides have equal space', () => { - const position = getHoverPopoverPosition( - { top: 200, left: 340, right: 640 }, - 1280 - ) - expect(position).toEqual({ top: 200, left: 648 }) - }) - - it('falls back to left when right space is less than popover width', () => { - const position = getHoverPopoverPosition( - { top: 100, left: 600, right: 1000 }, - 1280 - ) - expect(position).toEqual({ top: 100, left: 292 }) - }) - - it('handles narrow viewport where popover barely fits', () => { - const position = getHoverPopoverPosition( - { top: 50, left: 8, right: 100 }, - 316 - ) - expect(position).toEqual({ top: 50, left: 8 }) - }) -}) diff --git a/src/components/queue/job/getHoverPopoverPosition.ts b/src/components/queue/job/getHoverPopoverPosition.ts deleted file mode 100644 index b339b78997..0000000000 --- a/src/components/queue/job/getHoverPopoverPosition.ts +++ /dev/null @@ -1,39 +0,0 @@ -const POPOVER_GAP = 8 -const POPOVER_WIDTH = 300 -const VIEWPORT_PADDING = 8 - -type AnchorRect = Pick - -type HoverPopoverPosition = { - top: number - left: number -} - -export function getHoverPopoverPosition( - rect: AnchorRect, - viewportWidth: number -): HoverPopoverPosition { - const availableLeft = rect.left - POPOVER_GAP - const availableRight = viewportWidth - rect.right - POPOVER_GAP - const preferredLeft = rect.right + POPOVER_GAP - const fallbackLeft = rect.left - POPOVER_WIDTH - POPOVER_GAP - const maxLeft = Math.max( - VIEWPORT_PADDING, - viewportWidth - POPOVER_WIDTH - VIEWPORT_PADDING - ) - - if ( - availableRight >= POPOVER_WIDTH && - (availableRight >= availableLeft || availableLeft < POPOVER_WIDTH) - ) { - return { - top: Math.max(VIEWPORT_PADDING, rect.top), - left: Math.min(maxLeft, preferredLeft) - } - } - - return { - top: Math.max(VIEWPORT_PADDING, rect.top), - left: Math.max(VIEWPORT_PADDING, Math.min(maxLeft, fallbackLeft)) - } -} diff --git a/src/components/ui/popover/Popover.vue b/src/components/ui/popover/Popover.vue new file mode 100644 index 0000000000..f1eed73eb9 --- /dev/null +++ b/src/components/ui/popover/Popover.vue @@ -0,0 +1,16 @@ + + + diff --git a/src/components/ui/popover/PopoverContent.vue b/src/components/ui/popover/PopoverContent.vue new file mode 100644 index 0000000000..62d1be247d --- /dev/null +++ b/src/components/ui/popover/PopoverContent.vue @@ -0,0 +1,49 @@ + + + diff --git a/src/composables/queue/useJobDetailsHover.ts b/src/composables/queue/useJobDetailsHover.ts deleted file mode 100644 index 8efd98fc43..0000000000 --- a/src/composables/queue/useJobDetailsHover.ts +++ /dev/null @@ -1,107 +0,0 @@ -import { onBeforeUnmount, ref, watch } from 'vue' - -import type { JobGroup } from '@/composables/queue/useJobList' - -const DETAILS_SHOW_DELAY_MS = 200 -const DETAILS_HIDE_DELAY_MS = 150 - -interface UseJobDetailsHoverOptions { - getActiveId: (active: TActive) => string - getDisplayedJobGroups: () => JobGroup[] - onReset?: () => void -} - -export function useJobDetailsHover({ - getActiveId, - getDisplayedJobGroups, - onReset -}: UseJobDetailsHoverOptions) { - const activeDetails = ref(null) - const hideTimer = ref(null) - const hideTimerJobId = ref(null) - const showTimer = ref(null) - - function clearHideTimer() { - if (hideTimer.value !== null) { - clearTimeout(hideTimer.value) - hideTimer.value = null - } - hideTimerJobId.value = null - } - - function clearShowTimer() { - if (showTimer.value !== null) { - clearTimeout(showTimer.value) - showTimer.value = null - } - } - - function clearHoverTimers() { - clearHideTimer() - clearShowTimer() - } - - function resetActiveDetails() { - clearHoverTimers() - activeDetails.value = null - onReset?.() - } - - function hasDisplayedJob(jobId: string) { - return getDisplayedJobGroups().some((group) => - group.items.some((item) => item.id === jobId) - ) - } - - function scheduleDetailsShow(nextActive: TActive, onShow?: () => void) { - const nextActiveId = getActiveId(nextActive) - clearShowTimer() - showTimer.value = window.setTimeout(() => { - showTimer.value = null - if (!hasDisplayedJob(nextActiveId)) return - - activeDetails.value = nextActive - onShow?.() - }, DETAILS_SHOW_DELAY_MS) - } - - function scheduleDetailsHide(jobId?: string, onHide?: () => void) { - if (!jobId) return - - clearShowTimer() - if (hideTimerJobId.value && hideTimerJobId.value !== jobId) { - return - } - - clearHideTimer() - hideTimerJobId.value = jobId - hideTimer.value = window.setTimeout(() => { - const currentActive = activeDetails.value - if (currentActive && getActiveId(currentActive) === jobId) { - activeDetails.value = null - onHide?.() - } - hideTimer.value = null - hideTimerJobId.value = null - }, DETAILS_HIDE_DELAY_MS) - } - - watch(getDisplayedJobGroups, () => { - const currentActive = activeDetails.value - if (!currentActive) return - - if (!hasDisplayedJob(getActiveId(currentActive))) { - resetActiveDetails() - } - }) - - onBeforeUnmount(resetActiveDetails) - - return { - activeDetails, - clearHoverTimers, - resetActiveDetails, - scheduleDetailsHide, - scheduleDetailsShow - } -} From 6822a6883d564eab0ebe03f755c2b73fb8286524 Mon Sep 17 00:00:00 2001 From: pythongosssss <125205205+pythongosssss@users.noreply.github.com> Date: Tue, 5 May 2026 09:47:07 +0100 Subject: [PATCH 020/172] test: add tests for layout settings (#11692) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary Adds tests for UI layout settings ## Changes - **What**: - add initialFeatureFlags to allow setting feature flags before initial setup to prevent needing to reload page - tests sidebar + topbar settings ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-11692-test-add-tests-for-layout-settings-34f6d73d36508117b1daedbb68176e04) by [Unito](https://www.unito.io) --------- Co-authored-by: github-actions --- browser_tests/fixtures/ComfyPage.ts | 11 +- browser_tests/fixtures/components/Topbar.ts | 7 + browser_tests/fixtures/selectors.ts | 2 + .../tests/layoutSidebarSettings.spec.ts | 127 ++++++++++++++++++ src/components/graph/GraphCanvas.vue | 1 + src/components/topbar/WorkflowTabs.vue | 1 + 6 files changed, 148 insertions(+), 1 deletion(-) create mode 100644 browser_tests/tests/layoutSidebarSettings.spec.ts diff --git a/browser_tests/fixtures/ComfyPage.ts b/browser_tests/fixtures/ComfyPage.ts index 7be16a4748..4d06834f06 100644 --- a/browser_tests/fixtures/ComfyPage.ts +++ b/browser_tests/fixtures/ComfyPage.ts @@ -460,10 +460,15 @@ export const testComfySnapToGridGridSize = 50 const COLLECT_COVERAGE = process.env.COLLECT_COVERAGE === 'true' export const comfyPageFixture = base.extend<{ + initialFeatureFlags: Record comfyPage: ComfyPage comfyMouse: ComfyMouse comfyFiles: ComfyFiles }>({ + // Allows configuring feature flags for tests with before initial setup: + // `test.use({ initialFeatureFlags: { my_flag: true } })`. + initialFeatureFlags: [{}, { option: true }], + page: async ({ page, browserName }, use) => { if (browserName !== 'chromium' || !COLLECT_COVERAGE) { return use(page) @@ -480,7 +485,7 @@ export const comfyPageFixture = base.extend<{ await mcr.add(coverage) }, - comfyPage: async ({ page, request }, use, testInfo) => { + comfyPage: async ({ page, request, initialFeatureFlags }, use, testInfo) => { const comfyPage = new ComfyPage(page, request) const { parallelIndex } = testInfo @@ -524,6 +529,10 @@ export const comfyPageFixture = base.extend<{ await comfyPage.cloudAuth.mockAuth() } + if (Object.keys(initialFeatureFlags).length > 0) { + await comfyPage.featureFlags.seedFlags(initialFeatureFlags) + } + await comfyPage.setup() if (isVueNodes) { diff --git a/browser_tests/fixtures/components/Topbar.ts b/browser_tests/fixtures/components/Topbar.ts index 96e6e3866c..c91ccf11e8 100644 --- a/browser_tests/fixtures/components/Topbar.ts +++ b/browser_tests/fixtures/components/Topbar.ts @@ -1,16 +1,23 @@ import type { Locator, Page } from '@playwright/test' import type { WorkspaceStore } from '@e2e/types/globals' +import { TestIds } from '@e2e/fixtures/selectors' export class Topbar { private readonly menuLocator: Locator private readonly menuTrigger: Locator readonly newWorkflowButton: Locator + readonly workflowTabs: Locator + readonly integratedTabBarActions: Locator constructor(public readonly page: Page) { this.menuLocator = page.locator('.comfy-command-menu') this.menuTrigger = page.locator('.comfy-menu-button-wrapper') this.newWorkflowButton = page.locator('.new-blank-workflow-button') + this.workflowTabs = page.getByTestId(TestIds.topbar.workflowTabs) + this.integratedTabBarActions = this.workflowTabs.getByTestId( + TestIds.topbar.integratedTabBarActions + ) } async getTabNames(): Promise { diff --git a/browser_tests/fixtures/selectors.ts b/browser_tests/fixtures/selectors.ts index 26de794dff..3f7a9f9c09 100644 --- a/browser_tests/fixtures/selectors.ts +++ b/browser_tests/fixtures/selectors.ts @@ -91,6 +91,8 @@ export const TestIds = { loginButton: 'login-button', loginButtonPopover: 'login-button-popover', loginButtonPopoverLearnMore: 'login-button-popover-learn-more', + workflowTabs: 'topbar-workflow-tabs', + integratedTabBarActions: 'integrated-tab-bar-actions', actionBarButtons: 'action-bar-buttons' }, nodeLibrary: { diff --git a/browser_tests/tests/layoutSidebarSettings.spec.ts b/browser_tests/tests/layoutSidebarSettings.spec.ts new file mode 100644 index 0000000000..497df6f8a1 --- /dev/null +++ b/browser_tests/tests/layoutSidebarSettings.spec.ts @@ -0,0 +1,127 @@ +import { + comfyExpect as expect, + comfyPageFixture as test +} from '@e2e/fixtures/ComfyPage' + +test.describe('Layout & sidebar settings', { tag: ['@settings'] }, () => { + test.describe('Comfy.Sidebar.Size', () => { + test('"small" applies small-sidebar class', async ({ comfyPage }) => { + await comfyPage.settings.setSetting('Comfy.Sidebar.Size', 'small') + await expect(comfyPage.menu.sideToolbar).toContainClass('small-sidebar') + }) + + test('"normal" does not apply small-sidebar class', async ({ + comfyPage + }) => { + await comfyPage.settings.setSetting('Comfy.Sidebar.Size', 'normal') + await expect(comfyPage.menu.sideToolbar).not.toContainClass( + 'small-sidebar' + ) + }) + }) + + test.describe('Comfy.Sidebar.Style', () => { + // `isConnected` overrides the Style setting when the toolbar overflows; + // small (48px) items keep content under the default viewport so Style + // actually drives rendering. + test.beforeEach(async ({ comfyPage }) => { + await comfyPage.settings.setSetting('Comfy.Sidebar.Size', 'small') + }) + + test('"connected" applies connected-sidebar class', async ({ + comfyPage + }) => { + await comfyPage.settings.setSetting('Comfy.Sidebar.Style', 'connected') + await expect(comfyPage.menu.sideToolbar).toContainClass( + 'connected-sidebar' + ) + await expect(comfyPage.menu.sideToolbar).not.toContainClass( + 'floating-sidebar' + ) + }) + + test('"floating" applies floating-sidebar class', async ({ comfyPage }) => { + await comfyPage.settings.setSetting('Comfy.Sidebar.Style', 'floating') + await expect(comfyPage.menu.sideToolbar).toContainClass( + 'floating-sidebar' + ) + await expect(comfyPage.menu.sideToolbar).not.toContainClass( + 'connected-sidebar' + ) + }) + + test('"floating" + Size "normal" is overridden to connected by overflow', async ({ + comfyPage + }) => { + await comfyPage.settings.setSetting('Comfy.Sidebar.Size', 'normal') + await comfyPage.settings.setSetting('Comfy.Sidebar.Style', 'floating') + await expect(comfyPage.menu.sideToolbar).toContainClass( + 'connected-sidebar' + ) + await expect(comfyPage.menu.sideToolbar).toContainClass( + 'overflowing-sidebar' + ) + }) + + test('"floating" + Size "normal" renders floating in a viewport tall enough to fit', async ({ + comfyPage + }) => { + await comfyPage.settings.setSetting('Comfy.Sidebar.Size', 'normal') + await comfyPage.settings.setSetting('Comfy.Sidebar.Style', 'floating') + await comfyPage.page.setViewportSize({ width: 1280, height: 1500 }) + await expect(comfyPage.menu.sideToolbar).toContainClass( + 'floating-sidebar' + ) + await expect(comfyPage.menu.sideToolbar).not.toContainClass( + 'overflowing-sidebar' + ) + }) + }) + + test.describe('Comfy.UI.TabBarLayout', () => { + test('"Default" renders integrated tab bar actions container', async ({ + comfyPage + }) => { + await comfyPage.settings.setSetting('Comfy.UI.TabBarLayout', 'Default') + await expect(comfyPage.menu.topbar.integratedTabBarActions).toBeAttached() + }) + + test('"Legacy" does not render integrated tab bar actions container', async ({ + comfyPage + }) => { + await comfyPage.settings.setSetting('Comfy.UI.TabBarLayout', 'Legacy') + await expect(comfyPage.menu.topbar.integratedTabBarActions).toHaveCount(0) + }) + }) + + test.describe('Comfy.TreeExplorer.ItemPadding', () => { + // The setting writes a CSS var consumed by .p-tree-node-content, + // which only renders in the legacy PrimeVue Tree. + test.beforeEach(async ({ comfyPage }) => { + await comfyPage.settings.setSetting('Comfy.NodeLibrary.NewDesign', false) + await comfyPage.menu.nodeLibraryTab.open() + }) + + test('low padding (0px) is applied to tree node content', async ({ + comfyPage + }) => { + await comfyPage.settings.setSetting('Comfy.TreeExplorer.ItemPadding', 0) + await expect( + comfyPage.menu.nodeLibraryTab.nodeLibraryTree + .locator('.p-tree-node-content') + .first() + ).toHaveCSS('padding', '0px') + }) + + test('high padding (8px) is applied to tree node content', async ({ + comfyPage + }) => { + await comfyPage.settings.setSetting('Comfy.TreeExplorer.ItemPadding', 8) + await expect( + comfyPage.menu.nodeLibraryTab.nodeLibraryTree + .locator('.p-tree-node-content') + .first() + ).toHaveCSS('padding', '8px') + }) + }) +}) diff --git a/src/components/graph/GraphCanvas.vue b/src/components/graph/GraphCanvas.vue index cd4909417c..67ef64d744 100644 --- a/src/components/graph/GraphCanvas.vue +++ b/src/components/graph/GraphCanvas.vue @@ -6,6 +6,7 @@