Fix/selection toolbox reflow (#5158)

* fix: layout perf issue

* feat: skip a whole host of transform issues created by the SelectionOverlay and instead allowing the canvas to render the overlay and then injecting props to the SelecitonToolbox itself

* refactor: removed unused files/functionality

* refactor: removed unused types

* fix: z index issue

* fix: PR feedback

* fix: PR feedback and more perf improvements

* Update test expectations [skip ci]

---------

Co-authored-by: github-actions <github-actions@github.com>
This commit is contained in:
Simula_r
2025-08-22 12:36:20 -07:00
committed by GitHub
parent 3169628144
commit 84e7102f70
17 changed files with 238 additions and 166 deletions

View File

@@ -38,9 +38,7 @@
canvasStore.canvas to be initialized. -->
<template v-if="comfyAppReady">
<TitleEditor />
<SelectionOverlay v-if="selectionToolboxEnabled">
<SelectionToolbox />
</SelectionOverlay>
<SelectionToolbox v-if="selectionToolboxEnabled" />
<DomWidgets />
</template>
</template>
@@ -55,7 +53,6 @@ import DomWidgets from '@/components/graph/DomWidgets.vue'
import GraphCanvasMenu from '@/components/graph/GraphCanvasMenu.vue'
import MiniMap from '@/components/graph/MiniMap.vue'
import NodeTooltip from '@/components/graph/NodeTooltip.vue'
import SelectionOverlay from '@/components/graph/SelectionOverlay.vue'
import SelectionToolbox from '@/components/graph/SelectionToolbox.vue'
import TitleEditor from '@/components/graph/TitleEditor.vue'
import NodeSearchboxPopover from '@/components/searchbox/NodeSearchBoxPopover.vue'

View File

@@ -1,106 +0,0 @@
<!-- This component is used to bound the selected items on the canvas. -->
<template>
<div
v-show="visible"
class="selection-overlay-container pointer-events-none z-40"
:class="{
'show-border': showBorder
}"
:style="style"
>
<slot />
</div>
</template>
<script setup lang="ts">
import { whenever } from '@vueuse/core'
import { provide, readonly, ref, watch } from 'vue'
import { useSelectedLiteGraphItems } from '@/composables/canvas/useSelectedLiteGraphItems'
import { useAbsolutePosition } from '@/composables/element/useAbsolutePosition'
import { createBounds } from '@/lib/litegraph/src/litegraph'
import { useCanvasStore } from '@/stores/graphStore'
import { SelectionOverlayInjectionKey } from '@/types/selectionOverlayTypes'
const canvasStore = useCanvasStore()
const { style, updatePosition } = useAbsolutePosition()
const { getSelectableItems } = useSelectedLiteGraphItems()
const visible = ref(false)
const showBorder = ref(false)
// Increment counter to notify child components of position/visibility change
// This does not include viewport changes.
const overlayUpdateCount = ref(0)
provide(SelectionOverlayInjectionKey, {
visible: readonly(visible),
updateCount: readonly(overlayUpdateCount)
})
const positionSelectionOverlay = () => {
const selectableItems = getSelectableItems()
showBorder.value = selectableItems.size > 1
if (!selectableItems.size) {
visible.value = false
return
}
visible.value = true
const bounds = createBounds(selectableItems)
if (bounds) {
updatePosition({
pos: [bounds[0], bounds[1]],
size: [bounds[2], bounds[3]]
})
}
}
whenever(
() => canvasStore.getCanvas().state.selectionChanged,
() => {
requestAnimationFrame(() => {
positionSelectionOverlay()
overlayUpdateCount.value++
canvasStore.getCanvas().state.selectionChanged = false
})
},
{ immediate: true }
)
canvasStore.getCanvas().ds.onChanged = positionSelectionOverlay
watch(
() => canvasStore.canvas?.state?.draggingItems,
(draggingItems) => {
// Litegraph draggingItems state can end early before the bounding boxes of
// the selected items are updated. Delay to make sure we put the overlay in
// the correct position.
// https://github.com/Comfy-Org/ComfyUI_frontend/issues/2656
if (draggingItems === false) {
requestAnimationFrame(() => {
visible.value = true
positionSelectionOverlay()
overlayUpdateCount.value++
})
} else {
// Selection change update to visible state is delayed by a frame. Here
// we also delay a frame so that the order of events is correct when
// the initial selection and dragging happens at the same time.
requestAnimationFrame(() => {
visible.value = false
overlayUpdateCount.value++
})
}
}
)
</script>
<style scoped>
.selection-overlay-container > * {
pointer-events: auto;
}
.show-border {
@apply border-dashed rounded-md border-2 border-[var(--border-color)];
}
</style>

