Cleanup: YAGNI readonly props, private swap on ComfyApp, Canvas resize events simplification, v-memos on individual instances (#5869)

## Summary

Assorted cleanup opportunities found while working through the Vue node
rendering logic cleanup.

## Review Focus

Am I wrong that the readonly logic was never actually executing because
it was defined as False in GraphCanvas when making each LGraphNode?

Is there an edge case or some other reason that the ResizeObserver
wouldn't work as a single signal to resize the canvas?

┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-5869-Cleanup-YAGNI-readonly-props-private-swap-on-ComfyApp-Canvas-resize-events-simplificat-27e6d73d3650811ba1dcf29e8d43091e)
by [Unito](https://www.unito.io)

---------

Co-authored-by: github-actions <github-actions@github.com>
This commit is contained in:
Alexander Brown
2025-10-02 10:35:10 -07:00
committed by GitHub
parent 706ff953de
commit 37fab21daf
35 changed files with 88 additions and 247 deletions

View File

@@ -6,7 +6,7 @@
ref="connectionDotRef"
:color="slotColor"
:class="cn('-translate-x-1/2', errorClassesDot)"
v-on="readonly ? {} : { pointerdown: onPointerDown }"
@pointerdown="onPointerDown"
/>
<!-- Slot Name -->
@@ -54,7 +54,6 @@ interface InputSlotProps {
index: number
connected?: boolean
compatible?: boolean
readonly?: boolean
dotOnly?: boolean
}
@@ -117,7 +116,7 @@ const slotColor = computed(() => {
const slotWrapperClass = computed(() =>
cn(
'lg-slot lg-slot--input flex items-center group rounded-r-lg h-6',
props.readonly ? 'cursor-default opacity-70' : 'cursor-crosshair',
'cursor-crosshair',
props.dotOnly
? 'lg-slot--dot-only'
: 'pr-6 hover:bg-black/5 hover:dark:bg-white/5',
@@ -148,7 +147,6 @@ useSlotElementTracking({
const { onPointerDown } = useSlotLinkInteraction({
nodeId: props.nodeId ?? '',
index: props.index,
type: 'input',
readonly: props.readonly
type: 'input'
})
</script>

View File

@@ -50,15 +50,7 @@
<SlotConnectionDot multi class="absolute right-0 translate-x-1/2" />
</template>
<NodeHeader
v-memo="[
nodeData.title,
nodeData.color,
nodeData.bgcolor,
isCollapsed,
nodeData.flags?.pinned
]"
:node-data="nodeData"
:readonly="readonly"
:collapsed="isCollapsed"
@collapse="handleCollapse"
@update:title="handleHeaderTitleUpdate"
@@ -100,37 +92,19 @@
:data-testid="`node-body-${nodeData.id}`"
>
<!-- Slots only rendered at full detail -->
<NodeSlots
v-memo="[
nodeData.inputs?.length,
nodeData.outputs?.length,
executionStore.lastNodeErrors
]"
:node-data="nodeData"
:readonly="readonly"
/>
<NodeSlots :node-data="nodeData" />
<!-- Widgets rendered at reduced+ detail -->
<NodeWidgets
v-if="nodeData.widgets?.length"
v-memo="[nodeData.widgets?.length]"
:node-data="nodeData"
:readonly="readonly"
/>
<NodeWidgets v-if="nodeData.widgets?.length" :node-data="nodeData" />
<!-- Custom content at reduced+ detail -->
<NodeContent
v-if="hasCustomContent"
:node-data="nodeData"
:readonly="readonly"
:image-urls="nodeImageUrls"
/>
<!-- Live preview image -->
<div
v-if="shouldShowPreviewImg"
v-memo="[latestPreviewUrl]"
class="px-4"
>
<div v-if="shouldShowPreviewImg" class="px-4">
<img
:src="latestPreviewUrl"
alt="preview"
@@ -180,16 +154,11 @@ import SlotConnectionDot from './SlotConnectionDot.vue'
// Extended props for main node component
interface LGraphNodeProps {
nodeData: VueNodeData
readonly?: boolean
error?: string | null
zoomLevel?: number
}
const {
nodeData,
error = null,
readonly = false
} = defineProps<LGraphNodeProps>()
const { nodeData, error = null } = defineProps<LGraphNodeProps>()
const {
handleNodeCollapse,

View File

@@ -27,7 +27,6 @@ import ImagePreview from './ImagePreview.vue'
interface NodeContentProps {
node?: LGraphNode // For backwards compatibility
nodeData?: VueNodeData // New clean data structure
readonly?: boolean
imageUrls?: string[]
}

View File

@@ -118,7 +118,6 @@ const mountHeader = (
...config,
props: {
nodeData: makeNodeData(),
readonly: false,
collapsed: false,
...props
}
@@ -182,20 +181,6 @@ describe('NodeHeader.vue', () => {
expect(wrapper.get('[data-testid="node-title"]').text()).toContain('KeepMe')
})
it('honors readonly: hides collapse button and prevents editing', async () => {
const wrapper = mountHeader({ readonly: true })
// Collapse button should be hidden
const btn = wrapper.find('[data-testid="node-collapse-button"]')
expect(btn.exists()).toBe(true)
// v-show hides via display:none
expect((btn.element as HTMLButtonElement).style.display).toBe('none')
// In unit test, presence is fine; simulate double click should not create input
await wrapper.get('[data-testid="node-header-1"]').trigger('dblclick')
const input = wrapper.find('[data-testid="node-title-input"]')
expect(input.exists()).toBe(false)
})
it('renders correct chevron icon based on collapsed prop', async () => {
const wrapper = mountHeader({ collapsed: false })
const expandedIcon = wrapper.get('i')
@@ -222,16 +207,6 @@ describe('NodeHeader.vue', () => {
expect(directive).toBeTruthy()
})
it('disables tooltip when in readonly mode', () => {
const wrapper = mountHeader({
readonly: true,
nodeData: makeNodeData({ type: 'KSampler' })
})
const titleElement = wrapper.find('[data-testid="node-title"]')
expect(titleElement.exists()).toBe(true)
})
it('disables tooltip when editing is active', async () => {
const wrapper = mountHeader({
nodeData: makeNodeData({ type: 'KSampler' })

View File

@@ -12,7 +12,6 @@
<div class="flex items-center justify-between gap-2.5 relative">
<!-- Collapse/Expand Button -->
<button
v-show="!readonly"
class="bg-transparent border-transparent flex items-center lod-toggle"
data-testid="node-collapse-button"
@click.stop="handleCollapse"
@@ -43,7 +42,7 @@
data-testid="node-pin-indicator"
/>
</div>
<div v-if="!readonly" class="flex items-center lod-toggle shrink-0">
<div class="flex items-center lod-toggle shrink-0">
<IconButton
v-if="isSubgraphNode"
size="sm"
@@ -85,11 +84,10 @@ import LODFallback from './LODFallback.vue'
interface NodeHeaderProps {
nodeData?: VueNodeData
readonly?: boolean
collapsed?: boolean
}
const { nodeData, readonly, collapsed } = defineProps<NodeHeaderProps>()
const { nodeData, collapsed } = defineProps<NodeHeaderProps>()
const emit = defineEmits<{
collapse: []
@@ -118,7 +116,7 @@ const { getNodeDescription, createTooltipConfig } = useNodeTooltips(
)
const tooltipConfig = computed(() => {
if (readonly || isEditing.value) {
if (isEditing.value) {
return { value: '', disabled: true }
}
const description = getNodeDescription.value
@@ -189,9 +187,7 @@ const handleCollapse = () => {
}
const handleDoubleClick = () => {
if (!readonly) {
isEditing.value = true
}
isEditing.value = true
}
const handleTitleEdit = (newTitle: string) => {

View File

@@ -11,7 +11,6 @@
:node-type="nodeData?.type || ''"
:node-id="nodeData?.id != null ? String(nodeData.id) : ''"
:index="getActualInputIndex(input, index)"
:readonly="readonly"
/>
</div>
@@ -23,7 +22,6 @@
:node-type="nodeData?.type || ''"
:node-id="nodeData?.id != null ? String(nodeData.id) : ''"
:index="index"
:readonly="readonly"
/>
</div>
</div>
@@ -42,10 +40,9 @@ import OutputSlot from './OutputSlot.vue'
interface NodeSlotsProps {
nodeData?: VueNodeData
readonly?: boolean
}
const { nodeData = null, readonly } = defineProps<NodeSlotsProps>()
const { nodeData = null } = defineProps<NodeSlotsProps>()
// Filter out input slots that have corresponding widgets
const filteredInputs = computed(() => {

View File

@@ -34,7 +34,6 @@
}"
:node-id="nodeData?.id != null ? String(nodeData.id) : ''"
:index="getWidgetInputIndex(widget)"
:readonly="readonly"
:dot-only="true"
/>
</div>
@@ -44,7 +43,6 @@
v-tooltip.left="widget.tooltipConfig"
:widget="widget.simplified"
:model-value="widget.value"
:readonly="readonly"
:node-id="nodeData?.id != null ? String(nodeData.id) : ''"
class="flex-1"
@update:model-value="widget.updateHandler"
@@ -76,10 +74,9 @@ import InputSlot from './InputSlot.vue'
interface NodeWidgetsProps {
nodeData?: VueNodeData
readonly?: boolean
}
const { nodeData, readonly } = defineProps<NodeWidgetsProps>()
const { nodeData } = defineProps<NodeWidgetsProps>()
const { shouldHandleNodePointerEvents, forwardEventToCanvas } =
useCanvasInteractions()

View File

@@ -16,7 +16,7 @@
ref="connectionDotRef"
:color="slotColor"
class="translate-x-1/2"
v-on="readonly ? {} : { pointerdown: onPointerDown }"
@pointerdown="onPointerDown"
/>
</div>
</template>
@@ -50,7 +50,6 @@ interface OutputSlotProps {
index: number
connected?: boolean
compatible?: boolean
readonly?: boolean
dotOnly?: boolean
}
@@ -87,7 +86,7 @@ const slotColor = computed(() => getSlotColor(props.slotData.type))
const slotWrapperClass = computed(() =>
cn(
'lg-slot lg-slot--output flex items-center justify-end group rounded-l-lg h-6',
props.readonly ? 'cursor-default opacity-70' : 'cursor-crosshair',
'cursor-crosshair',
props.dotOnly
? 'lg-slot--dot-only justify-center'
: 'pl-6 hover:bg-black/5 hover:dark:bg-white/5',
@@ -120,7 +119,6 @@ useSlotElementTracking({
const { onPointerDown } = useSlotLinkInteraction({
nodeId: props.nodeId ?? '',
index: props.index,
type: 'output',
readonly: props.readonly
type: 'output'
})
</script>

View File

@@ -28,7 +28,6 @@ interface SlotInteractionOptions {
nodeId: string
index: number
type: 'input' | 'output'
readonly?: boolean
}
interface SlotInteractionHandlers {
@@ -78,15 +77,8 @@ function createPointerSession(): PointerSession {
export function useSlotLinkInteraction({
nodeId,
index,
type,
readonly
type
}: SlotInteractionOptions): SlotInteractionHandlers {
if (readonly) {
return {
onPointerDown: () => {}
}
}
const { state, beginDrag, endDrag, updatePointerPosition } =
useSlotLinkDragState()

View File

@@ -3,12 +3,7 @@
<label v-if="widget.name" class="text-sm opacity-80">{{
widget.name
}}</label>
<Button
v-bind="filteredProps"
:disabled="readonly"
size="small"
@click="handleClick"
/>
<Button v-bind="filteredProps" size="small" @click="handleClick" />
</div>
</template>
@@ -25,7 +20,6 @@ import {
// Button widgets don't have a v-model value, they trigger actions
const props = defineProps<{
widget: SimplifiedWidget<void>
readonly?: boolean
}>()
// Button specific excluded props
@@ -36,7 +30,7 @@ const filteredProps = computed(() =>
)
const handleClick = () => {
if (!props.readonly && props.widget.callback) {
if (props.widget.callback) {
props.widget.callback()
}
}

View File

@@ -22,7 +22,6 @@ const value = defineModel<ChartData>({ required: true })
const props = defineProps<{
widget: SimplifiedWidget<ChartData, ChartWidgetOptions>
readonly?: boolean
}>()
const chartType = computed(() => props.widget.options?.type ?? 'line')

View File

@@ -9,7 +9,6 @@
<ColorPicker
v-model="localValue"
v-bind="filteredProps"
:disabled="readonly"
class="w-8 h-4 !rounded-full overflow-hidden border-none"
:pt="{
preview: '!w-full !h-full !border-none'
@@ -48,7 +47,6 @@ type WidgetOptions = { format?: ColorFormat } & Record<string, unknown>
const props = defineProps<{
widget: SimplifiedWidget<string, WidgetOptions>
modelValue: string
readonly?: boolean
}>()
const emit = defineEmits<{

View File

@@ -31,7 +31,6 @@
icon="pi pi-folder"
size="small"
class="!w-8 !h-8"
:disabled="readonly"
@click="triggerFileInput"
/>
</div>
@@ -47,7 +46,6 @@
/>
<!-- Control buttons in top right on hover -->
<div
v-if="!readonly"
class="absolute top-2 right-2 flex gap-1 opacity-0 group-hover:opacity-100 transition-opacity duration-200"
>
<!-- Edit button -->
@@ -100,7 +98,6 @@
icon="pi pi-folder"
size="small"
class="!w-8 !h-8"
:disabled="readonly"
@click="triggerFileInput"
/>
</div>
@@ -128,7 +125,7 @@
</div>
<!-- Control buttons -->
<div v-if="!readonly" class="flex gap-1">
<div class="flex gap-1">
<!-- Delete button -->
<button
class="w-8 h-8 rounded flex items-center justify-center transition-all duration-150 focus:outline-none border-none hover:bg-[#262729]"
@@ -159,7 +156,6 @@
size="small"
severity="secondary"
class="text-xs"
:disabled="readonly"
@click="triggerFileInput"
/>
</div>
@@ -173,7 +169,6 @@
class="hidden"
:accept="widget.options?.accept"
:multiple="false"
:disabled="readonly"
@change="handleFileChange"
/>
</template>
@@ -187,14 +182,9 @@ import { useWidgetValue } from '@/composables/graph/useWidgetValue'
import { useTransformCompatOverlayProps } from '@/composables/useTransformCompatOverlayProps'
import type { SimplifiedWidget } from '@/types/simplifiedWidget'
const {
widget,
modelValue,
readonly = false
} = defineProps<{
const { widget, modelValue } = defineProps<{
widget: SimplifiedWidget<File[] | null>
modelValue: File[] | null
readonly?: boolean
}>()
const emit = defineEmits<{
@@ -284,7 +274,7 @@ const triggerFileInput = () => {
const handleFileChange = (event: Event) => {
const target = event.target as HTMLInputElement
if (!readonly && target.files && target.files.length > 0) {
if (target.files && target.files.length > 0) {
// Since we only support single file, take the first one
const file = target.files[0]

View File

@@ -61,8 +61,7 @@ function createMockWidget(
function mountComponent(
widget: SimplifiedWidget<GalleryValue>,
modelValue: GalleryValue,
readonly = false
modelValue: GalleryValue
) {
return mount(WidgetGalleria, {
global: {
@@ -71,7 +70,6 @@ function mountComponent(
},
props: {
widget,
readonly,
modelValue
}
})
@@ -87,11 +85,10 @@ function createImageStrings(count: number): string[] {
// Factory function that takes images, creates widget internally, returns wrapper
function createGalleriaWrapper(
images: GalleryValue,
options: Partial<GalleriaProps> = {},
readonly = false
options: Partial<GalleriaProps> = {}
) {
const widget = createMockWidget(images, options)
return mountComponent(widget, images, readonly)
return mountComponent(widget, images)
}
describe('WidgetGalleria Image Display', () => {
@@ -249,25 +246,6 @@ describe('WidgetGalleria Image Display', () => {
})
})
describe('Readonly Mode', () => {
it('passes readonly state to galleria when readonly', () => {
const images = createImageStrings(3)
const widget = createMockWidget(images)
const wrapper = mountComponent(widget, images, true)
// Galleria component should receive readonly state (though it may not support disabled)
expect(wrapper.props('readonly')).toBe(true)
})
it('passes readonly state to galleria when not readonly', () => {
const images = createImageStrings(3)
const widget = createMockWidget(images)
const wrapper = mountComponent(widget, images, false)
expect(wrapper.props('readonly')).toBe(false)
})
})
describe('Widget Options Handling', () => {
it('passes through valid widget options', () => {
const images = createImageStrings(2)

View File

@@ -72,7 +72,6 @@ const value = defineModel<GalleryValue>({ required: true })
const props = defineProps<{
widget: SimplifiedWidget<GalleryValue>
readonly?: boolean
}>()
const activeIndex = ref(0)

View File

@@ -41,7 +41,6 @@ export interface ImageCompareValue {
// Image compare widgets typically don't have v-model, they display comparison
const props = defineProps<{
widget: SimplifiedWidget<ImageCompareValue | string>
readonly?: boolean
}>()
const beforeImage = computed(() => {

View File

@@ -6,7 +6,6 @@ import WidgetInputNumberSlider from './WidgetInputNumberSlider.vue'
defineProps<{
widget: SimplifiedWidget<number>
readonly?: boolean
}>()
const modelValue = defineModel<number>({ default: 0 })
@@ -21,7 +20,6 @@ const modelValue = defineModel<number>({ default: 0 })
"
v-model="modelValue"
:widget="widget"
:readonly="readonly"
v-bind="$attrs"
/>
</template>

View File

@@ -16,7 +16,6 @@ import WidgetLayoutField from './layout/WidgetLayoutField.vue'
const props = defineProps<{
widget: SimplifiedWidget<number>
modelValue: number
readonly?: boolean
}>()
const emit = defineEmits<{
@@ -72,7 +71,6 @@ const buttonsDisabled = computed(() => {
// Tooltip message for disabled buttons
const buttonTooltip = computed(() => {
if (props.readonly) return null
if (buttonsDisabled.value) {
return 'Increment/decrement disabled: value exceeds JavaScript precision limit (±2^53)'
}
@@ -89,7 +87,6 @@ const buttonTooltip = computed(() => {
:show-buttons="!buttonsDisabled"
button-layout="horizontal"
size="small"
:disabled="readonly"
:step="stepValue"
:use-grouping="useGrouping"
:class="cn(WidgetInputBaseClass, 'w-full text-xs')"

View File

@@ -8,7 +8,6 @@
<Slider
:model-value="[localValue]"
v-bind="filteredProps"
:disabled="readonly"
class="flex-grow text-xs"
:step="stepValue"
@update:model-value="updateLocalValue"
@@ -17,7 +16,6 @@
:key="timesEmptied"
:model-value="localValue"
v-bind="filteredProps"
:disabled="readonly"
:step="stepValue"
:min-fraction-digits="precision"
:max-fraction-digits="precision"
@@ -46,10 +44,9 @@ import {
import { WidgetInputBaseClass } from './layout'
import WidgetLayoutField from './layout/WidgetLayoutField.vue'
const { widget, modelValue, readonly } = defineProps<{
const { widget, modelValue } = defineProps<{
widget: SimplifiedWidget<number>
modelValue: number
readonly?: boolean
}>()
const emit = defineEmits<{

View File

@@ -3,7 +3,6 @@
<InputText
v-model="localValue"
v-bind="filteredProps"
:disabled="readonly"
:class="cn(WidgetInputBaseClass, 'w-full text-xs py-2 px-4')"
size="small"
@update:model-value="onChange"
@@ -29,7 +28,6 @@ import WidgetLayoutField from './layout/WidgetLayoutField.vue'
const props = defineProps<{
widget: SimplifiedWidget<string>
modelValue: string
readonly?: boolean
}>()
const emit = defineEmits<{

View File

@@ -15,7 +15,6 @@
v-show="isEditing"
ref="textareaRef"
v-model="localValue"
:disabled="readonly"
class="w-full min-h-[60px] absolute inset-0 resize-none"
:pt="{
root: {
@@ -45,7 +44,6 @@ import LODFallback from '../../components/LODFallback.vue'
const props = defineProps<{
widget: SimplifiedWidget<string>
modelValue: string
readonly?: boolean
}>()
const emit = defineEmits<{
@@ -70,7 +68,7 @@ const renderedHtml = computed(() => {
// Methods
const startEditing = async () => {
if (props.readonly || isEditing.value) return
if (isEditing.value) return
isEditing.value = true
await nextTick()

View File

@@ -4,7 +4,6 @@
v-model="localValue"
:options="multiSelectOptions"
v-bind="combinedProps"
:disabled="readonly"
class="w-full text-xs"
size="small"
display="chip"
@@ -33,7 +32,6 @@ import WidgetLayoutField from './layout/WidgetLayoutField.vue'
const props = defineProps<{
widget: SimplifiedWidget<T[]>
modelValue: T[]
readonly?: boolean
}>()
const emit = defineEmits<{

View File

@@ -31,7 +31,6 @@ import WidgetSelectDropdown from './WidgetSelectDropdown.vue'
const props = defineProps<{
widget: SimplifiedWidget<string | number | undefined>
modelValue: string | number | undefined
readonly?: boolean
}>()
const emit = defineEmits<{

View File

@@ -3,7 +3,6 @@
<FormSelectButton
v-model="localValue"
:options="widget.options?.values || []"
:disabled="readonly"
class="w-full"
@update:model-value="onChange"
/>
@@ -20,7 +19,6 @@ import WidgetLayoutField from './layout/WidgetLayoutField.vue'
const props = defineProps<{
widget: SimplifiedWidget<string>
modelValue: string
readonly?: boolean
}>()
const emit = defineEmits<{

View File

@@ -4,7 +4,6 @@
v-model="localValue"
:options="selectOptions"
v-bind="combinedProps"
:disabled="readonly"
:class="cn(WidgetInputBaseClass, 'w-full text-xs')"
size="small"
:pt="{
@@ -35,7 +34,6 @@ import WidgetLayoutField from './layout/WidgetLayoutField.vue'
const props = defineProps<{
widget: SimplifiedWidget<string | number | undefined>
modelValue: string | number | undefined
readonly?: boolean
}>()
const emit = defineEmits<{

View File

@@ -25,7 +25,6 @@ import WidgetLayoutField from './layout/WidgetLayoutField.vue'
const props = defineProps<{
widget: SimplifiedWidget<string | number | undefined>
modelValue: string | number | undefined
readonly?: boolean
assetKind?: AssetKind
allowUpload?: boolean
uploadFolder?: ResultItemType
@@ -222,7 +221,6 @@ const filterOptions = ref<FilterOption[]>([
:placeholder="mediaPlaceholder"
:multiple="false"
:uploadable="uploadable"
:disabled="readonly"
:filter-options="filterOptions"
v-bind="combinedProps"
class="w-full"

View File

@@ -3,7 +3,6 @@
<Textarea
v-model="localValue"
v-bind="filteredProps"
:disabled="readonly"
:class="cn(WidgetInputBaseClass, 'w-full text-xs lod-toggle')"
:placeholder="placeholder || widget.name || ''"
size="small"
@@ -33,7 +32,6 @@ import { WidgetInputBaseClass } from './layout'
const props = defineProps<{
widget: SimplifiedWidget<string>
modelValue: string
readonly?: boolean
placeholder?: string
}>()

View File

@@ -3,7 +3,6 @@
<ToggleSwitch
v-model="localValue"
v-bind="filteredProps"
:disabled="readonly"
@update:model-value="onChange"
/>
</WidgetLayoutField>
@@ -25,7 +24,6 @@ import WidgetLayoutField from './layout/WidgetLayoutField.vue'
const props = defineProps<{
widget: SimplifiedWidget<boolean>
modelValue: boolean
readonly?: boolean
}>()
const emit = defineEmits<{

View File

@@ -3,7 +3,6 @@
<TreeSelect
v-model="localValue"
v-bind="combinedProps"
:disabled="readonly"
class="w-full text-xs"
size="small"
@update:model-value="onChange"
@@ -37,7 +36,6 @@ export type TreeNode = {
const props = defineProps<{
widget: SimplifiedWidget<any>
modelValue: any
readonly?: boolean
}>()
const emit = defineEmits<{