Skip to content
Merged
Show file tree
Hide file tree
Changes from 7 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.
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.
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.
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.
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.
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.
108 changes: 75 additions & 33 deletions src/components/breadcrumb/SubgraphBreadcrumb.vue
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
<template>
<div
class="subgraph-breadcrumb w-auto drop-shadow-[var(--interface-panel-drop-shadow)]"
class="subgraph-breadcrumb flex w-auto drop-shadow-[var(--interface-panel-drop-shadow)]"
:class="{
'subgraph-breadcrumb-collapse': collapseTabs,
'subgraph-breadcrumb-overflow': overflowingTabs
Expand All @@ -13,17 +13,37 @@
'--p-breadcrumb-icon-width': `${ICON_WIDTH}px`
}"
>
<Button
class="context-menu-button pointer-events-auto h-8 w-8 shrink-0 border border-transparent bg-transparent p-0 transition-all hover:rounded-lg hover:border-interface-stroke hover:bg-comfy-menu-bg"
icon="pi pi-bars"
text
severity="secondary"
size="small"
@click="handleMenuClick"
/>
<Button
v-if="isInSubgraph"
class="back-button pointer-events-auto h-8 w-8 shrink-0 border border-transparent bg-transparent p-0 transition-all hover:rounded-lg hover:border-interface-stroke hover:bg-comfy-menu-bg"
text
severity="secondary"
size="small"
@click="handleBackClick"
>
<i class="icon-[lucide--undo-2]" />
</Button>
Comment on lines +16 to +33
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Replace PrimeVue Button with IconButton from common components.

The new menu and back buttons use PrimeVue's Button component directly, which violates the coding guideline to avoid new PrimeVue usage and prefer the repo's common button components.

Based on coding guidelines, the project prefers common button components from src/components/button/.

♻️ Proposed refactor to use IconButton

Update the import:

-import Button from 'primevue/button'
+import IconButton from '@/components/button/IconButton.vue'

Replace the menu button:

-    <Button
-      class="context-menu-button pointer-events-auto h-8 w-8 shrink-0 border border-transparent bg-transparent p-0 transition-all hover:rounded-lg hover:border-interface-stroke hover:bg-comfy-menu-bg"
-      icon="pi pi-bars"
-      text
-      severity="secondary"
-      size="small"
-      @click="handleMenuClick"
-    />
+    <IconButton
+      class="context-menu-button"
+      icon="pi pi-bars"
+      @click="handleMenuClick"
+    />

Replace the back button:

-    <Button
-      v-if="isInSubgraph"
-      class="back-button pointer-events-auto h-8 w-8 shrink-0 border border-transparent bg-transparent p-0 transition-all hover:rounded-lg hover:border-interface-stroke hover:bg-comfy-menu-bg"
-      text
-      severity="secondary"
-      size="small"
-      @click="handleBackClick"
-    >
-      <i class="icon-[lucide--undo-2]" />
-    </Button>
+    <IconButton
+      v-if="isInSubgraph"
+      class="back-button"
+      icon="icon-[lucide--undo-2]"
+      @click="handleBackClick"
+    />

Also applies to: 58-58

🤖 Prompt for AI Agents
In @src/components/breadcrumb/SubgraphBreadcrumb.vue around lines 16 - 33,
Replace the PrimeVue <Button> usages in SubgraphBreadcrumb.vue with the repo’s
IconButton component: add an import for IconButton, swap the two <Button>
elements (the menu button that calls handleMenuClick and the conditional back
button that calls handleBackClick) to use IconButton, keep the existing class
attributes and click handlers, set the menu icon to "pi pi-bars" and the back
icon to "lucide--undo-2", and remove PrimeVue-specific props (text, severity,
size) in favor of IconButton’s API so the visual/behavioral intent is preserved.

<Breadcrumb
ref="breadcrumbRef"
class="w-fit rounded-lg p-0"
:class="{ hidden: !isInSubgraph }"
:model="items"
:pt="{ item: { class: 'pointer-events-auto' } }"
:aria-label="$t('g.graphNavigation')"
>
<template #item="{ item }">
<SubgraphBreadcrumbItem
:ref="(el) => setItemRef(item, el)"
:item="item"
:is-active="item === items.at(-1)"
:is-active="item.key === activeItemKey"
/>
</template>
<template #separator
Expand All @@ -35,6 +55,7 @@

<script setup lang="ts">
import Breadcrumb from 'primevue/breadcrumb'
import Button from 'primevue/button'
import type { MenuItem } from 'primevue/menuitem'
import { computed, onUpdated, ref, watch } from 'vue'