View File

@@ -1,34 +1,45 @@
<template>
<Panel
class="selection-toolbox absolute left-1/2 rounded-lg"
:class="{ 'animate-slide-up': shouldAnimate }"
:pt="{
header: 'hidden',
content: 'p-0 flex flex-row'
}"
@wheel="canvasInteractions.handleWheel"
>
<ExecuteButton />
<ColorPickerButton />
<BypassButton />
<PinButton />
<Load3DViewerButton />
<MaskEditorButton />
<ConvertToSubgraphButton />
<DeleteButton />
<RefreshSelectionButton />
<ExtensionCommandButton
v-for="command in extensionToolboxCommands"
:key="command.id"
:command="command"
/>
<HelpButton />
</Panel>
<Transition name="slide-up">
<!-- Wrapping panel in div to get correct ref because panel ref is not of raw dom el -->
<div
v-show="visible"
ref="toolboxRef"
style="
transform: translate(calc(var(--tb-x) - 50%), calc(var(--tb-y) - 120%));
"
class="selection-toolbox fixed left-0 top-0 z-40"
>
<Panel
class="rounded-lg"
:pt="{
header: 'hidden',
content: 'p-0 flex flex-row'
}"
@wheel="canvasInteractions.handleWheel"
>
<ExecuteButton />
<ColorPickerButton />
<BypassButton />
<PinButton />
<Load3DViewerButton />
<MaskEditorButton />
<ConvertToSubgraphButton />
<DeleteButton />
<RefreshSelectionButton />
<ExtensionCommandButton
v-for="command in extensionToolboxCommands"
:key="command.id"
:command="command"
/>
<HelpButton />
</Panel>
</div>
</Transition>
</template>
<script setup lang="ts">
import Panel from 'primevue/panel'
import { computed, inject } from 'vue'
import { computed, ref } from 'vue'
import BypassButton from '@/components/graph/selectionToolbox/BypassButton.vue'
import ColorPickerButton from '@/components/graph/selectionToolbox/ColorPickerButton.vue'
@@ -41,23 +52,19 @@ import Load3DViewerButton from '@/components/graph/selectionToolbox/Load3DViewer
import MaskEditorButton from '@/components/graph/selectionToolbox/MaskEditorButton.vue'
import PinButton from '@/components/graph/selectionToolbox/PinButton.vue'
import RefreshSelectionButton from '@/components/graph/selectionToolbox/RefreshSelectionButton.vue'
import { useRetriggerableAnimation } from '@/composables/element/useRetriggerableAnimation'
import { useSelectionToolboxPosition } from '@/composables/canvas/useSelectionToolboxPosition'
import { useCanvasInteractions } from '@/composables/graph/useCanvasInteractions'
import { useExtensionService } from '@/services/extensionService'
import { type ComfyCommandImpl, useCommandStore } from '@/stores/commandStore'
import { useCanvasStore } from '@/stores/graphStore'
import { SelectionOverlayInjectionKey } from '@/types/selectionOverlayTypes'
const commandStore = useCommandStore()
const canvasStore = useCanvasStore()
const extensionService = useExtensionService()
const canvasInteractions = useCanvasInteractions()
const selectionOverlayState = inject(SelectionOverlayInjectionKey)
const { shouldAnimate } = useRetriggerableAnimation(
selectionOverlayState?.updateCount,
{ animateOnMount: true }
)
const toolboxRef = ref<HTMLElement | undefined>()
const { visible } = useSelectionToolboxPosition(toolboxRef)
const extensionToolboxCommands = computed<ComfyCommandImpl[]>(() => {
const commandIds = new Set<string>(
@@ -77,23 +84,22 @@ const extensionToolboxCommands = computed<ComfyCommandImpl[]>(() => {
</script>
<style scoped>
.selection-toolbox {
transform: translateX(-50%) translateY(-120%);
.slide-up-enter-active {
opacity: 1;
transition: all 0.3s ease-out;
}
/* Slide up animation using CSS animation */
@keyframes slideUp {
from {
transform: translateX(-50%) translateY(-100%);
opacity: 0;
}
to {
transform: translateX(-50%) translateY(-120%);
opacity: 1;
}
.slide-up-leave-active {
transition: none;
}
.animate-slide-up {
animation: slideUp 0.3s ease-out;
.slide-up-enter-from {
transform: translateY(-100%);
opacity: 0;
}
.slide-up-leave-to {
transform: translateY(0);
opacity: 0;
}
</style>