Skip to content

Commit 60f98bd

Browse files
committed
fix: address promoted widget review feedback
- keep selected advanced widgets visible in app mode - pin promoted slot metadata at its producer boundary - simplify and clarify promoted visibility coverage
1 parent faf8230 commit 60f98bd

4 files changed

Lines changed: 176 additions & 61 deletions

File tree

Lines changed: 154 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,154 @@
1+
import { createTestingPinia } from '@pinia/testing'
2+
import { render, screen } from '@testing-library/vue'
3+
import { fromAny } from '@total-typescript/shoehorn'
4+
import { setActivePinia } from 'pinia'
5+
import { createI18n } from 'vue-i18n'
6+
import { beforeEach, describe, expect, it, vi } from 'vitest'
7+
8+
import type { VueNodeData } from '@/composables/graph/useGraphNodeManager'
9+
import type { LGraphNode } from '@/lib/litegraph/src/LGraphNode'
10+
import { LGraphEventMode } from '@/lib/litegraph/src/types/globalEnums'
11+
import type { IBaseWidget } from '@/lib/litegraph/src/types/widgets'
12+
import { toNodeId } from '@/types/nodeId'
13+
import type { WidgetId } from '@/types/widgetId'
14+
15+
import AppModeWidgetList from './AppModeWidgetList.vue'
16+
17+
const mocks = vi.hoisted(() => ({
18+
extractVueNodeData: vi.fn(),
19+
resolvedInputs: { value: [] as unknown[] }
20+
}))
21+
22+
vi.mock('@/components/builder/useResolvedSelectedInputs', () => ({
23+
useResolvedSelectedInputs: () => mocks.resolvedInputs
24+
}))
25+
26+
vi.mock('@/composables/graph/useGraphNodeManager', () => ({
27+
extractVueNodeData: mocks.extractVueNodeData
28+
}))
29+
30+
vi.mock('@/composables/maskeditor/useMaskEditor', () => ({
31+
useMaskEditor: () => ({ openMaskEditor: vi.fn() })
32+
}))
33+
34+
vi.mock('@/renderer/core/canvas/canvasStore', () => ({
35+
useCanvasStore: () => ({
36+
canvas: { graph: { rootGraph: { id: 'graph-test' } } }
37+
})
38+
}))
39+
40+
vi.mock(
41+
'@/renderer/extensions/vueNodes/composables/useNodeEventHandlers',
42+
() => ({
43+
useNodeEventHandlers: () => ({ handleNodeRightClick: vi.fn() })
44+
})
45+
)
46+
47+
vi.mock('@/renderer/extensions/vueNodes/composables/useNodeTooltips', () => ({
48+
useNodeTooltips: () => ({
49+
createTooltipConfig: () => ({}),
50+
getWidgetTooltip: () => ''
51+
})
52+
}))
53+
54+
vi.mock(
55+
'@/renderer/extensions/vueNodes/widgets/registry/widgetRegistry',
56+
() => ({
57+
getComponent: () => ({
58+
props: ['widget'],
59+
template: '<div data-testid="advanced-widget-control" />'
60+
}),
61+
shouldExpand: () => false,
62+
shouldRenderAsVue: () => true
63+
})
64+
)
65+
66+
vi.mock('@/scripts/app', () => ({
67+
app: {
68+
isGraphReady: false,
69+
rootGraph: { id: 'graph-test' }
70+
}
71+
}))
72+
73+
const i18n = createI18n({
74+
legacy: false,
75+
locale: 'en',
76+
messages: {
77+
en: {
78+
g: { remove: 'Remove', rename: 'Rename' }
79+
}
80+
}
81+
})
82+
83+
describe('AppModeWidgetList', () => {
84+
beforeEach(() => {
85+
const widgetId = 'graph-test:1:max_shift' as WidgetId
86+
const widget = fromAny<IBaseWidget, unknown>({
87+
label: 'Max shift',
88+
name: 'max_shift',
89+
widgetId
90+
})
91+
const node = fromAny<LGraphNode, unknown>({
92+
id: toNodeId(1),
93+
mode: LGraphEventMode.ALWAYS,
94+
title: 'Subgraph',
95+
type: 'SubgraphNode'
96+
})
97+
const nodeData: VueNodeData = {
98+
executing: false,
99+
id: toNodeId(1),
100+
inputs: [],
101+
mode: LGraphEventMode.ALWAYS,
102+
outputs: [],
103+
selected: false,
104+
title: 'Subgraph',
105+
type: 'SubgraphNode',
106+
widgets: [
107+
{
108+
name: 'max_shift',
109+
options: { advanced: true },
110+
slotMetadata: {
111+
index: 0,
112+
linked: false,
113+
promoted: true,
114+
type: 'FLOAT'
115+
},
116+
type: 'number',
117+
widgetId
118+
}
119+
]
120+
}
121+
122+
mocks.resolvedInputs.value = [
123+
{
124+
displayName: 'max_shift',
125+
node,
126+
status: 'resolved',
127+
widget,
128+
widgetId
129+
}
130+
]
131+
mocks.extractVueNodeData.mockReturnValue(nodeData)
132+
})
133+
134+
it('renders a selected promoted advanced widget', () => {
135+
const pinia = createTestingPinia({ stubActions: false })
136+
setActivePinia(pinia)
137+
138+
render(AppModeWidgetList, {
139+
global: {
140+
directives: { tooltip: { mounted: () => {} } },
141+
plugins: [pinia, i18n],
142+
stubs: {
143+
Button: { template: '<button><slot /></button>' },
144+
DropZone: { template: '<div><slot /></div>' },
145+
InputSlot: true,
146+
Popover: { template: '<div><slot name="button" /></div>' },
147+
WidgetDescription: true
148+
}
149+
}
150+
})
151+
152+
expect(screen.getByTestId('advanced-widget-control')).toBeVisible()
153+
})
154+
})

