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
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
136 changes: 136 additions & 0 deletions browser_tests/tests/vueNodes/interactions/node/bringToFront.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,136 @@
import {
comfyExpect as expect,
comfyPageFixture as test
} from '../../../../fixtures/ComfyPage'
import { fitToViewInstant } from '../../../../helpers/fitToView'

test.describe('Vue Node Bring to Front', () => {
test.beforeEach(async ({ comfyPage }) => {
await comfyPage.setSetting('Comfy.UseNewMenu', 'Disabled')
await comfyPage.setSetting('Comfy.VueNodes.Enabled', true)
await comfyPage.loadWorkflow('vueNodes/simple-triple')
await comfyPage.vueNodes.waitForNodes()
await fitToViewInstant(comfyPage)
})

/**
* Helper to get the z-index of a node by its title
*/
async function getNodeZIndex(
comfyPage: Awaited<
ReturnType<(typeof test)['__dirtyFixtureLookup']>
>['comfyPage'],
title: string
): Promise<number> {
const node = comfyPage.vueNodes.getNodeByTitle(title)
const style = await node.getAttribute('style')
const match = style?.match(/z-index:\s*(\d+)/)
return match ? parseInt(match[1], 10) : 0
}
Comment on lines +19 to +29
Copy link
Contributor

Choose a reason for hiding this comment

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

🧹 Nitpick | 🔵 Trivial

Consider extracting a reusable comfyPage fixture type alias

Both helpers inline the same relatively complex comfyPage type using Awaited<ReturnType<(typeof test)['__dirtyFixtureLookup']>['comfyPage']>, which hurts readability and is easy to get out of sync if the fixture shape changes. If there isn’t already an exported type from fixtures/ComfyPage, consider introducing a local alias and reusing it across helpers:

type ComfyPage = Awaited<
  ReturnType<(typeof test)['__dirtyFixtureLookup']>
>['comfyPage']

async function getNodeZIndex(comfyPage: ComfyPage, title: string) { ... }

async function getNodeCenter(comfyPage: ComfyPage, title: string) { ... }

This keeps the helpers clearer and reduces duplication while still avoiding any.

Also applies to: 34-44

🤖 Prompt for AI Agents
In browser_tests/tests/vueNodes/interactions/node/bringToFront.spec.ts around
lines 19-29 (and similarly 34-44), the comfyPage fixture type is repeated
verbosely; create a local type alias at the top of the file (e.g., "type
ComfyPage = Awaited<ReturnType<(typeof
test)['__dirtyFixtureLookup']>>['comfyPage']") and then replace the long inline
type in both helper signatures (getNodeZIndex and getNodeCenter) with the new
ComfyPage alias to improve readability and avoid duplication.


/**
* Helper to get the bounding box center of a node
*/
async function getNodeCenter(
comfyPage: Awaited<
ReturnType<(typeof test)['__dirtyFixtureLookup']>
>['comfyPage'],
title: string
): Promise<{ x: number; y: number }> {
const node = comfyPage.vueNodes.getNodeByTitle(title)
const box = await node.boundingBox()
if (!box) throw new Error(`Node "${title}" not found`)
return { x: box.x + box.width / 2, y: box.y + box.height / 2 }
}

test('should bring overlapped node to front when clicking on it', async ({
comfyPage
}) => {
// Get initial positions
const clipCenter = await getNodeCenter(comfyPage, 'CLIP Text Encode')
const ksamplerHeader = await comfyPage.page
.getByText('KSampler')
.boundingBox()
if (!ksamplerHeader) throw new Error('KSampler header not found')

// Drag KSampler on top of CLIP Text Encode
await comfyPage.dragAndDrop(
{ x: ksamplerHeader.x + 50, y: ksamplerHeader.y + 10 },
clipCenter
)
Comment on lines +49 to +60
Copy link
Contributor

Choose a reason for hiding this comment

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

🧹 Nitpick | 🔵 Trivial

Reduce brittleness from magic offsets and align comments with actual click position

The overall flow of this test looks good and matches the header-click bring-to-front behavior, but a couple of details could be tightened:

  • The comment in Line 79 says “bottom-left corner” while the click uses clipBox.y + 10, which is near the top-left, not bottom-left. This can be misleading for future maintainers.
  • Both the drag (ksamplerHeader.x + 50, ksamplerHeader.y + 10) and the click (clipBox.x + 30, clipBox.y + 10) rely on hard-coded offsets that assume specific node sizes and layout. That increases the risk of flakiness across different viewports or style tweaks (e.g. if KSampler ends up fully covering that region of CLIP, the click might hit KSampler again instead of CLIP).

Suggested improvements:

  • Update the comment to describe the actual behavior (e.g. “a visible edge of CLIP” rather than “bottom-left corner”).
  • Where feasible, derive the click point from both CLIP and KSampler bounding boxes to deliberately pick a coordinate that’s inside CLIP’s box but outside KSampler’s box, instead of relying on fixed offsets. That would better future-proof the test against layout changes while keeping the scenario identical.

If the current offsets are based on manual inspection and you’re confident they remain stable across your configured projects/viewports, this can stay as-is, but it’s worth considering for long-term stability.

Also applies to: 73-81

🤖 Prompt for AI Agents
In browser_tests/tests/vueNodes/interactions/node/bringToFront.spec.ts around
lines 49-60 and 73-81, update the misleading comment that says “bottom-left
corner” to reflect the actual click target (e.g. “a visible edge of CLIP” or “an
inside point of CLIP visible and not covered by KSampler”), and replace the
hard-coded offset coordinates used for dragging/clicking with computed
coordinates derived from both CLIP and KSampler bounding boxes: calculate a
point guaranteed to be inside CLIP’s boundingBox and outside KSampler’s
boundingBox (for example pick CLIP center or an inner offset and if that point
intersects KSampler, shift it toward CLIP’s interior until it does not), then
use those computed coordinates for dragAndDrop and click to reduce flakiness.

await comfyPage.nextFrame()

// Screenshot showing KSampler on top of CLIP
await expect(comfyPage.canvas).toHaveScreenshot(
'bring-to-front-overlapped-before.png'
)

// KSampler should be on top (higher z-index) after being dragged
const ksamplerZIndexBefore = await getNodeZIndex(comfyPage, 'KSampler')
const clipZIndexBefore = await getNodeZIndex(comfyPage, 'CLIP Text Encode')
expect(ksamplerZIndexBefore).toBeGreaterThan(clipZIndexBefore)

// Click on CLIP Text Encode (underneath) - need to click on a visible part
// Since KSampler is on top, we click on the edge of CLIP that should still be visible
const clipNode = comfyPage.vueNodes.getNodeByTitle('CLIP Text Encode')
const clipBox = await clipNode.boundingBox()
if (!clipBox) throw new Error('CLIP node not found')

// Click on the bottom-left corner of CLIP which should be visible
await comfyPage.page.mouse.click(clipBox.x + 30, clipBox.y + 10)
await comfyPage.nextFrame()

// CLIP should now be on top
const clipZIndexAfter = await getNodeZIndex(comfyPage, 'CLIP Text Encode')
expect(clipZIndexAfter).toBeGreaterThan(ksamplerZIndexBefore)

// Screenshot showing CLIP now on top
await expect(comfyPage.canvas).toHaveScreenshot(
'bring-to-front-overlapped-after.png'
)
})

test('should bring overlapped node to front when clicking on its widget', async ({
comfyPage
}) => {
// Get CLIP Text Encode position (it has a text widget)
const clipCenter = await getNodeCenter(comfyPage, 'CLIP Text Encode')

// Get VAE Decode position and drag it on top of CLIP
const vaeHeader = await comfyPage.page.getByText('VAE Decode').boundingBox()
if (!vaeHeader) throw new Error('VAE Decode header not found')

await comfyPage.dragAndDrop(
{ x: vaeHeader.x + 50, y: vaeHeader.y + 10 },
{ x: clipCenter.x - 50, y: clipCenter.y }
)
await comfyPage.nextFrame()

// VAE should be on top after drag
const vaeZIndexBefore = await getNodeZIndex(comfyPage, 'VAE Decode')
const clipZIndexBefore = await getNodeZIndex(comfyPage, 'CLIP Text Encode')
expect(vaeZIndexBefore).toBeGreaterThan(clipZIndexBefore)

// Screenshot showing VAE on top
await expect(comfyPage.canvas).toHaveScreenshot(
'bring-to-front-widget-overlapped-before.png'
)

// Click on the text widget of CLIP Text Encode
const clipNode = comfyPage.vueNodes.getNodeByTitle('CLIP Text Encode')
const clipBox = await clipNode.boundingBox()
const textWidget = clipNode.locator('textarea').first()
if (!clipBox) throw new Error('CLIP node not found')
await comfyPage.page.mouse.click(clipBox.x + 170, clipBox.y + 80)
await comfyPage.nextFrame()

// CLIP should now be on top
const clipZIndexAfter = await getNodeZIndex(comfyPage, 'CLIP Text Encode')
expect(clipZIndexAfter).toBeGreaterThan(vaeZIndexBefore)

// Screenshot showing CLIP now on top after widget click
await expect(comfyPage.canvas).toHaveScreenshot(
'bring-to-front-widget-overlapped-after.png'
)
})
})
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
10 changes: 10 additions & 0 deletions src/renderer/extensions/vueNodes/components/NodeWidgets.vue
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
:style="{
'grid-template-rows': gridTemplateRows
}"
@pointerdown.capture="handleBringToFront"
@pointerdown="handleWidgetPointerEvent"
@pointermove="handleWidgetPointerEvent"
@pointerup="handleWidgetPointerEvent"
Expand Down Expand Up @@ -78,6 +79,7 @@ import { useErrorHandling } from '@/composables/useErrorHandling'
import { st } from '@/i18n'
import { useCanvasInteractions } from '@/renderer/core/canvas/useCanvasInteractions'
import { useNodeTooltips } from '@/renderer/extensions/vueNodes/composables/useNodeTooltips'
import { useNodeZIndex } from '@/renderer/extensions/vueNodes/composables/useNodeZIndex'
import WidgetDOM from '@/renderer/extensions/vueNodes/widgets/components/WidgetDOM.vue'
// Import widget components directly
import WidgetLegacy from '@/renderer/extensions/vueNodes/widgets/components/WidgetLegacy.vue'
Expand All @@ -99,12 +101,20 @@ const { nodeData } = defineProps<NodeWidgetsProps>()

const { shouldHandleNodePointerEvents, forwardEventToCanvas } =
useCanvasInteractions()
const { bringNodeToFront } = useNodeZIndex()

function handleWidgetPointerEvent(event: PointerEvent) {
if (shouldHandleNodePointerEvents.value) return
event.stopPropagation()
forwardEventToCanvas(event)
}

function handleBringToFront() {
if (nodeData?.id != null) {
bringNodeToFront(String(nodeData.id))
}
}

// Error boundary implementation
const renderError = ref<string | null>(null)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -134,13 +134,21 @@ function useNodeEventHandlersIndividual() {
canvasStore.canvas.deselectAll()
canvasStore.canvas.select(node)
canvasStore.updateSelectedItems()
// Bring node to front when selected (unless pinned)
if (!node.flags?.pinned) {
bringNodeToFront(nodeId)
}
return
}

if (node.selected) {
canvasStore.canvas.deselect(node)
} else {
canvasStore.canvas.select(node)
// Bring node to front when selected (unless pinned)
if (!node.flags?.pinned) {
bringNodeToFront(nodeId)
}
}

canvasStore.updateSelectedItems()
Expand Down