diff --git a/browser_tests/tests/rendererToggleStability.spec.ts b/browser_tests/tests/rendererToggleStability.spec.ts new file mode 100644 index 0000000000..8bdb673d8a --- /dev/null +++ b/browser_tests/tests/rendererToggleStability.spec.ts @@ -0,0 +1,104 @@ +import { + comfyExpect as expect, + comfyPageFixture as test +} from '../fixtures/ComfyPage' +import type { ComfyPage } from '../fixtures/ComfyPage' +import type { Position } from '../fixtures/types' + +type NodeSnapshot = { id: number } & Position + +async function getAllNodePositions( + comfyPage: ComfyPage +): Promise { + return comfyPage.page.evaluate(() => + window.app!.graph.nodes.map((n) => ({ + id: n.id as number, + x: n.pos[0], + y: n.pos[1] + })) + ) +} + +async function getNodePosition( + comfyPage: ComfyPage, + nodeId: number +): Promise { + return comfyPage.page.evaluate((targetNodeId) => { + const node = window.app!.graph.nodes.find((n) => n.id === targetNodeId) + if (!node) return + + return { + x: node.pos[0], + y: node.pos[1] + } + }, nodeId) +} + +async function expectNodePositionStable( + comfyPage: ComfyPage, + initial: NodeSnapshot, + mode: string +) { + await expect + .poll( + async () => { + const current = await getNodePosition(comfyPage, initial.id) + return current?.x ?? Number.NaN + }, + { message: `node ${initial.id} x drifted in ${mode} mode` } + ) + .toBeCloseTo(initial.x, 1) + + await expect + .poll( + async () => { + const current = await getNodePosition(comfyPage, initial.id) + return current?.y ?? Number.NaN + }, + { message: `node ${initial.id} y drifted in ${mode} mode` } + ) + .toBeCloseTo(initial.y, 1) +} + +async function setVueMode(comfyPage: ComfyPage, enabled: boolean) { + await comfyPage.settings.setSetting('Comfy.VueNodes.Enabled', enabled) + if (enabled) { + await comfyPage.vueNodes.waitForNodes() + } + await comfyPage.nextFrame() +} + +test.describe( + 'Renderer toggle stability', + { tag: ['@node', '@canvas'] }, + () => { + test('node positions do not drift when toggling between Vue and LiteGraph renderers', async ({ + comfyPage + }) => { + const TOGGLE_COUNT = 5 + + const initialPositions = await getAllNodePositions(comfyPage) + expect(initialPositions.length).toBeGreaterThan(0) + + for (let i = 0; i < TOGGLE_COUNT; i++) { + await setVueMode(comfyPage, true) + for (const initial of initialPositions) { + await expectNodePositionStable( + comfyPage, + initial, + `Vue toggle ${i + 1}` + ) + } + + await setVueMode(comfyPage, false) + for (const initial of initialPositions) { + await expectNodePositionStable( + comfyPage, + initial, + `LiteGraph toggle ${i + 1}` + ) + } + } + }) + } +) diff --git a/src/components/TopMenuSection.test.ts b/src/components/TopMenuSection.test.ts index ac626a400b..c734ae9490 100644 --- a/src/components/TopMenuSection.test.ts +++ b/src/components/TopMenuSection.test.ts @@ -78,6 +78,14 @@ vi.mock('@/stores/firebaseAuthStore', () => ({ })) })) +vi.mock('@/scripts/app', () => ({ + app: { + menu: { + element: document.createElement('div') + } + } +})) + type WrapperOptions = { pinia?: ReturnType stubs?: Record @@ -131,6 +139,18 @@ function createWrapper({ }) } +function getLegacyCommandsContainer( + wrapper: ReturnType +): HTMLElement { + const legacyContainer = wrapper.find( + '[data-testid="legacy-topbar-container"]' + ).element + if (!(legacyContainer instanceof HTMLElement)) { + throw new Error('Expected legacy commands container to be present') + } + return legacyContainer +} + function createJob(id: string, status: JobStatus): JobListItem { return { id, @@ -515,4 +535,69 @@ describe('TopMenuSection', () => { expect(wrapper.find('span.bg-red-500').exists()).toBe(true) }) + + it('coalesces legacy topbar mutation scans to one check per frame', async () => { + localStorage.setItem('Comfy.MenuPosition.Docked', 'false') + + const rafCallbacks: FrameRequestCallback[] = [] + vi.stubGlobal('requestAnimationFrame', (cb: FrameRequestCallback) => { + rafCallbacks.push(cb) + return rafCallbacks.length + }) + vi.stubGlobal('cancelAnimationFrame', vi.fn()) + + const pinia = createTestingPinia({ createSpy: vi.fn }) + const settingStore = useSettingStore(pinia) + vi.mocked(settingStore.get).mockImplementation((key) => { + if (key === 'Comfy.UseNewMenu') return 'Top' + if (key === 'Comfy.UI.TabBarLayout') return 'Integrated' + if (key === 'Comfy.RightSidePanel.IsOpen') return true + return undefined + }) + + const wrapper = createWrapper({ pinia, attachTo: document.body }) + + try { + await nextTick() + + const actionbarContainer = wrapper.find('.actionbar-container') + expect(actionbarContainer.classes()).toContain('w-0') + + const legacyContainer = getLegacyCommandsContainer(wrapper) + const querySpy = vi.spyOn(legacyContainer, 'querySelector') + + if (rafCallbacks.length > 0) { + const initialCallbacks = [...rafCallbacks] + rafCallbacks.length = 0 + initialCallbacks.forEach((callback) => callback(0)) + await nextTick() + } + querySpy.mockClear() + querySpy.mockReturnValue(document.createElement('div')) + + for (let index = 0; index < 3; index++) { + const outer = document.createElement('div') + const inner = document.createElement('div') + inner.textContent = `legacy-${index}` + outer.appendChild(inner) + legacyContainer.appendChild(outer) + } + + await vi.waitFor(() => { + expect(rafCallbacks.length).toBeGreaterThan(0) + }) + expect(querySpy).not.toHaveBeenCalled() + + const callbacks = [...rafCallbacks] + rafCallbacks.length = 0 + callbacks.forEach((callback) => callback(0)) + await nextTick() + + expect(querySpy).toHaveBeenCalledTimes(1) + expect(actionbarContainer.classes()).toContain('px-2') + } finally { + wrapper.unmount() + vi.unstubAllGlobals() + } + }) }) diff --git a/src/components/TopMenuSection.vue b/src/components/TopMenuSection.vue index 5994be4da4..81ff088dca 100644 --- a/src/components/TopMenuSection.vue +++ b/src/components/TopMenuSection.vue @@ -39,6 +39,7 @@
@@ -116,7 +117,7 @@