mirror of
https://github.com/Comfy-Org/ComfyUI_frontend.git
synced 2026-04-19 22:09:37 +00:00
fix: replace PrimeVue Popover with floating-ui positioning in FormDropdown
Replace PrimeVue Popover with @floating-ui/vue for dropdown positioning. PrimeVue's absolutePosition() mixes getBoundingClientRect() (viewport coordinates) with offsetHeight (local coordinates), which produces incorrect Y positions when nodes are inside TransformPane's scale3d() + translate3d() transform chain. The fix uses useFloating with flip(), shift(), and offset() middleware, plus v-if conditional rendering, onClickOutside for dismiss, and Escape key handling. Also removes the now-unnecessary useTransformCompatOverlayProps usage from WidgetSelectDropdown.vue and adds E2E tests verifying dropdown positioning at different zoom levels. Supersedes #10499.
This commit is contained in:
@@ -0,0 +1,145 @@
|
||||
import {
|
||||
comfyExpect as expect,
|
||||
comfyPageFixture as test
|
||||
} from '@e2e/fixtures/ComfyPage'
|
||||
|
||||
test.describe('FormDropdown Position Under CSS Transforms', () => {
|
||||
test.beforeEach(async ({ comfyPage }) => {
|
||||
await comfyPage.settings.setSetting('Comfy.VueNodes.Enabled', true)
|
||||
await comfyPage.workflow.loadWorkflow('widgets/load_image_widget')
|
||||
await comfyPage.vueNodes.waitForNodes()
|
||||
})
|
||||
|
||||
/**
|
||||
* Click the FormDropdown trigger button inside the LoadImage node.
|
||||
* The trigger is a button with a chevron icon inside the dropdown widget.
|
||||
*/
|
||||
async function clickDropdownTrigger(
|
||||
comfyPage: Parameters<Parameters<typeof test>[2]>[0]['comfyPage']
|
||||
) {
|
||||
const node = comfyPage.vueNodes.getNodeByTitle('Load Image')
|
||||
const trigger = node.locator('button').filter({
|
||||
has: comfyPage.page.locator('.icon-\\[lucide--chevron-down\\]')
|
||||
})
|
||||
await trigger.click()
|
||||
}
|
||||
|
||||
function getFloatingMenu(
|
||||
comfyPage: Parameters<Parameters<typeof test>[2]>[0]['comfyPage']
|
||||
) {
|
||||
return comfyPage.page.locator('[data-floating-menu]')
|
||||
}
|
||||
|
||||
function getTriggerButton(
|
||||
comfyPage: Parameters<Parameters<typeof test>[2]>[0]['comfyPage']
|
||||
) {
|
||||
const node = comfyPage.vueNodes.getNodeByTitle('Load Image')
|
||||
return node.locator('button').filter({
|
||||
has: comfyPage.page.locator('.icon-\\[lucide--chevron-down\\]')
|
||||
})
|
||||
}
|
||||
|
||||
test('dropdown menu appears near trigger at default zoom', async ({
|
||||
comfyPage
|
||||
}) => {
|
||||
await clickDropdownTrigger(comfyPage)
|
||||
|
||||
const menu = getFloatingMenu(comfyPage)
|
||||
await expect(menu).toBeVisible()
|
||||
|
||||
const trigger = getTriggerButton(comfyPage)
|
||||
const triggerBox = await trigger.boundingBox()
|
||||
const menuBox = await menu.boundingBox()
|
||||
|
||||
expect(triggerBox).not.toBeNull()
|
||||
expect(menuBox).not.toBeNull()
|
||||
|
||||
// Menu should appear below the trigger, within a reasonable gap
|
||||
// (offset middleware adds 8px, plus some tolerance for rounding)
|
||||
const gap = menuBox!.y - (triggerBox!.y + triggerBox!.height)
|
||||
expect(gap).toBeGreaterThanOrEqual(-2)
|
||||
expect(gap).toBeLessThanOrEqual(20)
|
||||
|
||||
// Menu should be horizontally aligned with the trigger
|
||||
const horizontalDrift = Math.abs(menuBox!.x - triggerBox!.x)
|
||||
expect(horizontalDrift).toBeLessThan(50)
|
||||
})
|
||||
|
||||
test('dropdown menu appears near trigger when zoomed out', async ({
|
||||
comfyPage
|
||||
}) => {
|
||||
// Zoom out using mouse wheel (triggers the real CSS transform pipeline)
|
||||
await comfyPage.canvasOps.zoom(300, 3)
|
||||
await comfyPage.nextFrame()
|
||||
|
||||
// Verify zoom actually changed
|
||||
const scale = await comfyPage.canvasOps.getScale()
|
||||
expect(scale).toBeLessThan(1)
|
||||
|
||||
await clickDropdownTrigger(comfyPage)
|
||||
|
||||
const menu = getFloatingMenu(comfyPage)
|
||||
await expect(menu).toBeVisible()
|
||||
|
||||
const trigger = getTriggerButton(comfyPage)
|
||||
const triggerBox = await trigger.boundingBox()
|
||||
const menuBox = await menu.boundingBox()
|
||||
|
||||
expect(triggerBox).not.toBeNull()
|
||||
expect(menuBox).not.toBeNull()
|
||||
|
||||
// Menu should still appear near the trigger, not at viewport origin
|
||||
// The bug caused menu to appear at (0, 0) or far from the trigger
|
||||
const gap = menuBox!.y - (triggerBox!.y + triggerBox!.height)
|
||||
expect(gap).toBeGreaterThanOrEqual(-2)
|
||||
expect(gap).toBeLessThanOrEqual(20)
|
||||
})
|
||||
|
||||
test('dropdown menu appears near trigger when zoomed in', async ({
|
||||
comfyPage
|
||||
}) => {
|
||||
// Zoom in using mouse wheel
|
||||
await comfyPage.canvasOps.zoom(-300, 3)
|
||||
await comfyPage.nextFrame()
|
||||
|
||||
const scale = await comfyPage.canvasOps.getScale()
|
||||
expect(scale).toBeGreaterThan(1)
|
||||
|
||||
await clickDropdownTrigger(comfyPage)
|
||||
|
||||
const menu = getFloatingMenu(comfyPage)
|
||||
await expect(menu).toBeVisible()
|
||||
|
||||
const trigger = getTriggerButton(comfyPage)
|
||||
const triggerBox = await trigger.boundingBox()
|
||||
const menuBox = await menu.boundingBox()
|
||||
|
||||
expect(triggerBox).not.toBeNull()
|
||||
expect(menuBox).not.toBeNull()
|
||||
|
||||
const gap = menuBox!.y - (triggerBox!.y + triggerBox!.height)
|
||||
expect(gap).toBeGreaterThanOrEqual(-2)
|
||||
expect(gap).toBeLessThanOrEqual(20)
|
||||
})
|
||||
|
||||
test('dropdown closes on Escape key', async ({ comfyPage }) => {
|
||||
await clickDropdownTrigger(comfyPage)
|
||||
|
||||
const menu = getFloatingMenu(comfyPage)
|
||||
await expect(menu).toBeVisible()
|
||||
|
||||
await comfyPage.page.keyboard.press('Escape')
|
||||
await expect(menu).not.toBeVisible()
|
||||
})
|
||||
|
||||
test('dropdown closes on click outside', async ({ comfyPage }) => {
|
||||
await clickDropdownTrigger(comfyPage)
|
||||
|
||||
const menu = getFloatingMenu(comfyPage)
|
||||
await expect(menu).toBeVisible()
|
||||
|
||||
// Click on empty canvas area
|
||||
await comfyPage.canvasOps.clickEmptySpace()
|
||||
await expect(menu).not.toBeVisible()
|
||||
})
|
||||
})
|
||||
@@ -61,6 +61,7 @@
|
||||
"@comfyorg/registry-types": "workspace:*",
|
||||
"@comfyorg/shared-frontend-utils": "workspace:*",
|
||||
"@comfyorg/tailwind-utils": "workspace:*",
|
||||
"@floating-ui/vue": "catalog:",
|
||||
"@formkit/auto-animate": "catalog:",
|
||||
"@iconify/json": "catalog:",
|
||||
"@primeuix/forms": "catalog:",
|
||||
|
||||
852
pnpm-lock.yaml
generated
852
pnpm-lock.yaml
generated
File diff suppressed because it is too large
Load Diff
@@ -8,6 +8,7 @@ catalog:
|
||||
'@astrojs/vue': ^5.0.0
|
||||
'@comfyorg/comfyui-electron-types': 0.6.2
|
||||
'@eslint/js': ^9.39.1
|
||||
'@floating-ui/vue': ^1.1.11
|
||||
'@formkit/auto-animate': ^0.9.0
|
||||
'@iconify-json/lucide': ^1.1.178
|
||||
'@iconify/json': ^2.2.380
|
||||
|
||||
@@ -3,7 +3,6 @@ import { capitalize } from 'es-toolkit'
|
||||
import { computed, provide, ref, shallowRef, toRef, watch } from 'vue'
|
||||
import { useI18n } from 'vue-i18n'
|
||||
|
||||
import { useTransformCompatOverlayProps } from '@/composables/useTransformCompatOverlayProps'
|
||||
import { appendCloudResParam } from '@/platform/distribution/cloudPreviewUtil'
|
||||
import { SUPPORTED_EXTENSIONS_ACCEPT } from '@/extensions/core/load3d/constants'
|
||||
import { useAssetFilterOptions } from '@/platform/assets/composables/useAssetFilterOptions'
|
||||
@@ -75,12 +74,9 @@ const toastStore = useToastStore()
|
||||
|
||||
const outputMediaAssets = useMediaAssets('output')
|
||||
|
||||
const transformCompatProps = useTransformCompatOverlayProps()
|
||||
|
||||
const combinedProps = computed(() => ({
|
||||
...filterWidgetProps(props.widget.options, PANEL_EXCLUDED_PROPS),
|
||||
...transformCompatProps.value
|
||||
}))
|
||||
const combinedProps = computed(() =>
|
||||
filterWidgetProps(props.widget.options, PANEL_EXCLUDED_PROPS)
|
||||
)
|
||||
|
||||
const getAssetData = () => {
|
||||
const nodeType: string | undefined =
|
||||
|
||||
@@ -19,6 +19,19 @@ vi.mock('@/platform/updates/common/toastStore', () => ({
|
||||
})
|
||||
}))
|
||||
|
||||
vi.mock('@floating-ui/vue', () => ({
|
||||
useFloating: vi.fn(() => ({
|
||||
floatingStyles: {
|
||||
value: { position: 'absolute', top: '0px', left: '0px' }
|
||||
},
|
||||
update: vi.fn()
|
||||
})),
|
||||
autoUpdate: vi.fn(() => vi.fn()),
|
||||
offset: vi.fn(() => ({})),
|
||||
flip: vi.fn(() => ({})),
|
||||
shift: vi.fn(() => ({}))
|
||||
}))
|
||||
|
||||
const MockFormDropdownMenu = {
|
||||
name: 'FormDropdownMenu',
|
||||
props: [
|
||||
@@ -40,12 +53,7 @@ const MockFormDropdownMenu = {
|
||||
const MockFormDropdownInput = {
|
||||
name: 'FormDropdownInput',
|
||||
template:
|
||||
'<button class="mock-dropdown-trigger" @click="$emit(\'select-click\', $event)">Open</button>'
|
||||
}
|
||||
|
||||
const MockPopover = {
|
||||
name: 'Popover',
|
||||
template: '<div><slot /></div>'
|
||||
'<button data-testid="mock-trigger" @click="$emit(\'select-click\', $event)">Open</button>'
|
||||
}
|
||||
|
||||
interface MountDropdownOptions {
|
||||
@@ -55,6 +63,7 @@ interface MountDropdownOptions {
|
||||
onCleanup: (cleanupFn: () => void) => void
|
||||
) => Promise<FormDropdownItem[]>
|
||||
searchQuery?: string
|
||||
isOpen?: boolean
|
||||
}
|
||||
|
||||
function flushPromises() {
|
||||
@@ -72,7 +81,6 @@ function mountDropdown(
|
||||
plugins: [PrimeVue, i18n],
|
||||
stubs: {
|
||||
FormDropdownInput: MockFormDropdownInput,
|
||||
Popover: MockPopover,
|
||||
FormDropdownMenu: MockFormDropdownMenu
|
||||
}
|
||||
}
|
||||
@@ -85,18 +93,73 @@ function getMenuItems(): FormDropdownItem[] {
|
||||
return JSON.parse(menuEl.getAttribute('data-items') ?? '[]')
|
||||
}
|
||||
|
||||
async function openDropdown(
|
||||
result: ReturnType<typeof mountDropdown>
|
||||
): Promise<void> {
|
||||
await result.user.click(screen.getByTestId('mock-trigger'))
|
||||
await flushPromises()
|
||||
}
|
||||
|
||||
describe('FormDropdown', () => {
|
||||
describe('open/close behavior', () => {
|
||||
it('opens the dropdown menu when trigger is clicked', async () => {
|
||||
const result = mountDropdown([createItem('1', 'item1')])
|
||||
await flushPromises()
|
||||
|
||||
expect(screen.queryByTestId('dropdown-menu')).toBeNull()
|
||||
|
||||
await openDropdown(result)
|
||||
|
||||
expect(screen.getByTestId('dropdown-menu')).toBeInTheDocument()
|
||||
})
|
||||
|
||||
it('closes the dropdown when trigger is clicked again', async () => {
|
||||
const result = mountDropdown([createItem('1', 'item1')])
|
||||
await flushPromises()
|
||||
|
||||
await openDropdown(result)
|
||||
expect(screen.getByTestId('dropdown-menu')).toBeInTheDocument()
|
||||
|
||||
await openDropdown(result)
|
||||
expect(screen.queryByTestId('dropdown-menu')).toBeNull()
|
||||
})
|
||||
|
||||
it('closes the dropdown on Escape key', async () => {
|
||||
const result = mountDropdown([createItem('1', 'item1')])
|
||||
await flushPromises()
|
||||
|
||||
await openDropdown(result)
|
||||
expect(screen.getByTestId('dropdown-menu')).toBeInTheDocument()
|
||||
|
||||
await result.user.keyboard('{Escape}')
|
||||
await flushPromises()
|
||||
expect(screen.queryByTestId('dropdown-menu')).toBeNull()
|
||||
})
|
||||
})
|
||||
|
||||
describe('floating positioning', () => {
|
||||
it('renders floating menu container when open', async () => {
|
||||
const result = mountDropdown([createItem('1', 'item1')])
|
||||
await flushPromises()
|
||||
|
||||
await openDropdown(result)
|
||||
|
||||
expect(screen.getByTestId('dropdown-menu')).toBeInTheDocument()
|
||||
})
|
||||
})
|
||||
|
||||
describe('filteredItems updates when items prop changes', () => {
|
||||
it('updates displayed items when items prop changes', async () => {
|
||||
const { rerender } = mountDropdown([
|
||||
const result = mountDropdown([
|
||||
createItem('input-0', 'video1.mp4'),
|
||||
createItem('input-1', 'video2.mp4')
|
||||
])
|
||||
await flushPromises()
|
||||
await openDropdown(result)
|
||||
|
||||
expect(getMenuItems()).toHaveLength(2)
|
||||
|
||||
await rerender({
|
||||
await result.rerender({
|
||||
items: [
|
||||
createItem('output-0', 'rendered1.mp4'),
|
||||
createItem('output-1', 'rendered2.mp4')
|
||||
@@ -110,28 +173,30 @@ describe('FormDropdown', () => {
|
||||
})
|
||||
|
||||
it('updates when items change but IDs stay the same', async () => {
|
||||
const { rerender } = mountDropdown([createItem('1', 'alpha')])
|
||||
const result = mountDropdown([createItem('1', 'alpha')])
|
||||
await flushPromises()
|
||||
await openDropdown(result)
|
||||
|
||||
await rerender({ items: [createItem('1', 'beta')] })
|
||||
await result.rerender({ items: [createItem('1', 'beta')] })
|
||||
await flushPromises()
|
||||
|
||||
expect(getMenuItems()[0].name).toBe('beta')
|
||||
})
|
||||
|
||||
it('updates when switching between empty and non-empty items', async () => {
|
||||
const { rerender } = mountDropdown([])
|
||||
const result = mountDropdown([], { isOpen: true })
|
||||
await flushPromises()
|
||||
|
||||
expect(getMenuItems()).toHaveLength(0)
|
||||
|
||||
await rerender({ items: [createItem('1', 'video.mp4')] })
|
||||
await result.rerender({
|
||||
items: [createItem('1', 'video.mp4')],
|
||||
isOpen: true
|
||||
})
|
||||
await flushPromises()
|
||||
|
||||
expect(getMenuItems()).toHaveLength(1)
|
||||
expect(getMenuItems()[0].name).toBe('video.mp4')
|
||||
|
||||
await rerender({ items: [] })
|
||||
await result.rerender({ items: [], isOpen: true })
|
||||
await flushPromises()
|
||||
|
||||
expect(getMenuItems()).toHaveLength(0)
|
||||
@@ -144,7 +209,7 @@ describe('FormDropdown', () => {
|
||||
sourceItems.filter((item) => item.name.includes('video'))
|
||||
)
|
||||
|
||||
const { rerender } = mountDropdown(
|
||||
const result = mountDropdown(
|
||||
[createItem('1', 'video-a.mp4'), createItem('2', 'video-b.mp4')],
|
||||
{ searcher }
|
||||
)
|
||||
@@ -152,12 +217,12 @@ describe('FormDropdown', () => {
|
||||
|
||||
expect(searcher).not.toHaveBeenCalled()
|
||||
|
||||
await rerender({
|
||||
await result.rerender({
|
||||
items: [createItem('1', 'video-a.mp4'), createItem('2', 'video-b.mp4')],
|
||||
searcher,
|
||||
searchQuery: 'video-a'
|
||||
})
|
||||
await rerender({
|
||||
await result.rerender({
|
||||
items: [createItem('3', 'video-c.mp4'), createItem('4', 'video-d.mp4')],
|
||||
searcher,
|
||||
searchQuery: 'video-a'
|
||||
@@ -165,7 +230,6 @@ describe('FormDropdown', () => {
|
||||
await flushPromises()
|
||||
|
||||
expect(searcher).not.toHaveBeenCalled()
|
||||
expect(getMenuItems().map((item) => item.id)).toEqual(['3', '4'])
|
||||
})
|
||||
|
||||
it('runs filtering when dropdown opens', async () => {
|
||||
@@ -174,15 +238,13 @@ describe('FormDropdown', () => {
|
||||
sourceItems.filter((item) => item.id === 'keep')
|
||||
)
|
||||
|
||||
const { container, user } = mountDropdown(
|
||||
const result = mountDropdown(
|
||||
[createItem('keep', 'alpha'), createItem('drop', 'beta')],
|
||||
{ searcher }
|
||||
)
|
||||
await flushPromises()
|
||||
|
||||
// eslint-disable-next-line testing-library/no-node-access
|
||||
await user.click(container.querySelector('.mock-dropdown-trigger')!)
|
||||
await flushPromises()
|
||||
await openDropdown(result)
|
||||
|
||||
expect(searcher).toHaveBeenCalled()
|
||||
expect(getMenuItems().map((item) => item.id)).toEqual(['keep'])
|
||||
|
||||
@@ -1,10 +1,9 @@
|
||||
<script setup lang="ts">
|
||||
import { computedAsync, refDebounced } from '@vueuse/core'
|
||||
import Popover from 'primevue/popover'
|
||||
import { computed, ref, useTemplateRef } from 'vue'
|
||||
import { autoUpdate, flip, offset, shift, useFloating } from '@floating-ui/vue'
|
||||
import { computedAsync, onClickOutside, refDebounced } from '@vueuse/core'
|
||||
import { computed, useTemplateRef } from 'vue'
|
||||
import { useI18n } from 'vue-i18n'
|
||||
|
||||
import { useTransformCompatOverlayProps } from '@/composables/useTransformCompatOverlayProps'
|
||||
import { useToastStore } from '@/platform/updates/common/toastStore'
|
||||
|
||||
import type {
|
||||
@@ -51,7 +50,6 @@ interface Props {
|
||||
}
|
||||
|
||||
const { t } = useI18n()
|
||||
const overlayProps = useTransformCompatOverlayProps()
|
||||
|
||||
const {
|
||||
placeholder,
|
||||
@@ -95,8 +93,18 @@ const baseModelSelected = defineModel<Set<string>>('baseModelSelected', {
|
||||
const isOpen = defineModel<boolean>('isOpen', { default: false })
|
||||
|
||||
const toastStore = useToastStore()
|
||||
const popoverRef = ref<InstanceType<typeof Popover>>()
|
||||
const triggerRef = useTemplateRef('triggerRef')
|
||||
const floatingRef = useTemplateRef('floatingRef')
|
||||
|
||||
const { floatingStyles } = useFloating(triggerRef, floatingRef, {
|
||||
placement: 'bottom-start',
|
||||
whileElementsMounted: autoUpdate,
|
||||
middleware: [offset(8), flip(), shift({ padding: 8 })]
|
||||
})
|
||||
|
||||
onClickOutside(floatingRef, () => {
|
||||
if (isOpen.value) closeDropdown()
|
||||
}, { ignore: [triggerRef] })
|
||||
|
||||
const maxSelectable = computed(() => {
|
||||
if (multiple === true) return Infinity
|
||||
@@ -142,18 +150,19 @@ function internalIsSelected(item: FormDropdownItem, index: number): boolean {
|
||||
return isSelected(selected.value, item, index)
|
||||
}
|
||||
|
||||
const toggleDropdown = (event: Event) => {
|
||||
function toggleDropdown(event: Event) {
|
||||
if (disabled) return
|
||||
if (popoverRef.value && triggerRef.value) {
|
||||
popoverRef.value.toggle?.(event, triggerRef.value)
|
||||
isOpen.value = !isOpen.value
|
||||
}
|
||||
void event
|
||||
isOpen.value = !isOpen.value
|
||||
}
|
||||
|
||||
const closeDropdown = () => {
|
||||
if (popoverRef.value) {
|
||||
popoverRef.value.hide?.()
|
||||
isOpen.value = false
|
||||
function closeDropdown() {
|
||||
isOpen.value = false
|
||||
}
|
||||
|
||||
function handleKeydown(event: KeyboardEvent) {
|
||||
if (event.key === 'Escape' && isOpen.value) {
|
||||
closeDropdown()
|
||||
}
|
||||
}
|
||||
|
||||
@@ -192,36 +201,29 @@ function handleSelection(item: FormDropdownItem, index: number) {
|
||||
</script>
|
||||
|
||||
<template>
|
||||
<div ref="triggerRef">
|
||||
<FormDropdownInput
|
||||
:files
|
||||
:is-open
|
||||
:placeholder="placeholderText"
|
||||
:items
|
||||
:display-items
|
||||
:max-selectable
|
||||
:selected
|
||||
:uploadable
|
||||
:disabled
|
||||
:accept
|
||||
@select-click="toggleDropdown"
|
||||
@file-change="handleFileChange"
|
||||
/>
|
||||
<Popover
|
||||
ref="popoverRef"
|
||||
:dismissable="true"
|
||||
:close-on-escape="true"
|
||||
:append-to="overlayProps.appendTo"
|
||||
unstyled
|
||||
:pt="{
|
||||
root: {
|
||||
class: 'absolute z-50'
|
||||
},
|
||||
content: {
|
||||
class: ['bg-transparent border-none p-0 pt-2 rounded-lg shadow-lg']
|
||||
}
|
||||
}"
|
||||
@hide="isOpen = false"
|
||||
<div @keydown="handleKeydown">
|
||||
<div ref="triggerRef">
|
||||
<FormDropdownInput
|
||||
:files
|
||||
:is-open
|
||||
:placeholder="placeholderText"
|
||||
:items
|
||||
:display-items
|
||||
:max-selectable
|
||||
:selected
|
||||
:uploadable
|
||||
:disabled
|
||||
:accept
|
||||
@select-click="toggleDropdown"
|
||||
@file-change="handleFileChange"
|
||||
/>
|
||||
</div>
|
||||
<div
|
||||
v-if="isOpen"
|
||||
ref="floatingRef"
|
||||
data-floating-menu
|
||||
:style="floatingStyles"
|
||||
class="z-50"
|
||||
>
|
||||
<FormDropdownMenu
|
||||
v-model:filter-selected="filterSelected"
|
||||
@@ -243,6 +245,6 @@ function handleSelection(item: FormDropdownItem, index: number) {
|
||||
@close="closeDropdown"
|
||||
@item-click="handleSelection"
|
||||
/>
|
||||
</Popover>
|
||||
</div>
|
||||
</div>
|
||||
</template>
|
||||
|
||||
Reference in New Issue
Block a user