Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
import type { Page } from '@playwright/test'
import { mergeTests } from '@playwright/test'

import { assetApiFixture } from '@e2e/fixtures/assetApiFixture'
import {
comfyExpect as expect,
comfyPageFixture
} from '@e2e/fixtures/ComfyPage'
import { withInputFiles } from '@e2e/fixtures/helpers/AssetHelper'

const test = mergeTests(comfyPageFixture, assetApiFixture)

async function addNode(page: Page, nodeType: string): Promise<string> {
return page.evaluate((type) => {
const node = window.app!.graph.add(
window.LiteGraph!.createNode(type, undefined, {})
)
return String(node!.id)
}, nodeType)
}

test.describe(
'LoadImage form dropdown reopen (FE-535)',
{ tag: '@vue-nodes' },
() => {
test('items remain visible after scroll → close → reopen', async ({
comfyPage,
assetApi
}) => {
assetApi.configure(withInputFiles(60))
await assetApi.mock()

await comfyPage.appMode.enableLinearMode()
const loadImageId = await addNode(comfyPage.page, 'LoadImage')
await comfyPage.nextFrame()
await comfyPage.appMode.enterAppModeWithInputs([[loadImageId, 'image']])

const widgetList = comfyPage.appMode.linearWidgets
await expect(widgetList).toBeVisible()

const imageRow = widgetList.locator(
'div:has(> div > span:text-is("image"))'
)
const dropdownButton = imageRow.locator('button:has(> span)').first()

await dropdownButton.click()
const popover = comfyPage.appMode.imagePickerPopover
await expect(popover).toBeVisible()

const scrollContainer = popover
.locator('[data-capture-wheel] > div')
.nth(2)
await expect
.poll(() => scrollContainer.locator('img').count())
.toBeGreaterThan(0)

await scrollContainer.evaluate((el) =>
el.scrollTo({ top: el.scrollHeight, behavior: 'instant' })
)

await comfyPage.page.keyboard.press('Escape')
await expect(popover).toBeHidden()

await dropdownButton.click()
await expect(popover).toBeVisible()

await expect
.poll(() => popover.locator('img').count(), { timeout: 5000 })
.toBeGreaterThan(0)
})
}
)
58 changes: 58 additions & 0 deletions src/components/common/VirtualGrid.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -239,6 +239,64 @@ describe('VirtualGrid', () => {
expect(onApproachEnd).not.toHaveBeenCalled()
})

it('renders last page when scrollY exceeds natural max (FE-535)', async () => {
const items = createItems(5)
mockedWidth.value = 400
mockedHeight.value = 200
mockedScrollY.value = 5000

render(VirtualGrid, {
props: {
items,
gridStyle: defaultGridStyle,
defaultItemHeight: 100,
defaultItemWidth: 100,
maxColumns: 4,
bufferRows: 1
},
slots: {
item: `<template #item="{ item }">
<div class="test-item">{{ item.name }}</div>
</template>`
},
container: document.body.appendChild(document.createElement('div'))
})

await nextTick()

const renderedItems = screen.queryAllByText(/^Item \d+$/)
expect(renderedItems.length).toBeGreaterThan(0)
})

it('keeps last page visible when items shrink below current scroll (FE-535)', async () => {
const items = createItems(8)
mockedWidth.value = 400
mockedHeight.value = 240
mockedScrollY.value = 3000

render(VirtualGrid, {
props: {
items,
gridStyle: defaultGridStyle,
defaultItemHeight: 100,
defaultItemWidth: 100,
maxColumns: 4,
bufferRows: 1
},
slots: {
item: `<template #item="{ item }">
<div class="test-item">{{ item.name }}</div>
</template>`
},
container: document.body.appendChild(document.createElement('div'))
})

await nextTick()

const renderedItems = screen.queryAllByText(/^Item \d+$/)
expect(renderedItems.length).toBeGreaterThan(0)
})