Expand All @@ -43,6 +64,7 @@ import { useOverflowObserver } from '@/composables/element/useOverflowObserver'
import { useTelemetry } from '@/platform/telemetry'
import { useWorkflowStore } from '@/platform/workflow/management/stores/workflowStore'
import { useCanvasStore } from '@/renderer/core/canvas/canvasStore'
import { useCommandStore } from '@/stores/commandStore'
import { useSubgraphNavigationStore } from '@/stores/subgraphNavigationStore'
import { useSubgraphStore } from '@/stores/subgraphStore'
import { forEachSubgraphNode } from '@/utils/graphTraversalUtil'
Expand All @@ -55,24 +77,41 @@ const ICON_WIDTH = 20
const workflowStore = useWorkflowStore()
const navigationStore = useSubgraphNavigationStore()
const breadcrumbRef = ref<InstanceType<typeof Breadcrumb>>()
const rootItemRef = ref<InstanceType<typeof SubgraphBreadcrumbItem>>()
const setItemRef = (item: MenuItem, el: unknown) => {
if (item.key === 'root') {
rootItemRef.value = el as InstanceType<typeof SubgraphBreadcrumbItem>
}
}
const workflowName = computed(() => workflowStore.activeWorkflow?.filename)
const isBlueprint = computed(() =>
useSubgraphStore().isSubgraphBlueprint(workflowStore.activeWorkflow)
)
const collapseTabs = ref(false)
const overflowingTabs = ref(false)

const breadcrumbElement = computed(() => {
if (!breadcrumbRef.value) return null
const isInSubgraph = computed(() => navigationStore.navigationStack.length > 0)

const el = (breadcrumbRef.value as unknown as { $el: HTMLElement }).$el
const list = el?.querySelector('.p-breadcrumb-list') as HTMLElement
return list
})
const home = computed(() => ({
label: workflowName.value,
icon: 'pi pi-home',
key: 'root',
isBlueprint: isBlueprint.value,
command: () => {
useTelemetry()?.trackUiButtonClicked({
button_id: 'breadcrumb_subgraph_root_selected'
})
const canvas = useCanvasStore().getCanvas()
if (!canvas.graph) throw new TypeError('Canvas has no graph')

canvas.setGraph(canvas.graph.rootGraph)
}
}))
Comment on lines +95 to +109
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Consider improving error handling for missing graph.

The code throws a TypeError when canvas.graph is missing. While this surfaces the issue during development, it provides no user feedback and doesn't follow proper error handling patterns.

Consider one of these approaches:

  • Log the error and gracefully disable the command
  • Show a user-facing error message via a toast/notification
  • Guard the command execution earlier to prevent invalid states
💡 Example: Graceful error handling
  command: () => {
    useTelemetry()?.trackUiButtonClicked({
      button_id: 'breadcrumb_subgraph_root_selected'
    })
    const canvas = useCanvasStore().getCanvas()
-   if (!canvas.graph) throw new TypeError('Canvas has no graph')
+   if (!canvas.graph) {
+     console.error('[SubgraphBreadcrumb] Canvas has no graph')
+     return
+   }

    canvas.setGraph(canvas.graph.rootGraph)
  }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const home = computed(() => ({
label: workflowName.value,
icon: 'pi pi-home',
key: 'root',
isBlueprint: isBlueprint.value,
command: () => {
useTelemetry()?.trackUiButtonClicked({
button_id: 'breadcrumb_subgraph_root_selected'
})
const canvas = useCanvasStore().getCanvas()
if (!canvas.graph) throw new TypeError('Canvas has no graph')
canvas.setGraph(canvas.graph.rootGraph)
}
}))
const home = computed(() => ({
label: workflowName.value,
icon: 'pi pi-home',
key: 'root',
isBlueprint: isBlueprint.value,
command: () => {
useTelemetry()?.trackUiButtonClicked({
button_id: 'breadcrumb_subgraph_root_selected'
})
const canvas = useCanvasStore().getCanvas()
if (!canvas.graph) {
console.error('[SubgraphBreadcrumb] Canvas has no graph')
return
}
canvas.setGraph(canvas.graph.rootGraph)
}
}))
🤖 Prompt for AI Agents
In @src/components/breadcrumb/SubgraphBreadcrumb.vue around lines 95 - 109, The
breadcrumb home computed's command currently throws a TypeError when
canvas.graph is missing; instead catch and handle this case: update the command
function inside the home computed (the command property on the computed returned
object) to check for canvas and canvas.graph early, and if missing log the error
via the existing logger/telemetry and show a user-facing notification or toast
(or disable/return early) rather than throwing; ensure you still call
useTelemetry()?.trackUiButtonClicked and only call
canvas.setGraph(canvas.graph.rootGraph) when canvas.graph is valid, and add a
clear telemetry/log message and a user notification to surface the problem.