src/components/builder/AppModeWidgetList.vue

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -87,6 +87,7 @@ const mappedSelections = computed((): WidgetEntry[] => {
8787
description: config?.description,
8888
nodeData: {
8989
...fullNodeData,
90+
showAdvanced: true,
9091
widgets: [matchingWidget]
9192
},
9293
action: { widget, node }

src/composables/graph/useGraphNodeManager.test.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -112,7 +112,7 @@ describe('Widget slotMetadata reactivity on link disconnect', () => {
112112
return { graph, node, upstream, linkId: link.id }
113113
}
114114

115-
it('sets slotMetadata.linked to true when input has a link', () => {
115+
it('identifies a linked regular widget input as unpromoted', () => {
116116
const { graph, node } = createWidgetInputGraph()
117117
const { vueNodeData } = useGraphNodeManager(graph)
118118

@@ -121,6 +121,7 @@ describe('Widget slotMetadata reactivity on link disconnect', () => {
121121

122122
expect(widgetData?.slotMetadata).toBeDefined()
123123
expect(widgetData?.slotMetadata?.linked).toBe(true)
124+
expect(widgetData?.slotMetadata?.promoted).toBe(false)
124125
})
125126

126127
it('updates slotMetadata.linked to false after link disconnect event', async () => {
@@ -236,6 +237,7 @@ describe('Widget slotMetadata reactivity on link disconnect', () => {
236237
expect(widgetData).toBeDefined()
237238
expect(widgetData?.sourceWidgetName).toBe('prompt')
238239
expect(widgetData?.slotMetadata).toBeDefined()
240+
expect(widgetData?.slotMetadata?.promoted).toBe(true)
239241
})
240242

241243
it('clears stale slotMetadata when input no longer matches widget', async () => {

src/renderer/extensions/vueNodes/composables/useProcessedWidgets.test.ts

Lines changed: 18 additions & 60 deletions
Original file line numberDiff line numberDiff line change
@@ -4,15 +4,9 @@ import { setActivePinia } from 'pinia'
44
import { fromAny } from '@total-typescript/shoehorn'
55
import { beforeEach, describe, expect, it, vi } from 'vitest'
66

7-
import { LGraphNode } from '@/lib/litegraph/src/litegraph'
87
import type { LGraph } from '@/lib/litegraph/src/litegraph'
9-
import {
10-
createTestSubgraph,
11-
createTestSubgraphNode
12-
} from '@/lib/litegraph/src/subgraph/__fixtures__/subgraphHelpers'
138
import { useMissingMediaStore } from '@/platform/missingMedia/missingMediaStore'
149
import { useMissingModelStore } from '@/platform/missingModel/missingModelStore'
15-
import { app } from '@/scripts/app'
1610
import { useExecutionErrorStore } from '@/stores/executionErrorStore'
1711
import { useWidgetValueStore } from '@/stores/widgetValueStore'
1812
import {
@@ -24,8 +18,6 @@ import { widgetId } from '@/types/widgetId'
2418
import type * as GraphTraversalUtil from '@/utils/graphTraversalUtil'
2519

2620
import type { SafeWidgetData } from '@/composables/graph/useGraphNodeManager'
27-
import { extractVueNodeData } from '@/composables/graph/useGraphNodeManager'
28-
import { promoteValueWidgetViaSubgraphInput } from '@/core/graph/subgraph/promotionUtils'
2921
import {
3022
computeProcessedWidgets,
3123
getWidgetIdentity,
@@ -173,11 +165,11 @@ describe('isWidgetVisible', () => {
173165
expect(isWidgetVisible({ advanced: true }, true)).toBe(true)
174166
})
175167

176-
it('keeps advanced widgets visible when linked and showAdvanced is false', () => {
168+
it('keeps advanced widgets visible when advanced filtering is ignored', () => {
177169
expect(isWidgetVisible({ advanced: true }, false, true)).toBe(true)
178170
})
179171

180-
it('keeps hidden widgets hidden when linked', () => {
172+
it('keeps hidden widgets hidden when advanced filtering is ignored', () => {
181173
expect(isWidgetVisible({ hidden: true }, false, true)).toBe(false)
182174
})
183175
})
@@ -422,58 +414,24 @@ describe('computeProcessedWidgets visibility', () => {
422414
})
423415

424416
it('keeps a promoted advanced widget visible without source metadata', () => {
425-
const subgraph = createTestSubgraph()
426-
const subgraphNode = createTestSubgraphNode(subgraph)
427-
subgraph.rootGraph.add(subgraphNode)
428-
429-
const interiorNode = new LGraphNode('ModelSamplingFlux')
430-
const interiorInput = interiorNode.addInput('max_shift', 'FLOAT')
431-
const interiorWidget = interiorNode.addWidget(
432-
'number',
433-
'max_shift',
434-
1,
435-
() => {},
436-
{ advanced: true }
437-
)
438-
interiorInput.widget = { name: interiorWidget.name }
439-
subgraph.add(interiorNode)
417+
const widget = createMockWidget({
418+
name: 'max_shift',
419+
type: 'number',
420+
options: { advanced: true },
421+
slotMetadata: {
422+
index: 0,
423+
linked: false,
424+
promoted: true,
425+
type: 'FLOAT'
426+
},
427+
sourceExecutionId: undefined,
428+
sourceWidgetName: undefined
429+
})
440430

441-
const promotion = promoteValueWidgetViaSubgraphInput(
442-
subgraphNode,
443-
interiorNode,
444-
interiorWidget
445-
)
446-
expect(promotion.ok).toBe(true)
447-
448-
const rootGraphSpy = vi
449-
.spyOn(app, 'rootGraph', 'get')
450-
.mockReturnValue(subgraph.rootGraph)
451-
try {
452-
const nodeData = extractVueNodeData(subgraphNode)
453-
const processedWidgets = computeProcessedWidgets({
454-
nodeData: {
455-
...nodeData,
456-
widgets: nodeData.widgets?.map((widget) => ({
457-
...widget,
458-
sourceExecutionId: undefined,
459-
sourceWidgetName: undefined
460-
}))
461-
},
462-
graphId: subgraph.rootGraph.id,
463-
showAdvanced: false,
464-
isGraphReady: false,
465-
rootGraph: null,
466-
ui: noopUi
467-
})
468-
const promotedWidget = processedWidgets.find(
469-
(widget) => widget.name === 'max_shift'
470-
)
431+
const promotedWidget = processWidgets([widget])[0]
471432

472-
expect(promotedWidget).toBeDefined()
473-
expect(promotedWidget?.visible).toBe(true)
474-
} finally {
475-
rootGraphSpy.mockRestore()
476-
}
433+
expect(promotedWidget).toBeDefined()
434+
expect(promotedWidget?.visible).toBe(true)
477435
})
478436
})
479437

0 commit comments

Comments
 (0)