it('forces cols to maxColumns when maxColumns is finite', async () => {
mockedWidth.value = 100
mockedHeight.value = 200
Expand Down
13 changes: 12 additions & 1 deletion src/components/common/VirtualGrid.vue
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

WDYT about migratintg the form dropdown widget to tanstack first, rather than mass-migrating all consumers of VirtualGrid like this?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Done — narrowed to FormDropdownMenu in 89fedb2. VirtualGrid is back to its pre-migration state (with the clamp offsetRows fix already on main); only FormDropdownMenu uses useVirtualizer locally. Updated the PR description with the rationale.

Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,18 @@ const mergedGridStyle = computed<CSSProperties>(() => {
})

const viewRows = computed(() => Math.ceil(height.value / itemHeight.value))
const offsetRows = computed(() => Math.floor(scrollY.value / itemHeight.value))
const totalRows = computed(() => Math.ceil(items.length / cols.value))
const maxOffsetRows = computed(() =>
Math.max(0, totalRows.value - viewRows.value)
)
// Clamp offsetRows so the last page stays visible when scrollY drifts past
// the natural max (e.g. items list shrinks while scroll position is retained,
// or rubberband over-scroll temporarily exceeds the limit). Without this,
// state.start === state.end === items.length and the grid renders blank
// until another scroll event re-syncs the position. (FE-535)
const offsetRows = computed(() =>
Math.min(maxOffsetRows.value, Math.floor(scrollY.value / itemHeight.value))
)
const isValidGrid = computed(() => height.value && width.value && items?.length)

const state = computed<GridState>(() => {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,15 +1,44 @@
import { render, screen } from '@testing-library/vue'
import { describe, expect, it } from 'vitest'
import { beforeEach, describe, expect, it, vi } from 'vitest'
import type { Ref } from 'vue'
import { ref } from 'vue'

import FormDropdownMenu from './FormDropdownMenu.vue'
import type { FormDropdownItem, LayoutMode } from './types'

const VirtualGridStub = {
name: 'VirtualGrid',
props: ['items', 'maxColumns', 'itemHeight', 'scrollerHeight'],
template:
'<div data-testid="virtual-grid" :data-items="JSON.stringify(items)" :data-max-columns="maxColumns" />'
}
let mockedVisibleEnd: Ref<number>

vi.mock('@tanstack/vue-virtual', async () => {
const { computed } = await import('vue')
return {
useVirtualizer: (options: {
count: number
estimateSize: (index: number) => number
}) =>
computed(() => {
const count = options.count
const end = Math.min(count, mockedVisibleEnd.value)
const items = Array.from({ length: end }, (_, index) => {
const size = options.estimateSize(index)
return {
index,
key: index,
start: index * size,
size
}
})
const totalSize = count * (count > 0 ? options.estimateSize(0) : 0)
return {
getVirtualItems: () => items,
getTotalSize: () => totalSize
}
})
}
})

beforeEach(() => {
mockedVisibleEnd = ref(Infinity)
})

function createItem(id: string, name: string): FormDropdownItem {
return {
Expand All @@ -32,7 +61,11 @@ describe('FormDropdownMenu', () => {
stubs: {
FormDropdownMenuFilter: true,
FormDropdownMenuActions: true,
VirtualGrid: VirtualGridStub
FormDropdownMenuItem: {
name: 'FormDropdownMenuItem',
props: ['index', 'name', 'label', 'layout', 'selected', 'previewUrl'],
template: '<div data-testid="dropdown-item">{{ name }}</div>'
}
},
mocks: {
$t: (key: string) => key
Expand All @@ -53,44 +86,50 @@ describe('FormDropdownMenu', () => {
expect(emptyIcon).not.toBeNull()
})

it('renders VirtualGrid when items exist', () => {
it('renders dropdown items when items exist', () => {
render(FormDropdownMenu, {
props: defaultProps,
global: globalConfig
})

expect(screen.getByTestId('virtual-grid')).toBeTruthy()
const items = screen.getAllByTestId('dropdown-item')
expect(items.length).toBe(2)
expect(items[0].textContent).toBe('Item 1')
expect(items[1].textContent).toBe('Item 2')
})

it('transforms items to include key property for VirtualGrid', () => {
const items = [createItem('1', 'Item 1'), createItem('2', 'Item 2')]
render(FormDropdownMenu, {
it('uses single column layout for list mode', () => {
const { container } = render(FormDropdownMenu, {
props: {
...defaultProps,
items
layoutMode: 'list' as LayoutMode
},
global: globalConfig
})

const virtualGrid = screen.getByTestId('virtual-grid')
const virtualItems = JSON.parse(virtualGrid.getAttribute('data-items')!)

expect(virtualItems).toHaveLength(2)
expect(virtualItems[0]).toHaveProperty('key', '1')
expect(virtualItems[1]).toHaveProperty('key', '2')
// eslint-disable-next-line testing-library/no-container, testing-library/no-node-access
const gridRow = container.querySelector(
'[style*="grid-template-columns"]'
) as HTMLElement
expect(gridRow).not.toBeNull()
expect(gridRow.style.gridTemplateColumns).toBe('repeat(1, minmax(0, 1fr))')
})

it('uses single column layout for list modes', () => {
render(FormDropdownMenu, {
it('uses 4-column grid layout for grid mode', () => {
const { container } = render(FormDropdownMenu, {
props: {
...defaultProps,
layoutMode: 'list' as LayoutMode
layoutMode: 'grid' as LayoutMode
},
global: globalConfig
})

const virtualGrid = screen.getByTestId('virtual-grid')
expect(virtualGrid.getAttribute('data-max-columns')).toBe('1')
// eslint-disable-next-line testing-library/no-container, testing-library/no-node-access
const gridRow = container.querySelector(
'[style*="grid-template-columns"]'
) as HTMLElement
expect(gridRow).not.toBeNull()
expect(gridRow.style.gridTemplateColumns).toBe('repeat(4, minmax(0, 1fr))')
})

it('has data-capture-wheel="true" on the root element', () => {
Expand All @@ -104,4 +143,20 @@ describe('FormDropdownMenu', () => {
container.firstElementChild!.getAttribute('data-capture-wheel')
).toBe('true')
})

it('renders items without collapsing when count shrinks (FE-535)', async () => {
// FE-535: Popover keeps menu mounted on close, so scrollY persists across
// reopens. When `items` shrinks below the previous count, the previous
// hand-rolled offset math went blank. tanstack reads scrollOffset fresh
// from the DOM and the browser auto-clamps scrollTop when content shrinks,
// so the grid never silently collapses to a blank panel.
const items = Array.from({ length: 6 }, (_, i) =>
createItem(String(i + 1), `Item ${i + 1}`)
)
render(FormDropdownMenu, {
props: { ...defaultProps, items },
global: globalConfig
})
expect(screen.getAllByTestId('dropdown-item').length).toBe(items.length)
})
})
Loading
Loading