const items = computed(() => {
const items = navigationStore.navigationStack.map<MenuItem>((subgraph) => ({
label: subgraph.name,
key: `subgraph-${subgraph.id}`,
command: () => {
useTelemetry()?.trackUiButtonClicked({
button_id: 'breadcrumb_subgraph_item_selected'
Expand All @@ -95,21 +134,26 @@ const items = computed(() => {
return [home.value, ...items]
})

const home = computed(() => ({
label: workflowName.value,
icon: 'pi pi-home',
key: 'root',
isBlueprint: isBlueprint.value,
command: () => {
useTelemetry()?.trackUiButtonClicked({
button_id: 'breadcrumb_subgraph_root_selected'
})
const canvas = useCanvasStore().getCanvas()
if (!canvas.graph) throw new TypeError('Canvas has no graph')
const activeItemKey = computed(() => items.value.at(-1)?.key)

canvas.setGraph(canvas.graph.rootGraph)
}
}))
const handleMenuClick = (event: MouseEvent) => {
useTelemetry()?.trackUiButtonClicked({
button_id: 'breadcrumb_subgraph_menu_selected'
})
rootItemRef.value?.toggleMenu(event)
}

const handleBackClick = () => {
void useCommandStore().execute('Comfy.Graph.ExitSubgraph')
}
Comment on lines +146 to +148
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Missing await for async command execution.

Line 151 calls useCommandStore().execute() which returns a Promise, but the result is voided without awaiting. If the command fails, the error won't be caught.

🔎 Proposed fix
-const handleBackClick = () => {
+const handleBackClick = async () => {
-  void useCommandStore().execute('Comfy.Graph.ExitSubgraph')
+  try {
+    await useCommandStore().execute('Comfy.Graph.ExitSubgraph')
+  } catch (error) {
+    console.error('Failed to exit subgraph:', error)
+  }
 }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const handleBackClick = () => {
void useCommandStore().execute('Comfy.Graph.ExitSubgraph')
}
const handleBackClick = async () => {
try {
await useCommandStore().execute('Comfy.Graph.ExitSubgraph')
} catch (error) {
console.error('Failed to exit subgraph:', error)
}
}
🤖 Prompt for AI Agents
In @src/components/breadcrumb/SubgraphBreadcrumb.vue around lines 150 - 152, The
handler handleBackClick is calling
useCommandStore().execute('Comfy.Graph.ExitSubgraph') and dropping the returned
Promise with void, so failures will be lost; make handleBackClick async, await
useCommandStore().execute(...) and wrap the await in a try/catch (or return the
Promise) to surface/log errors (reference handleBackClick and
useCommandStore().execute).


const breadcrumbElement = computed(() => {
if (!breadcrumbRef.value) return null

const el = (breadcrumbRef.value as unknown as { $el: HTMLElement }).$el
const list = el?.querySelector('.p-breadcrumb-list') as HTMLElement
return list
})

// Check for overflow on breadcrumb items and collapse/expand the breadcrumb to fit
let overflowObserver: ReturnType<typeof useOverflowObserver> | undefined
Expand Down Expand Up @@ -189,13 +233,18 @@ onUpdated(() => {
}

:deep(.p-breadcrumb-item) {
@apply flex items-center overflow-hidden;
@apply flex items-center overflow-hidden h-8;
min-width: calc(var(--p-breadcrumb-item-min-width) + 1rem);
border: 1px solid transparent;
background-color: transparent;
transition: all 0.2s;
/* Collapse middle items first */
flex-shrink: 10000;
}

:deep(.p-breadcrumb-separator) {
border: 1px solid transparent;
background-color: transparent;
display: flex;
padding: 0 var(--p-breadcrumb-item-margin);
}
Expand All @@ -205,11 +254,9 @@ onUpdated(() => {
calc(var(--p-breadcrumb-item-margin) + var(--p-breadcrumb-item-padding));
}

:deep(.p-breadcrumb-separator),
:deep(.p-breadcrumb-item) {
@apply h-12;
border-top: 1px solid var(--interface-stroke);
border-bottom: 1px solid var(--interface-stroke);
:deep(.p-breadcrumb-item:hover) {
@apply rounded-lg;
border-color: var(--interface-stroke);
background-color: var(--comfy-menu-bg);
}

Expand All @@ -218,24 +265,19 @@ onUpdated(() => {
}

:deep(.p-breadcrumb-item:first-child) {
@apply rounded-l-lg;
/* Then collapse the root workflow */
flex-shrink: 5000;
border-left: 1px solid var(--interface-stroke);

.p-breadcrumb-item-link {
padding-left: var(--p-breadcrumb-item-padding);
}
}

:deep(.p-breadcrumb-item:last-child) {
@apply rounded-r-lg;
/* Then collapse the active item */
flex-shrink: 1;
border-right: 1px solid var(--interface-stroke);
}

:deep(.p-breadcrumb-item-link:hover),
:deep(.p-breadcrumb-item-link-menu-visible) {
background-color: color-mix(
in srgb,
Expand Down
118 changes: 31 additions & 87 deletions src/components/breadcrumb/SubgraphBreadcrumbItem.vue
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
}"
draggable="false"
href="#"
class="p-breadcrumb-item-link h-12 cursor-pointer px-2"
class="p-breadcrumb-item-link h-8 cursor-pointer px-2"
:class="{
'flex items-center gap-1': isActive,
'p-breadcrumb-item-link-menu-visible': menu?.overlayVisible,
Expand All @@ -25,7 +25,7 @@
<i v-if="isActive" class="pi pi-angle-down text-[10px]"></i>
</a>
<Menu
v-if="isActive"
v-if="isActive || isRoot"
ref="menu"
:model="menuItems"
:popup="true"
Expand Down Expand Up @@ -59,6 +59,7 @@ import Tag from 'primevue/tag'
import { computed, nextTick, ref } from 'vue'
import { useI18n } from 'vue-i18n'

import { useWorkflowActionsMenu } from '@/composables/useWorkflowActionsMenu'
import { useWorkflowService } from '@/platform/workflow/core/services/workflowService'
import {
ComfyWorkflow,
Expand Down Expand Up @@ -135,79 +136,28 @@ const tooltipText = computed(() => {
return props.item.label
})

const menuItems = computed<MenuItem[]>(() => {
return [
{
label: t('g.rename'),
icon: 'pi pi-pencil',
command: startRename
},
{
label: t('breadcrumbsMenu.duplicate'),
icon: 'pi pi-copy',
command: async () => {
await workflowService.duplicateWorkflow(workflowStore.activeWorkflow!)
},
visible: isRoot && !props.item.isBlueprint
},
{
separator: true,
visible: isRoot
},
{
label: t('menuLabels.Save'),
icon: 'pi pi-save',
command: async () => {
await useCommandStore().execute('Comfy.SaveWorkflow')
},
visible: isRoot
},
{
label: t('menuLabels.Save As'),
icon: 'pi pi-save',
command: async () => {
await useCommandStore().execute('Comfy.SaveWorkflowAs')
},
visible: isRoot
},
{
separator: true
},
{
label: t('breadcrumbsMenu.clearWorkflow'),
icon: 'pi pi-trash',
command: async () => {
await useCommandStore().execute('Comfy.ClearWorkflow')
const startRename = async () => {
// Check if element is hidden (collapsed breadcrumb)
// When collapsed, root item is hidden via CSS display:none, so use rename command
if (isRoot && wrapperRef.value?.offsetParent === null) {
await useCommandStore().execute('Comfy.RenameWorkflow')
return
}

isEditing.value = true
itemLabel.value = props.item.label as string
void nextTick(() => {
if (itemInputRef.value?.$el) {
itemInputRef.value.$el.focus()
itemInputRef.value.$el.select()
if (wrapperRef.value) {
itemInputRef.value.$el.style.width = `${Math.max(200, wrapperRef.value.offsetWidth)}px`
}
},
{
separator: true,
visible: props.item.key === 'root' && props.item.isBlueprint
},
{
label: t('subgraphStore.publish'),
icon: 'pi pi-copy',
command: async () => {
await workflowService.saveWorkflowAs(workflowStore.activeWorkflow!)
},
visible: props.item.key === 'root' && props.item.isBlueprint
},
{
separator: true,
visible: isRoot
},
{
label: props.item.isBlueprint
? t('breadcrumbsMenu.deleteBlueprint')
: t('breadcrumbsMenu.deleteWorkflow'),
icon: 'pi pi-times',
command: async () => {
await workflowService.deleteWorkflow(workflowStore.activeWorkflow!)
},
visible: isRoot
}
]
})
})
}

const { menuItems } = useWorkflowActionsMenu(startRename, { isRoot })

const handleClick = (event: MouseEvent) => {
if (isEditing.value) {
Expand All @@ -228,27 +178,21 @@ const handleClick = (event: MouseEvent) => {
}
}

const startRename = () => {
isEditing.value = true
itemLabel.value = props.item.label as string
void nextTick(() => {
if (itemInputRef.value?.$el) {
itemInputRef.value.$el.focus()
itemInputRef.value.$el.select()
if (wrapperRef.value) {
itemInputRef.value.$el.style.width = `${Math.max(200, wrapperRef.value.offsetWidth)}px`
}
}
})
}

const inputBlur = async (doRename: boolean) => {
if (doRename) {
await rename(itemLabel.value, props.item.label as string)
}

isEditing.value = false
}

const toggleMenu = (event: MouseEvent) => {
menu.value?.toggle(event)
}

defineExpose({
toggleMenu
})
</script>

<style scoped>
Expand Down
Loading
Loading