[DO NOT MERGE] Cloud/1.37 - cloud release branch#8063
[DO NOT MERGE] Cloud/1.37 - cloud release branch#8063christian-byrne wants to merge 50 commits intomainfrom
Conversation
…wright tests despite DisableWarnings setting (#8039) Backport of #8036 to `cloud/1.37` Automatically created by backport workflow. ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-8039-backport-cloud-1-37-fix-version-mismatch-warning-appearing-in-Playwright-tests-despite-2e86d73d3650817d9534c0449798e7b1) by [Unito](https://www.unito.io) Co-authored-by: Christian Byrne <cbyrne@comfy.org>
Manual backport of #7734 to `cloud/1.37` ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-8047-backport-cloud-1-37-linear-v2-Simple-Mode-2e86d73d365081948861debeae9604f0) by [Unito](https://www.unito.io)
…#8049) Backport of #7966 to `cloud/1.37` Automatically created by backport workflow. ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-8049-backport-cloud-1-37-API-Nodes-add-price-badges-for-Meshy-3D-nodes-2e86d73d3650815b8df4c0f4c2957f65) by [Unito](https://www.unito.io) Co-authored-by: Alexander Piskun <13381981+bigcat88@users.noreply.github.com>
Backport of #8031 to `cloud/1.37` Automatically created by backport workflow. ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-8051-backport-cloud-1-37-Fix-Update-for-Image-Widget-test-2e86d73d365081bba5e0e5f75aa8a7d9) by [Unito](https://www.unito.io) --------- Co-authored-by: Alexander Brown <drjkl@comfy.org> Co-authored-by: github-actions <github-actions@github.com> Co-authored-by: Alexander Brown <448862+DrJKL@users.noreply.github.com>
christian-byrne
requested review from
a team,
Myestery,
bigcat88,
comfyui-wiki,
jojodecayz and
pythongosssss
as code owners
dosubot
bot
added
the
size:XXL
📝 WalkthroughWalkthroughAdds workspace/team management, workspace-aware auth and remote-config flow, a linear-mode UI suite (controls, preview, history), extensive asset browser and asset-store pagination refactors, many new UI components (Select, TagsInput, Popover, ZoomPane), telemetry and command flow updates, and assorted UI/UX and test adjustments. Changes
Sequence DiagramssequenceDiagram
actor User
participant App
participant WorkspaceAuthGate
participant Firebase
participant RemoteConfig
participant TeamWorkspaceStore
participant SessionStorage
User->>App: Load application
App->>WorkspaceAuthGate: Mount gate
WorkspaceAuthGate->>Firebase: Await auth init (16s timeout)
Firebase-->>WorkspaceAuthGate: Auth ready or null
WorkspaceAuthGate->>RemoteConfig: refreshRemoteConfig(useAuth: true) (10s timeout)
RemoteConfig-->>WorkspaceAuthGate: Config loaded / error
WorkspaceAuthGate->>TeamWorkspaceStore: if flags.teamWorkspacesEnabled initialize store
TeamWorkspaceStore->>SessionStorage: load persisted workspace context
SessionStorage-->>TeamWorkspaceStore: context
TeamWorkspaceStore-->>WorkspaceAuthGate: initialized
WorkspaceAuthGate->>App: set isReady and render app
sequenceDiagram
actor User
participant ModeToggle
participant CommandStore
participant Telemetry
participant AppGraph
participant WorkflowStore
participant CanvasStore
User->>ModeToggle: Click toggle
ModeToggle->>CommandStore: execute Comfy.ToggleLinear({source:'button'})
CommandStore->>CommandStore: newMode = !canvas.linearMode
alt entering linear
CommandStore->>Telemetry: trackEnterLinear({source:'button'})
end
CommandStore->>AppGraph: persist extra.linearMode = newMode
CommandStore->>WorkflowStore: workflow.changeTracker.checkState()
CommandStore->>CanvasStore: canvas.linearMode = newMode
CanvasStore-->>User: UI updated
Possibly Related PRs
Suggested reviewers
✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
coderabbitai
bot
requested review from
AustinMroz,
KarryCharon,
Yorha4D and
shinshin86
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 36
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/components/topbar/WorkflowTab.vue (1)
228-232: Pre-existing<style>block violates coding guidelines.Per coding guidelines, Vue components should use Tailwind CSS only for styling (no custom CSS). This
<style>block with!importantexists in the current file. Consider addressing this in a follow-up to align with project conventions.
🤖 Fix all issues with AI agents
In `@src/components/sidebar/ModeToggle.vue`:
- Around line 1-27: The component imports t directly from '@/i18n' instead of
using the Vue i18n composable; replace the top-level import "import { t } from
'@/i18n'" with "import { useI18n } from 'vue-i18n'" and inside the <script
setup> call const { t } = useI18n() so the template uses the composable t; keep
existing canvasStore and useCommandStore usage (symbols: ModeToggle.vue,
useI18n, const { t } = useI18n(), canvasStore, useCommandStore) and remove the
old direct import.
In `@src/components/topbar/WorkflowTab.vue`:
- Around line 19-22: Add a descriptive title to the decorative icon in
WorkflowTab.vue and correct its accessibility flag: update the <i> element
rendered by the v-else-if branch (the element with class
"icon-[lucide--panels-top-left] bg-primary-background") to include a title
attribute like title="Linear mode" and ensure it uses aria-hidden="true" (or
omit aria-hidden entirely) since it is decorative.
In `@src/components/ui/Popover.vue`:
- Around line 26-32: The icon-only default trigger lacks an accessible name—add
an aria-label to the <Button> in the default slot: introduce a prop (e.g.,
ariaLabel) on this Popover component (props: ariaLabel: { type: String, default:
null }) and bind it to the Button as :aria-label="ariaLabel ??
$t('popover.openOptions')" (or a sensible default string if i18n isn't
available); keep the binding next to size="icon" inside the slot so
<PopoverTrigger> / slot="button" / Button / icon remain unchanged otherwise.
In `@src/components/ui/TypeformPopoverButton.vue`:
- Around line 22-26: The icon-only help Button in the template (`#button`) lacks
an accessible name; update the Button element in TypeformPopoverButton.vue to
include an aria-label bound to a vue-i18n translation (e.g.,
$t('typeform.help_button') or similar key) so screen readers get a descriptive
label; add the i18n key to your locale messages and use the aria-label binding
on the Button component instance.
- Around line 14-18: The whenever callback currently appends a new script every
time feedbackRef becomes truthy, risking duplicate injections; modify the
callback in whenever to first check for an existing script with the same src (or
a unique id) inside feedbackRef.value (or document) before creating/appending
scriptEl (e.g., use
feedbackRef.value.querySelector('script[src="//embed.typeform.com/next/embed.js"]')
or document.getElementById) and only create/append the script if that query
returns null, ensuring the Typeform embed script is injected once.
In `@src/components/ui/ZoomPane.vue`:
- Around line 10-22: handleWheel uses raw e.deltaY causing wildly different zoom
steps across devices; normalize and clamp deltaY, then compute scaler from that
normalized value and apply it to zoom and pan. Specifically, in handleWheel
normalize deltaY (e.g., let delta = Math.sign(e.deltaY) *
Math.min(Math.abs(e.deltaY), 100) or divide by a constant like 100 to get a
bounded delta in [-1,1]), use that normalized delta in the scaler formula
(replace 1.1 ** (e.deltaY / -30) with 1.1 ** (normalizedDelta / -30) or
similar), update zoom.value using the scaler (e.g., multiply or add using the
normalized delta consistently) and compute panX/panY adjustments using the same
scaler so zoom behavior is consistent across devices; make these changes inside
handleWheel and keep references to zoom.value, panX.value, panY.value, and the
scaler calculation.
- Around line 24-38: The module-scoped plain `dragging` causes cross-instance
interference; replace it with a component-local Vue ref (e.g., `const dragging =
ref(false)`) and import ref from 'vue', then update the handlers `handleDown`,
`handleMove` and the corresponding pointerup/pointercancel handler to read/write
`dragging.value` instead of `dragging`, ensure you call/release pointer capture
on the `zoomPane` element within `handleDown`/`handleUp` using the same logic,
and update any template event bindings to use the revised handlers that rely on
the instance-local `dragging` ref so multiple ZoomPane components no longer
share state.
In `@src/composables/graph/useGraphNodeManager.ts`:
- Around line 239-248: The computed safeWidgets populates slotMetadata from
node.inputs but never clears it, causing stale entries when inputs are removed;
update the reactiveComputed callback for safeWidgets to clear slotMetadata
(e.g., call slotMetadata.clear()) before iterating node.inputs so only current
inputs are mapped, then proceed to set entries and return node.widgets mapped
via safeWidgetMapper(node, slotMetadata).
- Around line 210-228: extractVueNodeData is mutating its input node by
redefining node.widgets and node.inputs to reactive proxies (using
shallowReactive and Object.defineProperty), which can double-wrap if called
twice; either rename the function to indicate mutation (e.g.,
setupReactiveNodeData) or add guards before redefining: check for existing
reactive proxies or previously defined descriptors on node (e.g., test if
Object.getOwnPropertyDescriptor(node, 'widgets') exists or if node.widgets is
already an array produced by shallowReactive) and skip redefinition, and ensure
the code uses the same reactiveWidgets/reactiveInputs variables
(reactiveWidgets, shallowReactive) so multiple calls are idempotent rather than
replacing/mutating existing reactive state.
In `@src/composables/node/useNodePricing.ts`:
- Around line 5-14: The helper meshyCreditsToUsd is implemented as an arrow
function contrary to project convention; change it to a function declaration
named meshyCreditsToUsd that returns credits * MESHY_CREDIT_PRICE_USD, keeping
the constant MESHY_CREDIT_PRICE_USD as-is, so the pure helper is hoisted and
matches codebase style.
In `@src/platform/workflow/core/services/workflowService.ts`:
- Around line 341-344: The loading logic only sets
app.rootGraph.extra.linearMode when useCanvasStore().linearMode is true, causing
false not to be persisted; change the block that currently guards with if
(useCanvasStore().linearMode) to always initialize app.rootGraph.extra and
assign app.rootGraph.extra.linearMode = useCanvasStore().linearMode so the
stored false value is saved as well (ensure app.rootGraph.extra ??= {} remains
to initialize the object before assignment).
In `@src/renderer/extensions/linearMode/DropZone.vue`:
- Around line 19-35: The drop zone div in DropZone.vue lacks accessibility
attributes; add role and an aria-label to the root div that describes the drop
target (e.g., role="region" and aria-label="Drop files here" or a prop-driven
label). Locate the div that uses v-if="onDragOver && onDragDrop" and add static
or bound attributes (e.g., :aria-label="dropLabel || 'Drop files here'") so
screen readers announce its purpose; ensure the label can be passed in via a
prop (e.g., dropLabel) if dynamic text is needed and keep existing drag handlers
unchanged.
- Around line 29-34: The drop handler currently calls onDragDrop without
awaiting its Promise, so failures still reset canAcceptDrop; make the inline
handler async, await the result of onDragDrop (onDragDrop!(e) or
onDragDrop?.(e)), and only set canAcceptDrop = false when the awaited result is
truthy (or handle errors explicitly). Also wrap the await in try/catch to
handle/rethrow or log errors and avoid resetting canAcceptDrop on failure (or
use finally if you intentionally always want to reset).
In `@src/renderer/extensions/linearMode/ImagePreview.vue`:
- Around line 17-42: The `@load` handler logic for setting width and height from
imageRef is duplicated in both the ZoomPane branch and the standalone <img>
branch; extract this into a single method (e.g., onImageLoad) in the component
and have both <img> elements call that method instead of inline handlers. Locate
usages of imageRef, width, height and the ZoomPane markup, move the
arrow-function body that checks imageRef and assigns width/height into the new
onImageLoad method, and replace both `@load`="() => { ... }" occurrences with
`@load`="onImageLoad".
In `@src/renderer/extensions/linearMode/LinearControls.vue`:
- Line 195: The Button element in LinearControls.vue is dead code due to
v-if="false"; remove this unused <Button> element or replace the hardcoded
condition with a real feature flag or prop (e.g., use a prop like
showPublishButton or a computed/feature-flag getter such as isPublishEnabled)
and update the template to use v-if="showPublishButton" (or
v-if="isPublishEnabled") so the publish button is either removed or properly
gated; ensure to add the corresponding prop/default or computed implementation
in the component script (e.g., define props: { showPublishButton: { type:
Boolean, default: false } } or import the feature-flag getter).
- Around line 279-283: Remove the dead Teleport block in LinearControls.vue: the
<Teleport v-if="false" ...> block (targeting notesTo) will never render, so
delete the entire Teleport element and its empty div, or if it was intentionally
kept for future use, replace v-if="false" with a clear TODO comment explaining
why it must remain and reference notesTo; ensure no unused bindings (like
notesTo) remain after removal.
- Around line 145-153: The interrupt Button in LinearControls.vue (the icon-only
button that calls commandStore.execute('Comfy.Interrupt') when
executionStore.isIdle is false) lacks an accessible label; add an aria-label
(e.g., aria-label="Interrupt" or a localized equivalent) to that Button element
to describe its action, and apply the same aria-label fix to the duplicate
Button instance later in the file (the other Button that also invokes
commandStore.execute('Comfy.Interrupt')) so both icon-only buttons are
accessible.
- Around line 60-62: The mapping mutates original widget objects by setting
widget.slotMetadata = undefined; instead, avoid side effects by cloning the
widgets array/objects before clearing slotMetadata (e.g., replace the in-place
loop in nodeToNodeData/where extractVueNodeData is used with a non-mutating
transform of nodeData.widgets such as mapping to new widget objects or doing a
shallow/deep clone and then setting slotMetadata on the clones) so the original
widget objects remain unchanged.
In `@src/renderer/extensions/linearMode/LinearPreview.vue`:
- Around line 124-133: The icon-only download Button lacks an accessible name;
update the Button component instance (the one calling downloadFile with
selectedOutput?.url) to include an aria-label such as "Download file" (or a
contextual label e.g. `Download ${selectedOutput.name}`) so screen readers can
announce its purpose; keep the existing click handler and icon class (i.e., the
Button that wraps <i class="icon-[lucide--download]">) and add the aria-label
prop to it.
- Around line 60-66: The array building for preview stats can include an empty
object because the fallback "?? {}" makes a truthy value; change the fallback
for the media-type entry so it returns a falsy value when there's no media type
(e.g., use undefined/null instead of {}), or alternatively compute the
media-entry and only push it if it has content (use
getMediaType(selectedOutput), mediaTypes, and selectedOutput to guard creation).
Ensure the entry creation around mediaTypes[getMediaType(selectedOutput)] yields
a falsy value when absent so the existing .filter((i) => !!i) removes it.
In `@src/renderer/extensions/linearMode/mediaTypes.ts`:
- Around line 6-27: mediaTypes is populated with translated strings at module
load using t(), so translations won't update on locale changes; change
mediaTypes into a function (e.g., getMediaTypes) or a computed getter that
returns Record<string, StatItem> and calls t() at invocation time (use the same
keys and iconClass values) so consumers call getMediaTypes() during render;
update all usages of mediaTypes to call the new function (or access the getter)
so translations are evaluated with the current locale.
In `@src/renderer/extensions/linearMode/OutputHistory.vue`:
- Around line 49-56: There are two watch() calls observing the same ref
selectedIndex; consolidate them into a single watch on selectedIndex that
performs both sets of side effects (the emit('updateSelection', [...]) logic and
the additional logic currently in the second watcher around lines 71-80) so
behavior remains identical but centralized; locate the watchers by the call site
watch(selectedIndex) and ensure the new watcher uses the newValue (or
selectedIndex.value) to compute index, references outputs.media.value,
selectedOutput, and selectedIndex.value[0] <= 0 exactly as before, merging any
conditional checks and cleanup into one callback.
- Around line 295-307: The Button used for scrolling (the Button element with
`@click`="resetOutputsScroll" containing the <i class="icon-[lucide--arrow-up]
...">) is icon-only and lacks an accessible name; add an aria-label (for example
aria-label="Scroll to top" or a localized equivalent) to the Button element so
screen readers can announce its purpose, and ensure the inner <i> icon remains
decorative (aria-hidden="true" if needed).
- Around line 257-269: The <img> tag rendering images in OutputHistory.vue lacks
an alt attribute; update the img element (the one guarded by
v-if="getMediaType(output) === 'images'") to include an alt that uses available
metadata (e.g., output.metadata.filename, output.metadata.description) and fall
back to a generic string like "Generated image" or the media type; ensure the
alt text is bound (e.g., :alt="...") so it updates per output and keep the
existing click/selection logic (selectedIndex) unchanged.
- Around line 251-252: The v-for loop uses the array index as the key which can
break rendering; change the key to a stable identifier by using item.id instead
of index in the v-for over outputs.media.value (the loop that binds item and
index) so each rendered element uses item.id as the :key.
- Around line 150-151: The module currently constructs a CanvasPointer and a
plain mutable scrollOffset at module scope, which can break SSR/tests and lives
outside Vue reactivity; move the CanvasPointer instantiation into the component
lifecycle by importing and using onMounted to create pointer (e.g., instantiate
CanvasPointer(document.body) inside onMounted and store it on a component-local
ref), and replace the top-level let scrollOffset with a reactive/ref value
(e.g., const scrollOffset = ref(0)) so state is tracked by Vue and safe during
SSR; also ensure pointer cleanup in onBeforeUnmount if needed.
- Around line 35-38: Replace the plain defineProps call with a destructured
props binding that provides a default for mobile: call
withDefaults(defineProps<...>(), { mobile: false }) and destructure the returned
object into mobile and scrollResetButtonTo so the template and code can use
mobile and scrollResetButtonTo directly instead of
props.mobile/props.scrollResetButtonTo; reference the existing defineProps,
mobile, and scrollResetButtonTo symbols when making the change.
In `@src/renderer/extensions/linearMode/VideoPreview.vue`:
- Around line 9-10: width and height refs are initialized to empty strings
causing the " x " span to render before video metadata loads; update the
component (VideoPreview.vue) to hide the dimensions span until actual values
exist by either initializing width and height to null/undefined or adding a
conditional render around the span (e.g., check width && height or a computed
showDimensions) so the span only appears once metadata has populated the width
and height refs.
In `@src/renderer/extensions/vueNodes/components/NodeHeader.vue`:
- Around line 14-17: The template currently calls
useSettingStore().get('Comfy.Node.Opacity') directly which runs on every render;
extract this into the script as a memoized computed value: call
useSettingStore() once (e.g. const settingStore = useSettingStore()) and define
a computed headerOpacity that returns settingStore.get('Comfy.Node.Opacity') ??
1, then replace the template usage with headerOpacity; follow the same pattern
used in LGraphNode.vue to ensure the node header uses the computed headerOpacity
instead of invoking the store getter inline.
In `@src/renderer/extensions/vueNodes/utils/nodeStyleUtils.ts`:
- Around line 7-12: applyLightThemeColor currently calls useColorPaletteStore()
internally, creating a hidden Pinia dependency and making it impure; change it
to either (A) keep it pure by removing the store call and adding an explicit
parameter like isLightTheme (i.e., signature applyLightThemeColor(color?:
string, isLightTheme?: boolean) and use isLightTheme instead of
useColorPaletteStore().completedActivePalette.light_theme), or (B) convert it
into a composable named useApplyLightThemeColor that calls
useColorPaletteStore() and returns a function that accepts color?: string and
uses adjustColor(color, { lightness: 0.5 }) only when the store's
completedActivePalette.light_theme is true; update all callers to pass the new
parameter or to use the composable accordingly.
In `@src/utils/dateTimeUtil.ts`:
- Around line 79-91: The function formatDuration lacks an explicit return type
and returns an empty string for a valid zero duration; add an explicit return
type of string to the function signature and ensure zero is handled by returning
"0s" (e.g., if durationSeconds === 0 or if parts is empty after composing parts,
return "0s"). Keep the existing parameter type (durationSeconds?: number) and
update only the signature and final return behavior in formatDuration.
In `@src/views/LinearView.vue`:
- Line 119: The two empty <div /> elements in LinearView.vue (currently at the
spots shown around lines 119 and 179) are ambiguous; either remove them if
unused, or replace them with a semantic spacer and explanatory comment. If they
are layout anchors, change them to <div class="spacer" aria-hidden="true"></div>
(or the project's spacer utility class) and add a brief inline comment like <!--
layout spacer for X -->; otherwise delete the empty tags to avoid dead markup.
Ensure any introduced class is defined in the component styles or an existing
stylesheet.
- Around line 74-81: Duplicate inline handlers for OutputHistory's
update-selection should be extracted into a single reusable handler; create a
method named handleUpdateSelection that accepts ([item, output, canShow]) and
sets selectedItem, selectedOutput, canShowPreview and hasPreview = false, then
replace the three inline `@update-selection`="([item, output, canShow]) => { ...
}" occurrences with `@update-selection`="handleUpdateSelection". Ensure the
handler is declared in the component's methods (Options API) or returned from
setup (Composition API) so the template bindings resolve correctly.
| <script setup lang="ts"> | ||
| import Button from '@/components/ui/button/Button.vue' | ||
| import { t } from '@/i18n' | ||
| import { useCanvasStore } from '@/renderer/core/canvas/canvasStore' | ||
| import { useCommandStore } from '@/stores/commandStore' | ||
|
|
||
| const canvasStore = useCanvasStore() | ||
| </script> | ||
| <template> | ||
| <div class="p-1 bg-secondary-background rounded-lg w-10"> | ||
| <Button | ||
| size="icon" | ||
| :title="t('linearMode.linearMode')" | ||
| :variant="canvasStore.linearMode ? 'inverted' : 'secondary'" | ||
| @click="useCommandStore().execute('Comfy.ToggleLinear')" | ||
| > | ||
| <i class="icon-[lucide--panels-top-left]" /> | ||
| </Button> | ||
| <Button | ||
| size="icon" | ||
| :title="t('linearMode.graphMode')" | ||
| :variant="canvasStore.linearMode ? 'secondary' : 'inverted'" | ||
| @click="useCommandStore().execute('Comfy.ToggleLinear')" | ||
| > | ||
| <i class="icon-[comfy--workflow]" /> | ||
| </Button> | ||
| </div> |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
rg -n "linearMode" src/locales/ -A 2 -B 2Repository: Comfy-Org/ComfyUI_frontend
Length of output: 3470
🏁 Script executed:
# Check how t() is imported and used in other components
rg "from '@/i18n'" src/components/ | head -20Repository: Comfy-Org/ComfyUI_frontend
Length of output: 1377
🏁 Script executed:
# Check if useI18n is used elsewhere
rg "useI18n" src/components/ | head -20Repository: Comfy-Org/ComfyUI_frontend
Length of output: 1552
Use useI18n() composable instead of direct t import to follow composition API conventions.
While the i18n keys linearMode.linearMode and linearMode.graphMode exist and are correct, the component should follow the established pattern used throughout the sidebar (ComfyMenuButton.vue, SidebarIcon.vue, etc.) and the coding guidelines:
import { useI18n } from 'vue-i18n'
const { t } = useI18n()
Replace the direct import import { t } from '@/i18n' with the VueUse pattern above. This is consistent with other sidebar components and aligns with the coding guideline: "Use VueUse function for useI18n in composition API for string literals."
The rest of the component implementation is clean: proper use of Composition API, no unnecessary style blocks, correct command binding, and clear toggle logic.
🤖 Prompt for AI Agents
In `@src/components/sidebar/ModeToggle.vue` around lines 1 - 27, The component
imports t directly from '@/i18n' instead of using the Vue i18n composable;
replace the top-level import "import { t } from '@/i18n'" with "import { useI18n
} from 'vue-i18n'" and inside the <script setup> call const { t } = useI18n() so
the template uses the composable t; keep existing canvasStore and
useCommandStore usage (symbols: ModeToggle.vue, useI18n, const { t } =
useI18n(), canvasStore, useCommandStore) and remove the old direct import.
| <i | ||
| v-else-if="workflowOption.workflow.activeState?.extra?.linearMode" | ||
| class="icon-[lucide--panels-top-left] bg-primary-background" | ||
| /> |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
rg -n "linearMode" src/locales/en/main.jsonRepository: Comfy-Org/ComfyUI_frontend
Length of output: 130
🏁 Script executed:
cat -n src/components/topbar/WorkflowTab.vue | head -100Repository: Comfy-Org/ComfyUI_frontend
Length of output: 3677
🏁 Script executed:
cat -n src/components/topbar/WorkflowTab.vue | tail -150Repository: Comfy-Org/ComfyUI_frontend
Length of output: 5359
Improve icon accessibility with title attribute, but fix aria-hidden value.
The icon is purely decorative and lacks an accessible label. Add a title attribute for tooltip support. However, if the icon is decorative (which it is), it should have aria-hidden="true" rather than aria-hidden="false", or omit the aria-hidden attribute entirely if only the title is needed.
🔧 Suggested fix
<i
v-else-if="workflowOption.workflow.activeState?.extra?.linearMode"
class="icon-[lucide--panels-top-left] bg-primary-background"
+ :title="t('linearMode.linearMode')"
+ aria-hidden="true"
/>🤖 Prompt for AI Agents
In `@src/components/topbar/WorkflowTab.vue` around lines 19 - 22, Add a
descriptive title to the decorative icon in WorkflowTab.vue and correct its
accessibility flag: update the <i> element rendered by the v-else-if branch (the
element with class "icon-[lucide--panels-top-left] bg-primary-background") to
include a title attribute like title="Linear mode" and ensure it uses
aria-hidden="true" (or omit aria-hidden entirely) since it is decorative.
| <PopoverTrigger as-child> | ||
| <slot name="button"> | ||
| <Button size="icon"> | ||
| <i :class="icon ?? 'icon-[lucide--ellipsis]'" /> | ||
| </Button> | ||
| </slot> | ||
| </PopoverTrigger> |
There was a problem hiding this comment.
Add aria-label for icon-only button accessibility.
The default trigger button contains only an icon without visible text. Per accessibility guidelines, icon-only buttons need an aria-label to provide an accessible name for screen readers.
♻️ Suggested fix
<slot name="button">
- <Button size="icon">
+ <Button size="icon" aria-label="Open menu">
<i :class="icon ?? 'icon-[lucide--ellipsis]'" />
</Button>
</slot>Note: Consider using vue-i18n for the aria-label string if this component is user-facing.
📝 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.
| <PopoverTrigger as-child> | |
| <slot name="button"> | |
| <Button size="icon"> | |
| <i :class="icon ?? 'icon-[lucide--ellipsis]'" /> | |
| </Button> | |
| </slot> | |
| </PopoverTrigger> | |
| <PopoverTrigger as-child> | |
| <slot name="button"> | |
| <Button size="icon" aria-label="Open menu"> | |
| <i :class="icon ?? 'icon-[lucide--ellipsis]'" /> | |
| </Button> | |
| </slot> | |
| </PopoverTrigger> |
🤖 Prompt for AI Agents
In `@src/components/ui/Popover.vue` around lines 26 - 32, The icon-only default
trigger lacks an accessible name—add an aria-label to the <Button> in the
default slot: introduce a prop (e.g., ariaLabel) on this Popover component
(props: ariaLabel: { type: String, default: null }) and bind it to the Button as
:aria-label="ariaLabel ?? $t('popover.openOptions')" (or a sensible default
string if i18n isn't available); keep the binding next to size="icon" inside the
slot so <PopoverTrigger> / slot="button" / Button / icon remain unchanged
otherwise.
| whenever(feedbackRef, () => { | ||
| const scriptEl = document.createElement('script') | ||
| scriptEl.src = '//embed.typeform.com/next/embed.js' | ||
| feedbackRef.value?.appendChild(scriptEl) | ||
| }) |
There was a problem hiding this comment.
Potential multiple script injections on repeated popover opens.
The whenever callback runs each time feedbackRef transitions from falsy to truthy. If the popover is opened multiple times, this could inject multiple <script> elements. Consider guarding against duplicate injections:
♻️ Suggested fix
+const scriptInjected = ref(false)
+
whenever(feedbackRef, () => {
+ if (scriptInjected.value) return
const scriptEl = document.createElement('script')
scriptEl.src = '//embed.typeform.com/next/embed.js'
feedbackRef.value?.appendChild(scriptEl)
+ scriptInjected.value = true
})📝 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.
| whenever(feedbackRef, () => { | |
| const scriptEl = document.createElement('script') | |
| scriptEl.src = '//embed.typeform.com/next/embed.js' | |
| feedbackRef.value?.appendChild(scriptEl) | |
| }) | |
| const scriptInjected = ref(false) | |
| whenever(feedbackRef, () => { | |
| if (scriptInjected.value) return | |
| const scriptEl = document.createElement('script') | |
| scriptEl.src = '//embed.typeform.com/next/embed.js' | |
| feedbackRef.value?.appendChild(scriptEl) | |
| scriptInjected.value = true | |
| }) |
🤖 Prompt for AI Agents
In `@src/components/ui/TypeformPopoverButton.vue` around lines 14 - 18, The
whenever callback currently appends a new script every time feedbackRef becomes
truthy, risking duplicate injections; modify the callback in whenever to first
check for an existing script with the same src (or a unique id) inside
feedbackRef.value (or document) before creating/appending scriptEl (e.g., use
feedbackRef.value.querySelector('script[src="//embed.typeform.com/next/embed.js"]')
or document.getElementById) and only create/append the script if that query
returns null, ensuring the Typeform embed script is injected once.
| <template #button> | ||
| <Button variant="inverted" class="rounded-full size-12"> | ||
| <i class="icon-[lucide--circle-question-mark] size-6" /> | ||
| </Button> | ||
| </template> |
There was a problem hiding this comment.
Add aria-label for icon-only help button.
The button contains only an icon without visible text. For screen reader accessibility, add an aria-label. As per coding guidelines, use vue-i18n for the label.
♻️ Suggested fix
+<script setup lang="ts">
+import { useI18n } from 'vue-i18n'
+// ... existing imports
+
+const { t } = useI18n()
+// ... rest of script
+</script>
+
<template `#button`>
- <Button variant="inverted" class="rounded-full size-12">
+ <Button variant="inverted" class="rounded-full size-12" :aria-label="t('feedback.openFeedback')">
<i class="icon-[lucide--circle-question-mark] size-6" />
</Button>
</template>🤖 Prompt for AI Agents
In `@src/components/ui/TypeformPopoverButton.vue` around lines 22 - 26, The
icon-only help Button in the template (`#button`) lacks an accessible name; update
the Button element in TypeformPopoverButton.vue to include an aria-label bound
to a vue-i18n translation (e.g., $t('typeform.help_button') or similar key) so
screen readers get a descriptive label; add the i18n key to your locale messages
and use the aria-label binding on the Button component instance.
| :style="{ | ||
| backgroundColor: applyLightThemeColor(nodeData?.color), | ||
| opacity: useSettingStore().get('Comfy.Node.Opacity') ?? 1 | ||
| }" |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Avoid calling useSettingStore() directly in the template.
Calling useSettingStore().get('Comfy.Node.Opacity') in the template executes on every render without memoization. For a component rendered per node, this causes unnecessary overhead.
Extract this to a computed property in the script section, consistent with how LGraphNode.vue handles it (see line 231).
♻️ Proposed fix
Add computed property in script:
const settingStore = useSettingStore()
const headerOpacity = computed(() => settingStore.get('Comfy.Node.Opacity') ?? 1)Update template:
:style="{
backgroundColor: applyLightThemeColor(nodeData?.color),
- opacity: useSettingStore().get('Comfy.Node.Opacity') ?? 1
+ opacity: headerOpacity
}"🤖 Prompt for AI Agents
In `@src/renderer/extensions/vueNodes/components/NodeHeader.vue` around lines 14 -
17, The template currently calls useSettingStore().get('Comfy.Node.Opacity')
directly which runs on every render; extract this into the script as a memoized
computed value: call useSettingStore() once (e.g. const settingStore =
useSettingStore()) and define a computed headerOpacity that returns
settingStore.get('Comfy.Node.Opacity') ?? 1, then replace the template usage
with headerOpacity; follow the same pattern used in LGraphNode.vue to ensure the
node header uses the computed headerOpacity instead of invoking the store getter
inline.
| export function applyLightThemeColor(color?: string): string { | ||
| if (!color) return '' | ||
|
|
||
| if (!useColorPaletteStore().completedActivePalette.light_theme) return color | ||
|
|
||
| return adjustColor(color, { lightness: 0.5 }) |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Calling Pinia store inside a utility function creates a hidden dependency and reduces testability.
The function now internally calls useColorPaletteStore(), making it impure and tightly coupled to Vue's reactivity context. This pattern:
- Makes unit testing harder (requires mocking the store)
- Could fail if called outside Vue's setup/template context
- Hides the light_theme dependency from callers
Consider either keeping this as a pure function (passing isLightTheme as before) or converting it to a composable that returns a reactive computed value.
♻️ Option A: Keep as pure function
-export function applyLightThemeColor(color?: string): string {
- if (!color) return ''
-
- if (!useColorPaletteStore().completedActivePalette.light_theme) return color
-
+export function applyLightThemeColor(color: string | undefined, isLightTheme: boolean): string {
+ if (!color) return ''
+ if (!isLightTheme) return color
return adjustColor(color, { lightness: 0.5 })
}♻️ Option B: Convert to composable
export function useApplyLightThemeColor() {
const colorPaletteStore = useColorPaletteStore()
return (color?: string): string => {
if (!color) return ''
if (!colorPaletteStore.completedActivePalette.light_theme) return color
return adjustColor(color, { lightness: 0.5 })
}
}🤖 Prompt for AI Agents
In `@src/renderer/extensions/vueNodes/utils/nodeStyleUtils.ts` around lines 7 -
12, applyLightThemeColor currently calls useColorPaletteStore() internally,
creating a hidden Pinia dependency and making it impure; change it to either (A)
keep it pure by removing the store call and adding an explicit parameter like
isLightTheme (i.e., signature applyLightThemeColor(color?: string,
isLightTheme?: boolean) and use isLightTheme instead of
useColorPaletteStore().completedActivePalette.light_theme), or (B) convert it
into a composable named useApplyLightThemeColor that calls
useColorPaletteStore() and returns a function that accepts color?: string and
uses adjustColor(color, { lightness: 0.5 }) only when the store's
completedActivePalette.light_theme is true; update all callers to pass the new
parameter or to use the composable accordingly.
| export function formatDuration(durationSeconds?: number) { | ||
| if (durationSeconds == undefined) return '' | ||
| const hours = (durationSeconds / 60 ** 2) | 0 | ||
| const minutes = ((durationSeconds % 60 ** 2) / 60) | 0 | ||
| const seconds = (durationSeconds % 60) | 0 | ||
| const parts = [] | ||
|
|
||
| if (hours > 0) parts.push(`${hours}h`) | ||
| if (minutes > 0) parts.push(`${minutes}m`) | ||
| if (seconds > 0) parts.push(`${seconds}s`) | ||
|
|
||
| return parts.join(' ') | ||
| } |
There was a problem hiding this comment.
Add explicit return type and handle zero-duration edge case.
Two issues:
- Missing explicit return type annotation for TypeScript clarity.
- When
durationSecondsis0, all conditions fail and the function returns''. For a valid zero-duration, returning'0s'is likely more appropriate.
Proposed fix
-export function formatDuration(durationSeconds?: number) {
- if (durationSeconds == undefined) return ''
+export function formatDuration(durationSeconds?: number): string {
+ if (durationSeconds === undefined) return ''
const hours = (durationSeconds / 60 ** 2) | 0
const minutes = ((durationSeconds % 60 ** 2) / 60) | 0
const seconds = (durationSeconds % 60) | 0
const parts = []
if (hours > 0) parts.push(`${hours}h`)
if (minutes > 0) parts.push(`${minutes}m`)
- if (seconds > 0) parts.push(`${seconds}s`)
+ if (seconds > 0 || parts.length === 0) parts.push(`${seconds}s`)
return parts.join(' ')
}📝 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.
| export function formatDuration(durationSeconds?: number) { | |
| if (durationSeconds == undefined) return '' | |
| const hours = (durationSeconds / 60 ** 2) | 0 | |
| const minutes = ((durationSeconds % 60 ** 2) / 60) | 0 | |
| const seconds = (durationSeconds % 60) | 0 | |
| const parts = [] | |
| if (hours > 0) parts.push(`${hours}h`) | |
| if (minutes > 0) parts.push(`${minutes}m`) | |
| if (seconds > 0) parts.push(`${seconds}s`) | |
| return parts.join(' ') | |
| } | |
| export function formatDuration(durationSeconds?: number): string { | |
| if (durationSeconds === undefined) return '' | |
| const hours = (durationSeconds / 60 ** 2) | 0 | |
| const minutes = ((durationSeconds % 60 ** 2) / 60) | 0 | |
| const seconds = (durationSeconds % 60) | 0 | |
| const parts = [] | |
| if (hours > 0) parts.push(`${hours}h`) | |
| if (minutes > 0) parts.push(`${minutes}m`) | |
| if (seconds > 0 || parts.length === 0) parts.push(`${seconds}s`) | |
| return parts.join(' ') | |
| } |
🤖 Prompt for AI Agents
In `@src/utils/dateTimeUtil.ts` around lines 79 - 91, The function formatDuration
lacks an explicit return type and returns an empty string for a valid zero
duration; add an explicit return type of string to the function signature and
ensure zero is handled by returning "0s" (e.g., if durationSeconds === 0 or if
parts is empty after composing parts, return "0s"). Keep the existing parameter
type (durationSeconds?: number) and update only the signature and final return
behavior in formatDuration.
| @update-selection=" | ||
| ([item, output, canShow]) => { | ||
| selectedItem = item | ||
| selectedOutput = output | ||
| canShowPreview = canShow | ||
| hasPreview = false | ||
| } | ||
| " |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Duplicate update-selection handlers.
The same handler logic is repeated three times for OutputHistory's update-selection event. Consider extracting to a reusable function.
♻️ Proposed refactor
+function handleSelectionUpdate([item, output, canShow]: [AssetItem | undefined, ResultItemImpl | undefined, boolean]) {
+ selectedItem.value = item
+ selectedOutput.value = output
+ canShowPreview.value = canShow
+ hasPreview.value = false
+}
// Then in template:
<OutputHistory
...
- `@update-selection`="
- ([item, output, canShow]) => {
- selectedItem = item
- selectedOutput = output
- canShowPreview = canShow
- hasPreview = false
- }
- "
+ `@update-selection`="handleSelectionUpdate"
/>Also applies to: 104-111, 170-177
🤖 Prompt for AI Agents
In `@src/views/LinearView.vue` around lines 74 - 81, Duplicate inline handlers for
OutputHistory's update-selection should be extracted into a single reusable
handler; create a method named handleUpdateSelection that accepts ([item,
output, canShow]) and sets selectedItem, selectedOutput, canShowPreview and
hasPreview = false, then replace the three inline `@update-selection`="([item,
output, canShow]) => { ... }" occurrences with
`@update-selection`="handleUpdateSelection". Ensure the handler is declared in the
component's methods (Options API) or returned from setup (Composition API) so
the template bindings resolve correctly.
| :toast-to="unrefElement(bottomLeftRef) ?? undefined" | ||
| :notes-to="unrefElement(topLeftRef) ?? undefined" | ||
| /> | ||
| <div /> |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Empty <div /> elements without clear purpose.
Lines 119 and 179 contain empty <div /> elements. If these serve as spacers or layout anchors, add a comment explaining their purpose. Otherwise, consider removing them.
🤖 Prompt for AI Agents
In `@src/views/LinearView.vue` at line 119, The two empty <div /> elements in
LinearView.vue (currently at the spots shown around lines 119 and 179) are
ambiguous; either remove them if unused, or replace them with a semantic spacer
and explanatory comment. If they are layout anchors, change them to <div
class="spacer" aria-hidden="true"></div> (or the project's spacer utility class)
and add a brief inline comment like <!-- layout spacer for X -->; otherwise
delete the empty tags to avoid dead markup. Ensure any introduced class is
defined in the component styles or an existing stylesheet.
…prices (#8059) Backport of #8046 to `cloud/1.37` Automatically created by backport workflow. ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-8059-backport-cloud-1-37-feat-price-badges-add-ByteDance-SeeDance-1-5-prices-2e96d73d3650817894e2e7350ccfb8c5) by [Unito](https://www.unito.io) Co-authored-by: Alexander Piskun <13381981+bigcat88@users.noreply.github.com>
Backport of #8054 to `cloud/1.37` Automatically created by backport workflow. ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-8072-backport-cloud-1-37-Linear-mode-bug-fixes-2e96d73d365081dfa542f08405043203) by [Unito](https://www.unito.io) Co-authored-by: AustinMroz <austin@comfy.org>
There was a problem hiding this comment.
Actionable comments posted: 8
🤖 Fix all issues with AI agents
In `@src/composables/useCoreCommands.ts`:
- Around line 1238-1243: The anonymous command function toggles linear mode but
assumes app.rootGraph.extra exists; mirror workflowService.ts by defensively
initializing app.rootGraph.extra using the nullish-coalescing assignment before
setting linearMode. In the function block (function: () => { ... }) add an
initialization for app.rootGraph.extra (e.g., app.rootGraph.extra ??= {}) then
set app.rootGraph.extra.linearMode = newMode and update canvasStore.linearMode
and call workflowStore.activeWorkflow?.changeTracker?.checkState() as currently
done.
In `@src/renderer/extensions/linearMode/LinearControls.vue`:
- Around line 75-88: partitionedNodes currently mutates reactive objects by
assigning to widget.options inside the loop; instead produce immutable copies:
when building parts (or specifically parts[0]), map over those note nodes and
their widgets to return new noteNode objects with a new widgets array where each
widget is shallow-copied and its options replaced with a copy that sets
read_only: true; update the returned parts to use these copied note nodes so you
never assign to widget.options on reactive objects (refer to partitionedNodes,
parts, noteNode, widget, nodeToNodeData, graphNodes).
In `@src/renderer/extensions/linearMode/LinearPreview.vue`:
- Around line 29-35: The props destructuring in LinearPreview.vue should use Vue
3.5 reactive pattern: replace the direct defineProps destructuring with a props
object created via withDefaults(defineProps<...>()) to supply defaults for
latentPreview and mobile, then convert to refs using toRefs/toRef so template
bindings remain reactive; update references to runButtonClick, selectedItem,
selectedOutput, latentPreview and mobile to use those refs (or .value in script)
instead of the current plain variables.
- Around line 154-178: The template uses non-null assertions on selectedOutput
(selectedOutput!) which can throw if selectedOutput is undefined even when
getMediaType(selectedOutput) returns a value; remove the non-null assertions and
guard usage of selectedOutput by either (a) verifying selectedOutput exists
before rendering each branch (compute mediaType = getMediaType(selectedOutput)
and use v-if checks that include selectedOutput) or (b) switch to optional
chaining for values (use selectedOutput?.url / :model-url="selectedOutput?.url")
and ensure ImagePreview/VideoPreview/Preview3d branches only render when
selectedOutput is present; update all references to selectedOutput! accordingly
(selectedOutput, getMediaType, ImagePreview, VideoPreview, Preview3d) so runtime
null/undefined access is avoided.
In `@src/renderer/extensions/linearMode/OutputHistory.vue`:
- Line 45: Move the defineExpose call so it appears after the onWheel function
definition to improve clarity; specifically, locate the defineExpose({ onWheel
}) call and relocate it below the onWheel function declaration in
OutputHistory.vue so the exposed symbol is defined before being exported.
In `@src/renderer/extensions/linearMode/Preview3d.vue`:
- Line 29: The template uses `@resize` on a div which never fires; replace that
with a ResizeObserver and call the same handler: in Preview3d.vue remove the
`@resize` binding and in the script setup (or component) import/use VueUse's
useResizeObserver (or window.addEventListener('resize') as an alternative) to
observe the div element and invoke viewer.handleResize whenever size changes;
ensure you reference the div's ref (e.g., containerRef) when calling
useResizeObserver and clean up the observer on unmount.
- Around line 15-19: The watcher initializes the 3D viewer via
viewer.initializeStandaloneViewer but never calls the composable's cleanup(),
causing WebGL/resource leaks; add a component unmount hook (onBeforeUnmount or
onUnmounted) to call the cleanup() method from useLoad3dViewer, and ensure any
previous viewer state is cleaned before re-initializing (e.g., call cleanup() at
the start of the watcher or before calling viewer.initializeStandaloneViewer) so
containerRef, modelUrl and viewer lifecycle are properly disposed.
In `@src/views/LinearView.vue`:
- Around line 88-92: The external anchor in LinearView.vue (the <a> element
rendering t('linearMode.beta') next to TypeformPopoverButton) must open in a new
tab and avoid window.opener risks; add target="_blank" and rel="noopener
noreferrer" to that <a> tag so the Typeform link is opened securely.
♻️ Duplicate comments (15)
src/renderer/extensions/linearMode/LinearPreview.vue (3)
60-66: Empty objects pass through the filter.When
mediaTypes[getMediaType(selectedOutput)]is undefined, the fallback?? {}creates an empty object that passes the!!icheck. This may render empty stat items in the UI.🐛 Proposed fix
return [ { content: formatTime(selectedItem.created_at) }, { content: formatDuration(user_metadata.executionTimeInSeconds) }, allOutputs && { content: t('g.asset', allOutputs.length) }, - (selectedOutput && mediaTypes[getMediaType(selectedOutput)]) ?? {} - ].filter((i) => !!i) + selectedOutput ? mediaTypes[getMediaType(selectedOutput)] : undefined + ].filter((i): i is StatItem => !!i && !!i.content) })
87-96: Race condition risk with hardcoded timeout.The
FIXMEcomment acknowledges this issue. The 500ms timeout is a fragile workaround that may fail under varying load conditions, and seeds still fail to update properly as noted. Consider implementing an event-driven approach that waits for the graph to be fully loaded.
124-133: Icon-only button missing accessible label.The download button contains only an icon without visible text. Add an
aria-labelto provide an accessible name. Based on learnings, icon-only buttons requirearia-label.♿ Proposed fix
<Button size="icon" + :aria-label="t('g.download')" `@click`=" () => { if (selectedOutput?.url) downloadFile(selectedOutput.url) } " > <i class="icon-[lucide--download]" /> </Button>src/renderer/extensions/linearMode/LinearControls.vue (4)
53-74: Side effect in mapping function mutates source data.The
nodeToNodeDatafunction mutateswidget.slotMetadataduring the mapping operation on line 63. This modifies the original widget objects, which can cause unexpected behavior. Consider cloning if mutation is needed.🐛 Proposed fix
function nodeToNodeData(node: LGraphNode) { const dropIndicator = node.type !== 'LoadImage' ? undefined : { iconClass: 'icon-[lucide--image]', label: t('linearMode.dragAndDropImage'), onClick: () => node.widgets?.[1]?.callback?.(undefined) } const nodeData = extractVueNodeData(node) - for (const widget of nodeData.widgets ?? []) widget.slotMetadata = undefined + const widgets = nodeData.widgets?.map((widget) => ({ + ...widget, + slotMetadata: undefined + })) return { ...nodeData, + widgets, //note lastNodeErrors uses exeuctionid, node.id is execution for root hasErrors: !!executionStore.lastNodeErrors?.[node.id], dropIndicator, onDragDrop: node.onDragDrop, onDragOver: node.onDragOver } }
154-162: Icon-only interrupt buttons missing accessible labels.Both interrupt buttons (mobile at lines 154-162 and desktop at lines 265-273) contain only icons. Add
aria-labelfor accessibility.♿ Proposed fix for both buttons
<Button v-if="!executionStore.isIdle" variant="destructive" size="lg" class="w-10 p-2" + :aria-label="t('menu.interrupt')" `@click`="commandStore.execute('Comfy.Interrupt')" > <i class="icon-[lucide--x]" /> </Button>Also applies to: 265-273
204-204: Dead code withv-if="false".This button will never be shown. Remove it or gate it behind a proper feature flag.
294-298: Dead Teleport block should be removed.This
<Teleport v-if="false">will never render. Remove the dead code or document why it's preserved for future use.src/renderer/extensions/linearMode/OutputHistory.vue (6)
35-38: Consider Vue 3.5 style reactive props destructuring.Per coding guidelines, prefer destructuring props with defaults directly.
♻️ Suggested refactor
-defineProps<{ - scrollResetButtonTo?: string | HTMLElement - mobile?: boolean -}>() +const { scrollResetButtonTo, mobile = false } = defineProps<{ + scrollResetButtonTo?: string | HTMLElement + mobile?: boolean +}>()
49-56: Consolidate duplicate watches onselectedIndex.Two separate
watch()calls monitor the same ref. Consolidating them improves maintainability and makes the reactive flow easier to follow.♻️ Suggested consolidation
watch(selectedIndex, () => { const [index, key] = selectedIndex.value emit('updateSelection', [ outputs.media.value[index], selectedOutput.value, selectedIndex.value[0] <= 0 ]) -}) -// ... other code ... - -watch(selectedIndex, () => { - const [index, key] = selectedIndex.value + // Scroll selected output into view if (!outputsRef.value) return - const outputElement = outputsRef.value?.children?.[index]?.children?.[key] if (!outputElement) return - - //container: 'nearest' is nice, but bleeding edge and chrome only outputElement.scrollIntoView({ block: 'nearest' }) })Also applies to: 71-80
150-151: SSR/test incompatibility and mutable state outside Vue's reactive system.
CanvasPointer(document.body)instantiated at module scope may fail during SSR or unit tests wheredocumentis unavailable. Additionally,scrollOffsetas a plainletvariable is mutable state outside Vue's reactive system.🐛 Suggested fix
+import { onMounted } from 'vue' + -let pointer = new CanvasPointer(document.body) -let scrollOffset = 0 +const scrollOffset = ref(0) +let pointer: CanvasPointer | null = null + +onMounted(() => { + pointer = new CanvasPointer(document.body) +}) + function onWheel(e: WheelEvent) { if (!e.ctrlKey && !e.metaKey) return e.preventDefault() e.stopPropagation() - if (!pointer.isTrackpadGesture(e)) { + if (!pointer?.isTrackpadGesture(e)) { if (e.deltaY > 0) gotoNextOutput() else gotoPreviousOutput() return } - scrollOffset += e.deltaY - while (scrollOffset >= 60) { - scrollOffset -= 60 + scrollOffset.value += e.deltaY + while (scrollOffset.value >= 60) { + scrollOffset.value -= 60 gotoNextOutput() } - while (scrollOffset <= -60) { - scrollOffset += 60 + while (scrollOffset.value <= -60) { + scrollOffset.value += 60 gotoPreviousOutput() } }
250-252: Use stable identifier instead of array index forv-forkey.Using
indexas the key can cause rendering issues when items are added, removed, or reordered. Useitem.idinstead.🐛 Suggested fix
<section v-for="(item, index) in outputs.media.value" - :key="index" + :key="item.id" data-testid="linear-job"
257-269: Addaltattribute for accessibility.Images should have descriptive
alttext for screen readers.♿ Suggested fix
<img v-if="getMediaType(output) === 'images'" :class="..." :src="output.url" + :alt="`Output ${index + 1}-${key + 1}`" `@click`="selectedIndex = [index, key]" />
296-308: Addaria-labelfor icon-only button.The scroll-to-top button contains only an icon. Add an accessible label.
♿ Suggested fix
<Button :class="..." + :aria-label="t('linearMode.scrollToTop')" `@click`="resetOutputsScroll" > <i class="icon-[lucide--arrow-up] size-4 bg-base-background" /> </Button>src/views/LinearView.vue (2)
77-84: Duplicate@update-selectionhandlers.The same handler logic is repeated three times for
OutputHistory'supdate-selectionevent. Extract to a reusable function for maintainability.♻️ Proposed refactor
+function handleSelectionUpdate([item, output, canShow]: [AssetItem | undefined, ResultItemImpl | undefined, boolean]) { + selectedItem.value = item + selectedOutput.value = output + canShowPreview.value = canShow + hasPreview.value = false +} + // Then in template, replace all three instances: <OutputHistory ... - `@update-selection`=" - ([item, output, canShow]) => { - selectedItem = item - selectedOutput = output - canShowPreview = canShow - hasPreview = false - } - " + `@update-selection`="handleSelectionUpdate" />Also applies to: 110-117, 175-182
125-125: Empty<div />elements without clear purpose.Lines 125 and 184 contain empty
<div />elements withinSplitterPanel. If these serve as layout spacers or anchors, add a comment explaining their purpose. Otherwise, consider removing them.Also applies to: 184-184
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (9)
src/components/topbar/TopMenuHelpButton.vuesrc/composables/useCoreCommands.tssrc/locales/en/commands.jsonsrc/locales/en/main.jsonsrc/renderer/extensions/linearMode/LinearControls.vuesrc/renderer/extensions/linearMode/LinearPreview.vuesrc/renderer/extensions/linearMode/OutputHistory.vuesrc/renderer/extensions/linearMode/Preview3d.vuesrc/views/LinearView.vue
🧰 Additional context used
📓 Path-based instructions (14)
src/**/*.{vue,ts}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
src/**/*.{vue,ts}: Leverage VueUse functions for performance-enhancing styles
Implement proper error handling
Use vue-i18n in composition API for any string literals. Place new translation entries in src/locales/en/main.json
Files:
src/composables/useCoreCommands.tssrc/renderer/extensions/linearMode/Preview3d.vuesrc/renderer/extensions/linearMode/LinearPreview.vuesrc/renderer/extensions/linearMode/OutputHistory.vuesrc/renderer/extensions/linearMode/LinearControls.vuesrc/views/LinearView.vuesrc/components/topbar/TopMenuHelpButton.vue
src/**/*.ts
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
src/**/*.ts: Use es-toolkit for utility functions
Use TypeScript for type safety
src/**/*.ts: Derive component types usingvue-component-type-helpers(ComponentProps,ComponentSlots) instead of separate type files
Use es-toolkit for utility functions
Minimize the surface area (exported values) of each module and composable
Favor pure functions, especially testable ones
Files:
src/composables/useCoreCommands.ts
src/**/{services,composables}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (src/CLAUDE.md)
src/**/{services,composables}/**/*.{ts,tsx}: Useapi.apiURL()for backend endpoints instead of constructing URLs directly
Useapi.fileURL()for static file access instead of constructing URLs directly
Files:
src/composables/useCoreCommands.ts
src/**/*.{ts,tsx,vue}
📄 CodeRabbit inference engine (src/CLAUDE.md)
src/**/*.{ts,tsx,vue}: Sanitize HTML with DOMPurify to prevent XSS attacks
Avoid using@ts-expect-error; use proper TypeScript types instead
Use es-toolkit for utility functions instead of other utility libraries
Implement proper TypeScript types throughout the codebase
src/**/*.{ts,tsx,vue}: Use separateimport typestatements instead of inlinetypein mixed imports
Apply Prettier formatting with 2-space indentation, single quotes, no trailing semicolons, 80-character width
Sort and group imports by plugin, runpnpm formatbefore committing
Never useanytype - use proper TypeScript types
Never useas anytype assertions - fix the underlying type issue
Write code that is expressive and self-documenting - avoid unnecessary comments
Do not add or retain redundant comments - clean as you go
Avoid mutable state - prefer immutability and assignment at point of declaration
Watch out for Code Smells and refactor to avoid them
Files:
src/composables/useCoreCommands.tssrc/renderer/extensions/linearMode/Preview3d.vuesrc/renderer/extensions/linearMode/LinearPreview.vuesrc/renderer/extensions/linearMode/OutputHistory.vuesrc/renderer/extensions/linearMode/LinearControls.vuesrc/views/LinearView.vuesrc/components/topbar/TopMenuHelpButton.vue
src/**/{composables,components}/**/*.{ts,tsx,vue}
📄 CodeRabbit inference engine (src/CLAUDE.md)
Clean up subscriptions in state management to prevent memory leaks
Files:
src/composables/useCoreCommands.tssrc/components/topbar/TopMenuHelpButton.vue
src/**/*.{vue,ts,tsx}
📄 CodeRabbit inference engine (src/CLAUDE.md)
Follow Vue 3 composition API style guide
Files:
src/composables/useCoreCommands.tssrc/renderer/extensions/linearMode/Preview3d.vuesrc/renderer/extensions/linearMode/LinearPreview.vuesrc/renderer/extensions/linearMode/OutputHistory.vuesrc/renderer/extensions/linearMode/LinearControls.vuesrc/views/LinearView.vuesrc/components/topbar/TopMenuHelpButton.vue
src/**/{components,composables}/**/*.{ts,tsx,vue}
📄 CodeRabbit inference engine (src/CLAUDE.md)
Use vue-i18n for ALL user-facing strings by adding them to
src/locales/en/main.json
Files:
src/composables/useCoreCommands.tssrc/components/topbar/TopMenuHelpButton.vue
src/composables/**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
Name composables as
useXyz.ts(e.g.,useForm.ts)
Files:
src/composables/useCoreCommands.ts
src/**/*.{ts,vue}
📄 CodeRabbit inference engine (AGENTS.md)
src/**/*.{ts,vue}: Usereffor reactive state,computed()for derived values, andwatch/watchEffectfor side effects in Composition API
Avoid usingrefwithwatchif acomputedwould suffice - minimize refs and derived state
Useprovide/injectfor dependency injection only when simpler alternatives (Store or shared composable) won't work
Leverage VueUse functions for performance-enhancing composables
Use VueUse function for useI18n in composition API for string literals
Files:
src/composables/useCoreCommands.tssrc/renderer/extensions/linearMode/Preview3d.vuesrc/renderer/extensions/linearMode/LinearPreview.vuesrc/renderer/extensions/linearMode/OutputHistory.vuesrc/renderer/extensions/linearMode/LinearControls.vuesrc/views/LinearView.vuesrc/components/topbar/TopMenuHelpButton.vue
src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
src/**/*.{ts,tsx}: Keep functions short and functional
Minimize nesting (if statements, for loops, etc.)
Use function declarations instead of function expressions when possible
Files:
src/composables/useCoreCommands.ts
src/**/*.vue
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
src/**/*.vue: Use the Vue 3 Composition API instead of the Options API when writing Vue components (exception: when overriding or extending PrimeVue components for compatibility)
Use setup() function for component logic
Utilize ref and reactive for reactive state
Implement computed properties with computed()
Use watch and watchEffect for side effects
Implement lifecycle hooks with onMounted, onUpdated, etc.
Utilize provide/inject for dependency injection
Use vue 3.5 style of default prop declaration
Use Tailwind CSS for styling
Implement proper props and emits definitions
Utilize Vue 3's Teleport component when needed
Use Suspense for async components
Follow Vue 3 style guide and naming conventions
src/**/*.vue: Use Vue 3 Single File Components (SFCs) with Composition API only
Use<script setup lang="ts">for component logic in Vue SFCs
Avoid<style>blocks in Vue components - use Tailwind 4 styling instead
Use vue-i18n for all string literals in Vue components - place translation entries insrc/locales/en/main.json
Use Tailwind utility classes instead ofdark:variant - use semantic values fromstyle.csstheme (e.g.,bg-node-component-surface)
Usecn()utility from@/utils/tailwindUtilfor merging Tailwind class names instead of:class="[]"or hardcoding
Never use!importantor!Tailwind prefix - fix interfering classes instead
Use Tailwind fraction utilities instead of arbitrary percentage values (e.g.,w-4/5instead ofw-[80%])
Use TypeScript Vue 3.5 style default prop declaration with reactive props destructuring - avoidwithDefaultsor runtime props
PreferdefineModelover separately defining a prop and emit for v-model bindings
Define slots via template usage, not viadefineSlots
Use same-name shorthand for slot prop bindings (e.g.,:isExpandedinstead of:is-expanded="isExpanded")
Do not import Vue macros unnecessarily
Avoid new usage of PrimeVue components
Use Tailwind's plurals system via i18n instead of hardcoding ...
Files:
src/renderer/extensions/linearMode/Preview3d.vuesrc/renderer/extensions/linearMode/LinearPreview.vuesrc/renderer/extensions/linearMode/OutputHistory.vuesrc/renderer/extensions/linearMode/LinearControls.vuesrc/views/LinearView.vuesrc/components/topbar/TopMenuHelpButton.vue
src/components/**/*.vue
📄 CodeRabbit inference engine (src/components/CLAUDE.md)
src/components/**/*.vue: Use setup() function in Vue 3 Composition API
Destructure props using Vue 3.5 style in Vue components
Use ref/reactive for state management in Vue 3 Composition API
Implement computed() for derived state in Vue 3 Composition API
Use provide/inject for dependency injection in Vue components
Prefer emit/@event-name for state changes over other communication patterns
Use defineExpose only for imperative operations (such as form.validate(), modal.open())
Replace PrimeVue Dropdown component with Select
Replace PrimeVue OverlayPanel component with Popover
Replace PrimeVue Calendar component with DatePicker
Replace PrimeVue InputSwitch component with ToggleSwitch
Replace PrimeVue Sidebar component with Drawer
Replace PrimeVue Chips component with AutoComplete with multiple enabled
Replace PrimeVue TabMenu component with Tabs without panels
Replace PrimeVue Steps component with Stepper without panels
Replace PrimeVue InlineMessage component with Message
Extract complex conditionals to computed properties
Implement cleanup for async operations in Vue components
Use lifecycle hooks: onMounted, onUpdated in Vue 3 Composition API
Use Teleport/Suspense when needed for component rendering
Define proper props and emits definitions in Vue componentsName Vue components in PascalCase (e.g.,
MenuHamburger.vue)
Files:
src/components/topbar/TopMenuHelpButton.vue
src/components/**/*.{vue,css}
📄 CodeRabbit inference engine (src/components/CLAUDE.md)
src/components/**/*.{vue,css}: Use Tailwind CSS only for styling (no custom CSS)
Use the correct tokens from style.css in the design system package
Files:
src/components/topbar/TopMenuHelpButton.vue
src/components/**/*.{vue,ts,js}
📄 CodeRabbit inference engine (src/components/CLAUDE.md)
src/components/**/*.{vue,ts,js}: Use existing VueUse composables (such as useElementHover) instead of manually managing event listeners
Use useIntersectionObserver for visibility detection instead of custom scroll handlers
Use vue-i18n for ALL UI strings
Files:
src/components/topbar/TopMenuHelpButton.vue
🧠 Learnings (53)
📚 Learning: 2025-12-09T04:35:43.971Z
Learnt from: christian-byrne
Repo: Comfy-Org/ComfyUI_frontend PR: 6300
File: src/locales/en/main.json:774-780
Timestamp: 2025-12-09T04:35:43.971Z
Learning: In the Comfy-Org/ComfyUI_frontend repository, locale files other than `src/locales/en/main.json` are generated automatically on every release. Developers only need to add English (en) key/values in `src/locales/en/main.json` when making PRs; manual updates to other locale files (fr, ja, ko, ru, zh, zh-TW, es, ar, tr, etc.) are not required and should not be suggested in reviews.
Applied to files:
src/locales/en/commands.jsonsrc/locales/en/main.json
📚 Learning: 2025-12-09T03:39:54.501Z
Learnt from: DrJKL
Repo: Comfy-Org/ComfyUI_frontend PR: 7169
File: src/platform/remote/comfyui/jobs/jobTypes.ts:1-107
Timestamp: 2025-12-09T03:39:54.501Z
Learning: In the ComfyUI_frontend project, Zod is on v3.x. Do not suggest Zod v4 standalone validators (z.uuid, z.ulid, z.cuid2, z.nanoid) until an upgrade to Zod 4 is performed. When reviewing TypeScript files (e.g., src/platform/remote/comfyui/jobs/jobTypes.ts) validate against Zod 3 capabilities and avoid introducing v4-specific features; flag any proposal to upgrade or incorporate v4-only validators and propose staying with compatible 3.x patterns.
Applied to files:
src/composables/useCoreCommands.ts
📚 Learning: 2025-12-13T11:03:11.264Z
Learnt from: christian-byrne
Repo: Comfy-Org/ComfyUI_frontend PR: 7416
File: src/stores/imagePreviewStore.ts:5-7
Timestamp: 2025-12-13T11:03:11.264Z
Learning: In the ComfyUI_frontend repository, lint rules require keeping 'import type' statements separate from non-type imports, even if importing from the same module. Do not suggest consolidating them into a single import statement. Ensure type imports remain on their own line (import type { ... } from 'module') and regular imports stay on separate lines.
Applied to files:
src/composables/useCoreCommands.ts
📚 Learning: 2025-12-17T00:40:09.635Z
Learnt from: DrJKL
Repo: Comfy-Org/ComfyUI_frontend PR: 7537
File: src/components/ui/button/Button.stories.ts:45-55
Timestamp: 2025-12-17T00:40:09.635Z
Learning: Prefer pure function declarations over function expressions (e.g., use function foo() { ... } instead of const foo = () => { ... }) for pure functions in the repository. Function declarations are more functional-leaning, offer better hoisting clarity, and can improve readability and tooling consistency. Apply this guideline across TypeScript files in Comfy-Org/ComfyUI_frontend, including story and UI component code, except where a function expression is semantically required (e.g., callbacks, higher-order functions with closures).
Applied to files:
src/composables/useCoreCommands.ts
📚 Learning: 2025-12-30T22:22:33.836Z
Learnt from: kaili-yang
Repo: Comfy-Org/ComfyUI_frontend PR: 7805
File: src/composables/useCoreCommands.ts:439-439
Timestamp: 2025-12-30T22:22:33.836Z
Learning: When accessing reactive properties from Pinia stores in TypeScript files, avoid using .value on direct property access (e.g., useStore().isOverlayExpanded). Pinia auto-wraps refs when accessed directly, returning the primitive value. The .value accessor is only needed when destructuring store properties or when using storeToRefs().
Applied to files:
src/composables/useCoreCommands.ts
📚 Learning: 2025-12-11T12:25:15.470Z
Learnt from: christian-byrne
Repo: Comfy-Org/ComfyUI_frontend PR: 7358
File: src/components/dialog/content/signin/SignUpForm.vue:45-54
Timestamp: 2025-12-11T12:25:15.470Z
Learning: This repository uses CI automation to format code (pnpm format). Do not include manual formatting suggestions in code reviews for Comfy-Org/ComfyUI_frontend. If formatting issues are detected, rely on the CI formatter or re-run pnpm format. Focus reviews on correctness, readability, performance, accessibility, and maintainability rather than style formatting.
Applied to files:
src/composables/useCoreCommands.tssrc/renderer/extensions/linearMode/Preview3d.vuesrc/renderer/extensions/linearMode/LinearPreview.vuesrc/renderer/extensions/linearMode/OutputHistory.vuesrc/renderer/extensions/linearMode/LinearControls.vuesrc/views/LinearView.vuesrc/components/topbar/TopMenuHelpButton.vue
📚 Learning: 2026-01-12T17:39:27.738Z
Learnt from: DrJKL
Repo: Comfy-Org/ComfyUI_frontend PR: 7906
File: src/components/sidebar/tabs/AssetsSidebarTab.vue:545-552
Timestamp: 2026-01-12T17:39:27.738Z
Learning: In Vue/TypeScript files (src/**/*.{ts,tsx,vue}), prefer if/else statements over ternary operators when performing side effects or actions (e.g., mutating state, calling methods with side effects). Ternaries should be reserved for computing and returning values.
Applied to files:
src/composables/useCoreCommands.tssrc/renderer/extensions/linearMode/Preview3d.vuesrc/renderer/extensions/linearMode/LinearPreview.vuesrc/renderer/extensions/linearMode/OutputHistory.vuesrc/renderer/extensions/linearMode/LinearControls.vuesrc/views/LinearView.vuesrc/components/topbar/TopMenuHelpButton.vue
📚 Learning: 2026-01-10T00:24:17.695Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-10T00:24:17.695Z
Learning: Applies to src/**/*.vue : Use Vue 3 Single File Components (SFCs) with Composition API only
Applied to files:
src/renderer/extensions/linearMode/Preview3d.vuesrc/renderer/extensions/linearMode/OutputHistory.vuesrc/renderer/extensions/linearMode/LinearControls.vue
📚 Learning: 2026-01-10T00:24:17.695Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-10T00:24:17.695Z
Learning: Applies to src/**/*.vue : Use TypeScript Vue 3.5 style default prop declaration with reactive props destructuring - avoid `withDefaults` or runtime props
Applied to files:
src/renderer/extensions/linearMode/Preview3d.vuesrc/renderer/extensions/linearMode/OutputHistory.vue
📚 Learning: 2025-11-24T19:47:34.324Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: src/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:47:34.324Z
Learning: Applies to src/**/*.{vue,ts,tsx} : Follow Vue 3 composition API style guide
Applied to files:
src/renderer/extensions/linearMode/Preview3d.vuesrc/renderer/extensions/linearMode/OutputHistory.vuesrc/renderer/extensions/linearMode/LinearControls.vue
📚 Learning: 2025-11-24T19:47:02.860Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-24T19:47:02.860Z
Learning: Applies to src/**/*.vue : Utilize Vue 3's Teleport component when needed
Applied to files:
src/renderer/extensions/linearMode/Preview3d.vuesrc/renderer/extensions/linearMode/LinearControls.vue
📚 Learning: 2025-12-18T20:39:30.137Z
Learnt from: jtydhr88
Repo: Comfy-Org/ComfyUI_frontend PR: 7621
File: src/components/load3d/Load3DScene.vue:4-4
Timestamp: 2025-12-18T20:39:30.137Z
Learning: In src/components/load3d/Load3DScene.vue, the scoped `<style>` block with `!important` declarations for the canvas element is necessary because Three.js dynamically creates the canvas with inline styles, preventing direct application of Tailwind classes. This is a valid exception to the Tailwind-only styling guideline.
Applied to files:
src/renderer/extensions/linearMode/Preview3d.vuesrc/renderer/extensions/linearMode/OutputHistory.vuesrc/views/LinearView.vue
📚 Learning: 2025-11-24T19:47:02.860Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-24T19:47:02.860Z
Learning: Applies to src/**/*.vue : Use vue 3.5 style of default prop declaration
Applied to files:
src/renderer/extensions/linearMode/Preview3d.vuesrc/renderer/extensions/linearMode/OutputHistory.vue
📚 Learning: 2025-11-24T19:47:02.860Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-24T19:47:02.860Z
Learning: Applies to src/**/*.vue : Use the Vue 3 Composition API instead of the Options API when writing Vue components (exception: when overriding or extending PrimeVue components for compatibility)
Applied to files:
src/renderer/extensions/linearMode/Preview3d.vue
📚 Learning: 2026-01-10T00:24:17.695Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-10T00:24:17.695Z
Learning: Applies to src/**/*.vue : Use `<script setup lang="ts">` for component logic in Vue SFCs
Applied to files:
src/renderer/extensions/linearMode/Preview3d.vue
📚 Learning: 2025-11-24T19:47:45.616Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: src/components/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:47:45.616Z
Learning: Applies to src/components/**/*.vue : Destructure props using Vue 3.5 style in Vue components
Applied to files:
src/renderer/extensions/linearMode/Preview3d.vuesrc/renderer/extensions/linearMode/OutputHistory.vue
📚 Learning: 2025-11-24T19:47:02.860Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-24T19:47:02.860Z
Learning: Applies to src/**/*.vue : Follow Vue 3 style guide and naming conventions
Applied to files:
src/renderer/extensions/linearMode/Preview3d.vuesrc/renderer/extensions/linearMode/OutputHistory.vue
📚 Learning: 2025-12-09T03:49:52.828Z
Learnt from: christian-byrne
Repo: Comfy-Org/ComfyUI_frontend PR: 6300
File: src/platform/updates/components/WhatsNewPopup.vue:5-13
Timestamp: 2025-12-09T03:49:52.828Z
Learning: In Vue files across the ComfyUI_frontend repo, when a button is needed, prefer the repo's common button components from src/components/button/ (IconButton.vue, TextButton.vue, IconTextButton.vue) over plain HTML <button> elements. These components wrap PrimeVue with the project’s design system styling. Use only the common button components for consistency and theming, and import them from src/components/button/ as needed.
Applied to files:
src/renderer/extensions/linearMode/Preview3d.vuesrc/renderer/extensions/linearMode/LinearPreview.vuesrc/renderer/extensions/linearMode/OutputHistory.vuesrc/renderer/extensions/linearMode/LinearControls.vuesrc/views/LinearView.vuesrc/components/topbar/TopMenuHelpButton.vue
📚 Learning: 2025-12-09T21:40:12.361Z
Learnt from: benceruleanlu
Repo: Comfy-Org/ComfyUI_frontend PR: 7297
File: src/components/actionbar/ComfyActionbar.vue:33-43
Timestamp: 2025-12-09T21:40:12.361Z
Learning: In Vue single-file components, allow inline Tailwind CSS class strings for static classes and avoid extracting them into computed properties solely for readability. Prefer keeping static class names inline for simplicity and performance. For dynamic or conditional classes, use Vue bindings (e.g., :class) to compose classes.
Applies to all Vue files in the repository (e.g., src/**/*.vue) where Tailwind utilities are used for static styling.
Applied to files:
src/renderer/extensions/linearMode/Preview3d.vuesrc/renderer/extensions/linearMode/LinearPreview.vuesrc/renderer/extensions/linearMode/OutputHistory.vuesrc/renderer/extensions/linearMode/LinearControls.vuesrc/views/LinearView.vuesrc/components/topbar/TopMenuHelpButton.vue
📚 Learning: 2025-12-16T22:26:49.463Z
Learnt from: DrJKL
Repo: Comfy-Org/ComfyUI_frontend PR: 7537
File: src/components/ui/button/Button.vue:17-17
Timestamp: 2025-12-16T22:26:49.463Z
Learning: In Vue 3.5+ with <script setup>, when using defineProps<Props>() with partial destructuring (e.g., const { as = 'button', class: customClass = '' } = defineProps<Props>() ), props that are not destructured (e.g., variant, size) stay accessible by name in the template scope. This pattern is valid: you can destructure only a subset of props for convenience while referencing the remaining props directly in template expressions. Apply this guideline to Vue components across the codebase (all .vue files).
Applied to files:
src/renderer/extensions/linearMode/Preview3d.vuesrc/renderer/extensions/linearMode/LinearPreview.vuesrc/renderer/extensions/linearMode/OutputHistory.vuesrc/renderer/extensions/linearMode/LinearControls.vuesrc/views/LinearView.vuesrc/components/topbar/TopMenuHelpButton.vue
📚 Learning: 2025-12-22T21:36:08.369Z
Learnt from: DrJKL
Repo: Comfy-Org/ComfyUI_frontend PR: 7649
File: src/platform/cloud/subscription/components/PricingTable.vue:185-201
Timestamp: 2025-12-22T21:36:08.369Z
Learning: In Vue components, avoid creating single-use variants for common UI components (e.g., Button and other shared components). Aim for reusable variants that cover multiple use cases. It’s acceptable to temporarily mix variant props with inline Tailwind classes when a styling need is unique to one place, but plan and consolidate into shared, reusable variants as patterns emerge across the codebase.
Applied to files:
src/renderer/extensions/linearMode/Preview3d.vuesrc/renderer/extensions/linearMode/LinearPreview.vuesrc/renderer/extensions/linearMode/OutputHistory.vuesrc/renderer/extensions/linearMode/LinearControls.vuesrc/views/LinearView.vuesrc/components/topbar/TopMenuHelpButton.vue
📚 Learning: 2026-01-08T02:26:18.357Z
Learnt from: DrJKL
Repo: Comfy-Org/ComfyUI_frontend PR: 7893
File: src/components/button/IconGroup.vue:5-6
Timestamp: 2026-01-08T02:26:18.357Z
Learning: In components that use the cn utility from '@/utils/tailwindUtil' with tailwind-merge, rely on the behavior that conflicting Tailwind classes are resolved by keeping the last one. For example, cn('base-classes bg-default', propClass) will have any conflicting background class from propClass override bg-default. This additive pattern is intentional and aligns with the shadcn-ui convention; ensure you document or review expectations accordingly in Vue components.
Applied to files:
src/renderer/extensions/linearMode/Preview3d.vuesrc/renderer/extensions/linearMode/LinearPreview.vuesrc/renderer/extensions/linearMode/OutputHistory.vuesrc/renderer/extensions/linearMode/LinearControls.vuesrc/views/LinearView.vuesrc/components/topbar/TopMenuHelpButton.vue
📚 Learning: 2025-12-18T02:07:38.870Z
Learnt from: DrJKL
Repo: Comfy-Org/ComfyUI_frontend PR: 7598
File: src/components/sidebar/tabs/AssetsSidebarTab.vue:131-131
Timestamp: 2025-12-18T02:07:38.870Z
Learning: Tailwind CSS v4 safe utilities (e.g., items-center-safe, justify-*-safe, place-*-safe) are allowed in Vue components under src/ and in story files. Do not flag these specific safe variants as invalid when reviewing code in src/**/*.vue or related stories.
Applied to files:
src/renderer/extensions/linearMode/Preview3d.vuesrc/renderer/extensions/linearMode/LinearPreview.vuesrc/renderer/extensions/linearMode/OutputHistory.vuesrc/renderer/extensions/linearMode/LinearControls.vuesrc/views/LinearView.vuesrc/components/topbar/TopMenuHelpButton.vue
📚 Learning: 2025-12-18T21:15:46.862Z
Learnt from: DrJKL
Repo: Comfy-Org/ComfyUI_frontend PR: 7603
File: src/components/queue/QueueOverlayHeader.vue:49-59
Timestamp: 2025-12-18T21:15:46.862Z
Learning: In the ComfyUI_frontend repository, for Vue components, do not add aria-label to buttons that have visible text content (e.g., buttons containing <span> text). The visible text provides the accessible name. Use aria-label only for elements without visible labels (e.g., icon-only buttons). If a button has no visible label, provide a clear aria-label or associate with an aria-labelledby describing its action.
Applied to files:
src/renderer/extensions/linearMode/Preview3d.vuesrc/renderer/extensions/linearMode/LinearPreview.vuesrc/renderer/extensions/linearMode/OutputHistory.vuesrc/renderer/extensions/linearMode/LinearControls.vuesrc/views/LinearView.vuesrc/components/topbar/TopMenuHelpButton.vue
📚 Learning: 2025-12-21T01:06:02.786Z
Learnt from: DrJKL
Repo: Comfy-Org/ComfyUI_frontend PR: 7649
File: src/components/graph/selectionToolbox/ColorPickerButton.vue:15-18
Timestamp: 2025-12-21T01:06:02.786Z
Learning: In Comfy-Org/ComfyUI_frontend, in Vue component files, when a filled icon is required (e.g., 'pi pi-circle-fill'), you may mix PrimeIcons with Lucide icons since Lucide lacks filled variants. This mixed usage is acceptable when one icon library does not provide an equivalent filled icon. Apply consistently across Vue components in the src directory where icons are used, and document the rationale when a mixed approach is chosen.
Applied to files:
src/renderer/extensions/linearMode/Preview3d.vuesrc/renderer/extensions/linearMode/LinearPreview.vuesrc/renderer/extensions/linearMode/OutputHistory.vuesrc/renderer/extensions/linearMode/LinearControls.vuesrc/views/LinearView.vuesrc/components/topbar/TopMenuHelpButton.vue
📚 Learning: 2025-11-24T19:47:02.860Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-24T19:47:02.860Z
Learning: Applies to src/**/*.vue : Implement proper props and emits definitions
Applied to files:
src/renderer/extensions/linearMode/OutputHistory.vue
📚 Learning: 2025-12-04T21:43:49.363Z
Learnt from: DrJKL
Repo: Comfy-Org/ComfyUI_frontend PR: 7137
File: src/components/rightSidePanel/parameters/TabParameters.vue:10-0
Timestamp: 2025-12-04T21:43:49.363Z
Learning: Vue 3.5+ supports reactive props destructure in <script setup>. Destructuring props directly (e.g., `const { nodes } = defineProps<{ nodes: LGraphNode[] }>()`) maintains reactivity through compiler transformation. This is the recommended modern approach and does not require using `props.x` or `toRef`/`toRefs`.
Applied to files:
src/renderer/extensions/linearMode/OutputHistory.vue
📚 Learning: 2026-01-10T00:24:17.695Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-10T00:24:17.695Z
Learning: Applies to src/**/*.vue : Prefer `defineModel` over separately defining a prop and emit for v-model bindings
Applied to files:
src/renderer/extensions/linearMode/OutputHistory.vue
📚 Learning: 2026-01-10T00:24:17.695Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-10T00:24:17.695Z
Learning: Applies to src/**/*.{ts,vue} : Avoid using `ref` with `watch` if a `computed` would suffice - minimize refs and derived state
Applied to files:
src/renderer/extensions/linearMode/OutputHistory.vuesrc/views/LinearView.vue
📚 Learning: 2026-01-10T00:24:17.695Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-10T00:24:17.695Z
Learning: Applies to src/**/*.{ts,tsx,vue} : Avoid mutable state - prefer immutability and assignment at point of declaration
Applied to files:
src/renderer/extensions/linearMode/OutputHistory.vue
📚 Learning: 2025-11-24T19:47:02.860Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-24T19:47:02.860Z
Learning: Applies to src/**/*.vue : Utilize ref and reactive for reactive state
Applied to files:
src/renderer/extensions/linearMode/OutputHistory.vuesrc/views/LinearView.vue
📚 Learning: 2025-12-05T08:52:50.787Z
Learnt from: DrJKL
Repo: Comfy-Org/ComfyUI_frontend PR: 7137
File: src/components/rightSidePanel/parameters/SectionWidgets.vue:41-49
Timestamp: 2025-12-05T08:52:50.787Z
Learning: In Vue 3.4+ SFCs with a build step, kebab-case prop shorthand (e.g., `:is-empty`) automatically resolves to the corresponding camelCase variable in scope (e.g., `isEmpty`). This is valid syntax combining same-name shorthand with automatic kebab-case-to-camelCase conversion and should not be flagged as an error.
Applied to files:
src/renderer/extensions/linearMode/OutputHistory.vue
📚 Learning: 2026-01-10T00:24:17.695Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-10T00:24:17.695Z
Learning: Applies to src/**/*.{ts,vue} : Use `ref` for reactive state, `computed()` for derived values, and `watch`/`watchEffect` for side effects in Composition API
Applied to files:
src/renderer/extensions/linearMode/OutputHistory.vuesrc/views/LinearView.vue
📚 Learning: 2026-01-10T00:24:17.695Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-10T00:24:17.695Z
Learning: Applies to src/**/*.{ts,tsx,vue} : Watch out for Code Smells and refactor to avoid them
Applied to files:
src/renderer/extensions/linearMode/OutputHistory.vue
📚 Learning: 2025-11-24T19:47:02.860Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-24T19:47:02.860Z
Learning: Applies to src/**/*.vue : Use watch and watchEffect for side effects
Applied to files:
src/renderer/extensions/linearMode/OutputHistory.vue
📚 Learning: 2025-11-24T19:47:45.616Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: src/components/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:47:45.616Z
Learning: Applies to src/components/**/*.{vue,ts,js} : Use useIntersectionObserver for visibility detection instead of custom scroll handlers
Applied to files:
src/renderer/extensions/linearMode/OutputHistory.vuesrc/views/LinearView.vue
📚 Learning: 2025-11-24T19:47:45.616Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: src/components/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:47:45.616Z
Learning: Applies to src/components/**/*.vue : Use ref/reactive for state management in Vue 3 Composition API
Applied to files:
src/renderer/extensions/linearMode/OutputHistory.vuesrc/views/LinearView.vue
📚 Learning: 2025-11-24T19:47:45.616Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: src/components/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:47:45.616Z
Learning: Applies to src/components/**/*.vue : Use Teleport/Suspense when needed for component rendering
Applied to files:
src/renderer/extensions/linearMode/LinearControls.vue
📚 Learning: 2025-12-18T16:03:09.642Z
Learnt from: henrikvilhelmberglund
Repo: Comfy-Org/ComfyUI_frontend PR: 7617
File: src/components/actionbar/ComfyActionbar.vue:301-308
Timestamp: 2025-12-18T16:03:09.642Z
Learning: In the ComfyUI frontend queue system (src/stores/queueStore.ts), the `useQueuePendingTaskCountStore().count` includes the currently executing task. When count = 1, there is only the active/running task with no pending tasks. When count > 1, there is an active task plus pending tasks waiting in the queue. The "Clear Pending Tasks" button should only be enabled when count > 1 to avoid clearing the currently running task, which should be handled by the "Cancel current run" button instead.
Applied to files:
src/renderer/extensions/linearMode/LinearControls.vue
📚 Learning: 2025-11-24T19:47:02.860Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-24T19:47:02.860Z
Learning: Applies to src/**/*.{vue,ts} : Leverage VueUse functions for performance-enhancing styles
Applied to files:
src/views/LinearView.vue
📚 Learning: 2026-01-10T00:24:17.695Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-10T00:24:17.695Z
Learning: Applies to src/**/*.{ts,vue} : Leverage VueUse functions for performance-enhancing composables
Applied to files:
src/views/LinearView.vue
📚 Learning: 2025-11-24T19:47:45.616Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: src/components/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:47:45.616Z
Learning: Applies to src/components/**/*.vue : Replace PrimeVue Steps component with Stepper without panels
Applied to files:
src/views/LinearView.vue
📚 Learning: 2025-12-05T06:11:09.383Z
Learnt from: DrJKL
Repo: Comfy-Org/ComfyUI_frontend PR: 7177
File: src/platform/assets/components/UploadModelFooter.vue:72-78
Timestamp: 2025-12-05T06:11:09.383Z
Learning: For the ComfyUI_frontend repository, avoid suggesting comments that would be redundant when the code is already self-explanatory through descriptive naming (e.g., filenames, prop names, aria-labels). The project prefers clean code without unnecessary documentation comments.
Applied to files:
src/views/LinearView.vue
📚 Learning: 2025-11-24T19:47:45.616Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: src/components/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:47:45.616Z
Learning: Applies to src/components/**/*.vue : Replace PrimeVue Sidebar component with Drawer
Applied to files:
src/views/LinearView.vue
📚 Learning: 2025-11-24T19:47:45.616Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: src/components/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:47:45.616Z
Learning: Applies to src/components/**/*.vue : Replace PrimeVue TabMenu component with Tabs without panels
Applied to files:
src/views/LinearView.vuesrc/components/topbar/TopMenuHelpButton.vue
📚 Learning: 2026-01-09T07:29:32.501Z
Learnt from: LittleSound
Repo: Comfy-Org/ComfyUI_frontend PR: 7812
File: src/components/rightSidePanel/RightSidePanel.vue:100-132
Timestamp: 2026-01-09T07:29:32.501Z
Learning: The `findParentGroupInGraph` function in `src/components/rightSidePanel/RightSidePanel.vue` is a temporary workaround for a bug where group sub-items were not updating correctly after a page refresh. It can be removed once that underlying bug is fixed.
Applied to files:
src/views/LinearView.vue
📚 Learning: 2025-11-24T19:47:45.616Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: src/components/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:47:45.616Z
Learning: Applies to src/components/**/*.vue : Replace PrimeVue InlineMessage component with Message
Applied to files:
src/components/topbar/TopMenuHelpButton.vue
📚 Learning: 2025-11-24T19:47:45.616Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: src/components/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:47:45.616Z
Learning: Applies to src/components/**/*.vue : Replace PrimeVue OverlayPanel component with Popover
Applied to files:
src/components/topbar/TopMenuHelpButton.vue
📚 Learning: 2026-01-10T00:24:17.695Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-10T00:24:17.695Z
Learning: Applies to src/components/**/*.vue : Name Vue components in PascalCase (e.g., `MenuHamburger.vue`)
Applied to files:
src/components/topbar/TopMenuHelpButton.vue
📚 Learning: 2025-11-24T19:47:45.616Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: src/components/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:47:45.616Z
Learning: Applies to src/components/**/*.{vue,ts,js} : Use vue-i18n for ALL UI strings
Applied to files:
src/components/topbar/TopMenuHelpButton.vue
📚 Learning: 2025-11-24T19:47:45.616Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: src/components/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:47:45.616Z
Learning: Applies to src/components/**/*.vue : Replace PrimeVue InputSwitch component with ToggleSwitch
Applied to files:
src/components/topbar/TopMenuHelpButton.vue
📚 Learning: 2025-11-24T19:47:34.324Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: src/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:47:34.324Z
Learning: Applies to src/**/{components,composables}/**/*.{ts,tsx,vue} : Use vue-i18n for ALL user-facing strings by adding them to `src/locales/en/main.json`
Applied to files:
src/components/topbar/TopMenuHelpButton.vue
📚 Learning: 2025-12-18T16:03:02.066Z
Learnt from: henrikvilhelmberglund
Repo: Comfy-Org/ComfyUI_frontend PR: 7617
File: src/components/actionbar/ComfyActionbar.vue:301-308
Timestamp: 2025-12-18T16:03:02.066Z
Learning: In the ComfyUI frontend queue system, useQueuePendingTaskCountStore().count indicates the number of tasks in the queue, where count = 1 means a single active/running task and count > 1 means there are pending tasks in addition to the active task. Therefore, in src/components/actionbar/ComfyActionbar.vue, enable the 'Clear Pending Tasks' button only when count > 1 to avoid clearing the currently running task. The active task should be canceled using the 'Cancel current run' button instead. This rule should be enforced via a conditional check on the queue count, with appropriate disabled/aria-disabled states for accessibility, and tests should verify behavior for count = 1 and count > 1.
Applied to files:
src/components/topbar/TopMenuHelpButton.vue
🧬 Code graph analysis (1)
src/composables/useCoreCommands.ts (1)
src/scripts/app.ts (1)
app(1746-1746)
🔇 Additional comments (9)
src/locales/en/main.json (1)
193-193: Localization entries look good.The pluralization for
assetfollows vue-i18n's expected format, and the newlinearModeentries provide comprehensive coverage for the Simple Mode feature. The filter additions and label updates are consistent with the feature rename throughout the PR.Also applies to: 681-682, 1187-1187, 2477-2485
src/locales/en/commands.json (1)
275-277: Label update is consistent with the feature rename.The command label "Toggle Simple Mode" aligns with the terminology changes in
main.jsonand throughout the PR.src/components/topbar/TopMenuHelpButton.vue (1)
7-7: Responsive visibility looks correct.The
not-md:hiddenclass hides the text on smaller screens while keeping it visible at themdbreakpoint and above. This is valid Tailwind v4 syntax.src/renderer/extensions/linearMode/Preview3d.vue (1)
32-41: The code is correct as-is — no changes needed.Vue 3 templates auto-unwrap only top-level refs. Since
vieweris an object (not a ref), accessing its ref properties requires explicit.value. The patternv-model:animations="viewer.animations.value"properly binds to the ref value, anddefineModelin the child component handles the two-way sync correctly. Load3dViewerContent.vue uses the identical pattern, confirming it works.Likely an incorrect or invalid review comment.
src/renderer/extensions/linearMode/LinearControls.vue (2)
1-45: LGTM - Imports and setup are well-structured.Good use of VueUse utilities (
useEventListener,useTimeout), es-toolkit (partition), and proper store integration. The component follows Vue 3 Composition API patterns correctly.
90-96:batchCountWidgettyped asconstbut cannot be deeply frozen.The
as constassertion on a mutable object only makes the reference read-only, not the nested properties. This is fine for the current usage, but consider defining the type explicitly for clarity.src/renderer/extensions/linearMode/OutputHistory.vue (1)
1-33: LGTM - Well-structured imports and store integration.Good use of VueUse composables (
useEventListener,useInfiniteScroll,useScroll), proper store setup, and thevoidexpression on line 33 correctly indicates intentional fire-and-forget behavior.src/views/LinearView.vue (2)
1-45: LGTM - Good use of VueUse utilities and clear state management.Well-structured script setup with proper use of
breakpointsTailwind,whenever, and template refs. The reactive state for preview and selection is cleanly organized.
142-145: Layout anchor divs are well-organized.These template refs (
topLeftRef,topRightRef,bottomLeftRef,bottomRightRef) with absolute positioning provide clear anchor points for teleported content. Good pattern for flexible UI composition.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
src/composables/useCoreCommands.ts
Outdated
| function: () => { | ||
| const newMode = !canvasStore.linearMode | ||
| app.rootGraph.extra.linearMode = newMode | ||
| workflowStore.activeWorkflow?.changeTracker?.checkState() | ||
| canvasStore.linearMode = newMode | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for rootGraph.extra usage patterns
echo "=== rootGraph.extra usage patterns ==="
rg "rootGraph\.extra" --type ts -B 2 -A 2 | head -100
echo -e "\n=== LGraph class definition and extra initialization ==="
# Find LGraph class definition
rg "class LGraph" --type ts -A 30 | head -80
echo -e "\n=== extra property initialization ==="
# Check how extra is typically initialized
rg "this\.extra\s*=" --type ts -B 2 -A 2 | head -80
echo -e "\n=== Defensive patterns with extra ==="
# Check if defensive patterns are used anywhere
rg "extra\s*\?\." --type ts -B 1 -A 1Repository: Comfy-Org/ComfyUI_frontend
Length of output: 13209
Add defensive initialization for consistency with workflowService.ts.
The extra property is always initialized by the LGraph constructor, but workflowService.ts already uses app.rootGraph.extra ??= {} when setting linearMode. Apply the same defensive pattern here for consistency.
Suggested fix
function: () => {
const newMode = !canvasStore.linearMode
+ app.rootGraph.extra ??= {}
app.rootGraph.extra.linearMode = newMode
workflowStore.activeWorkflow?.changeTracker?.checkState()
canvasStore.linearMode = newMode
}📝 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.
| function: () => { | |
| const newMode = !canvasStore.linearMode | |
| app.rootGraph.extra.linearMode = newMode | |
| workflowStore.activeWorkflow?.changeTracker?.checkState() | |
| canvasStore.linearMode = newMode | |
| } | |
| function: () => { | |
| const newMode = !canvasStore.linearMode | |
| app.rootGraph.extra ??= {} | |
| app.rootGraph.extra.linearMode = newMode | |
| workflowStore.activeWorkflow?.changeTracker?.checkState() | |
| canvasStore.linearMode = newMode | |
| } |
🤖 Prompt for AI Agents
In `@src/composables/useCoreCommands.ts` around lines 1238 - 1243, The anonymous
command function toggles linear mode but assumes app.rootGraph.extra exists;
mirror workflowService.ts by defensively initializing app.rootGraph.extra using
the nullish-coalescing assignment before setting linearMode. In the function
block (function: () => { ... }) add an initialization for app.rootGraph.extra
(e.g., app.rootGraph.extra ??= {}) then set app.rootGraph.extra.linearMode =
newMode and update canvasStore.linearMode and call
workflowStore.activeWorkflow?.changeTracker?.checkState() as currently done.
| const partitionedNodes = computed(() => { | ||
| const parts = partition( | ||
| graphNodes.value | ||
| .filter((node) => node.mode === 0 && node.widgets?.length) | ||
| .map(nodeToNodeData) | ||
| .reverse(), | ||
| (node) => ['MarkdownNote', 'Note'].includes(node.type) | ||
| ) | ||
| for (const noteNode of parts[0]) { | ||
| for (const widget of noteNode.widgets ?? []) | ||
| widget.options = { ...widget.options, read_only: true } | ||
| } | ||
| return parts | ||
| }) |
There was a problem hiding this comment.
Mutation of widget options in partition callback.
Similar to the previous concern, lines 84-85 mutate widget.options for note nodes. While this uses spread syntax, it still modifies objects in the reactive parts array during iteration.
🐛 Proposed fix - immutable approach
const partitionedNodes = computed(() => {
const parts = partition(
graphNodes.value
.filter((node) => node.mode === 0 && node.widgets?.length)
.map(nodeToNodeData)
.reverse(),
(node) => ['MarkdownNote', 'Note'].includes(node.type)
)
- for (const noteNode of parts[0]) {
- for (const widget of noteNode.widgets ?? [])
- widget.options = { ...widget.options, read_only: true }
- }
- return parts
+ const notesWithReadOnly = parts[0].map((noteNode) => ({
+ ...noteNode,
+ widgets: noteNode.widgets?.map((widget) => ({
+ ...widget,
+ options: { ...widget.options, read_only: true }
+ }))
+ }))
+ return [notesWithReadOnly, parts[1]]
})🤖 Prompt for AI Agents
In `@src/renderer/extensions/linearMode/LinearControls.vue` around lines 75 - 88,
partitionedNodes currently mutates reactive objects by assigning to
widget.options inside the loop; instead produce immutable copies: when building
parts (or specifically parts[0]), map over those note nodes and their widgets to
return new noteNode objects with a new widgets array where each widget is
shallow-copied and its options replaced with a copy that sets read_only: true;
update the returned parts to use these copied note nodes so you never assign to
widget.options on reactive objects (refer to partitionedNodes, parts, noteNode,
widget, nodeToNodeData, graphNodes).
| const { runButtonClick, selectedItem, selectedOutput } = defineProps<{ | ||
| latentPreview?: string | ||
| runButtonClick?: (e: Event) => void | ||
| selectedItem?: AssetItem | ||
| selectedOutput?: ResultItemImpl | ||
| mobile?: boolean | ||
| }>() |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider Vue 3.5 style reactive props destructuring.
Per coding guidelines, prefer destructuring props with defaults directly for cleaner template usage.
♻️ Suggested refactor
-const { runButtonClick, selectedItem, selectedOutput } = defineProps<{
+const {
+ latentPreview,
+ runButtonClick,
+ selectedItem,
+ selectedOutput,
+ mobile = false
+} = defineProps<{
latentPreview?: string
runButtonClick?: (e: Event) => void
selectedItem?: AssetItem
selectedOutput?: ResultItemImpl
mobile?: boolean
}>()🤖 Prompt for AI Agents
In `@src/renderer/extensions/linearMode/LinearPreview.vue` around lines 29 - 35,
The props destructuring in LinearPreview.vue should use Vue 3.5 reactive
pattern: replace the direct defineProps destructuring with a props object
created via withDefaults(defineProps<...>()) to supply defaults for
latentPreview and mobile, then convert to refs using toRefs/toRef so template
bindings remain reactive; update references to runButtonClick, selectedItem,
selectedOutput, latentPreview and mobile to use those refs (or .value in script)
instead of the current plain variables.
| <ImagePreview | ||
| v-if="latentPreview ?? getMediaType(selectedOutput) === 'images'" | ||
| :mobile | ||
| :src="latentPreview ?? selectedOutput!.url" | ||
| /> | ||
| <VideoPreview | ||
| v-else-if="getMediaType(selectedOutput) === 'video'" | ||
| :src="selectedOutput!.url" | ||
| class="object-contain flex-1 contain-size" | ||
| /> | ||
| <audio | ||
| v-else-if="getMediaType(selectedOutput) === 'audio'" | ||
| class="w-full m-auto" | ||
| controls | ||
| :src="selectedOutput!.url" | ||
| /> | ||
| <article | ||
| v-else-if="getMediaType(selectedOutput) === 'text'" | ||
| class="w-full max-w-128 m-auto my-12 overflow-y-auto" | ||
| v-text="selectedOutput!.url" | ||
| /> | ||
| <Preview3d | ||
| v-else-if="getMediaType(selectedOutput) === '3d'" | ||
| :model-url="selectedOutput!.url" | ||
| /> |
There was a problem hiding this comment.
Non-null assertions on selectedOutput may cause runtime errors.
Multiple conditional branches use selectedOutput! after checking getMediaType(selectedOutput). However, if getMediaType returns a truthy value for undefined input (or has inconsistent handling), accessing selectedOutput!.url will throw. Consider using optional chaining or restructuring the conditionals.
🐛 Proposed fix using optional chaining
<ImagePreview
v-if="latentPreview ?? getMediaType(selectedOutput) === 'images'"
:mobile
- :src="latentPreview ?? selectedOutput!.url"
+ :src="latentPreview ?? selectedOutput?.url"
/>
<VideoPreview
v-else-if="getMediaType(selectedOutput) === 'video'"
- :src="selectedOutput!.url"
+ :src="selectedOutput?.url ?? ''"
class="object-contain flex-1 contain-size"
/>
<audio
v-else-if="getMediaType(selectedOutput) === 'audio'"
class="w-full m-auto"
controls
- :src="selectedOutput!.url"
+ :src="selectedOutput?.url"
/>
<article
v-else-if="getMediaType(selectedOutput) === 'text'"
class="w-full max-w-128 m-auto my-12 overflow-y-auto"
- v-text="selectedOutput!.url"
+ v-text="selectedOutput?.url"
/>
<Preview3d
v-else-if="getMediaType(selectedOutput) === '3d'"
- :model-url="selectedOutput!.url"
+ :model-url="selectedOutput?.url ?? ''"
/>🤖 Prompt for AI Agents
In `@src/renderer/extensions/linearMode/LinearPreview.vue` around lines 154 - 178,
The template uses non-null assertions on selectedOutput (selectedOutput!) which
can throw if selectedOutput is undefined even when getMediaType(selectedOutput)
returns a value; remove the non-null assertions and guard usage of
selectedOutput by either (a) verifying selectedOutput exists before rendering
each branch (compute mediaType = getMediaType(selectedOutput) and use v-if
checks that include selectedOutput) or (b) switch to optional chaining for
values (use selectedOutput?.url / :model-url="selectedOutput?.url") and ensure
ImagePreview/VideoPreview/Preview3d branches only render when selectedOutput is
present; update all references to selectedOutput! accordingly (selectedOutput,
getMediaType, ImagePreview, VideoPreview, Preview3d) so runtime null/undefined
access is avoided.
| ] | ||
| }>() | ||
|
|
||
| defineExpose({ onWheel }) |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
defineExpose called before onWheel is defined.
While this works due to function hoisting, it's unconventional and may confuse readers. Consider moving defineExpose after the function definition for clarity.
🤖 Prompt for AI Agents
In `@src/renderer/extensions/linearMode/OutputHistory.vue` at line 45, Move the
defineExpose call so it appears after the onWheel function definition to improve
clarity; specifically, locate the defineExpose({ onWheel }) call and relocate it
below the onWheel function declaration in OutputHistory.vue so the exposed
symbol is defined before being exported.
| watch([containerRef, () => modelUrl], async () => { | ||
| if (!containerRef.value || !modelUrl) return | ||
|
|
||
| await viewer.initializeStandaloneViewer(containerRef.value, modelUrl) | ||
| }) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Check if useLoad3dViewer or similar composables have dispose/cleanup methods
fd useLoad3dViewer -type fRepository: Comfy-Org/ComfyUI_frontend
Length of output: 240
🏁 Script executed:
# Also search for viewer dispose/cleanup patterns in the codebase
rg "dispose|cleanup|destroy" --type ts --type vue -C 2 | head -100Repository: Comfy-Org/ComfyUI_frontend
Length of output: 97
🏁 Script executed:
# Examine the Preview3d.vue file to see the full context
cat -n src/renderer/extensions/linearMode/Preview3d.vueRepository: Comfy-Org/ComfyUI_frontend
Length of output: 1777
🏁 Script executed:
# Search for any existing cleanup patterns for 3D viewers
rg "onBeforeUnmount|onUnmounted" --type vue -A 3 | grep -A 3 "viewer\|Viewer" | head -50Repository: Comfy-Org/ComfyUI_frontend
Length of output: 97
🏁 Script executed:
# Find the useLoad3dViewer composable file
find . -name "*useLoad3dViewer*" -type fRepository: Comfy-Org/ComfyUI_frontend
Length of output: 148
🏁 Script executed:
# Search for dispose/cleanup in composables and src directory
rg "dispose|cleanup|destroy" -g "*.ts" -g "*.js" -C 2Repository: Comfy-Org/ComfyUI_frontend
Length of output: 50384
🏁 Script executed:
# Look for the composable specifically
find . -path "*/composables/*" -name "*.ts" | head -20Repository: Comfy-Org/ComfyUI_frontend
Length of output: 1134
🏁 Script executed:
cat -n src/composables/useLoad3dViewer.tsRepository: Comfy-Org/ComfyUI_frontend
Length of output: 21409
🏁 Script executed:
# Also check the test file to understand expected behavior
cat -n src/composables/useLoad3dViewer.test.tsRepository: Comfy-Org/ComfyUI_frontend
Length of output: 23496
🏁 Script executed:
# Search for dispose/cleanup specifically in useLoad3dViewer
rg "dispose|cleanup|destroy" src/composables/useLoad3dViewer.ts src/services/load3dService.ts -A 3Repository: Comfy-Org/ComfyUI_frontend
Length of output: 613
Add cleanup hook to dispose 3D viewer resources on unmount.
The useLoad3dViewer composable provides a cleanup() method (not dispose) that should be called when the component unmounts to prevent WebGL context and resource leaks.
♻️ Suggested addition
+import { useTemplateRef, watch, onBeforeUnmount } from 'vue'
-import { useTemplateRef, watch } from 'vue'
// ... existing code ...
watch([containerRef, () => modelUrl], async () => {
if (!containerRef.value || !modelUrl) return
await viewer.initializeStandaloneViewer(containerRef.value, modelUrl)
})
+onBeforeUnmount(() => {
+ viewer.cleanup()
+})📝 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.
| watch([containerRef, () => modelUrl], async () => { | |
| if (!containerRef.value || !modelUrl) return | |
| await viewer.initializeStandaloneViewer(containerRef.value, modelUrl) | |
| }) | |
| import { useTemplateRef, watch, onBeforeUnmount } from 'vue' | |
| // ... other code ... | |
| watch([containerRef, () => modelUrl], async () => { | |
| if (!containerRef.value || !modelUrl) return | |
| await viewer.initializeStandaloneViewer(containerRef.value, modelUrl) | |
| }) | |
| onBeforeUnmount(() => { | |
| viewer.cleanup() | |
| }) |
🤖 Prompt for AI Agents
In `@src/renderer/extensions/linearMode/Preview3d.vue` around lines 15 - 19, The
watcher initializes the 3D viewer via viewer.initializeStandaloneViewer but
never calls the composable's cleanup(), causing WebGL/resource leaks; add a
component unmount hook (onBeforeUnmount or onUnmounted) to call the cleanup()
method from useLoad3dViewer, and ensure any previous viewer state is cleaned
before re-initializing (e.g., call cleanup() at the start of the watcher or
before calling viewer.initializeStandaloneViewer) so containerRef, modelUrl and
viewer lifecycle are properly disposed.
| class="relative w-full h-full" | ||
| @mouseenter="viewer.handleMouseEnter" | ||
| @mouseleave="viewer.handleMouseLeave" | ||
| @resize="viewer.handleResize" |
There was a problem hiding this comment.
@resize event will never fire on a <div>.
The resize event is only dispatched by the window object. This handler will never be triggered. Use ResizeObserver via VueUse's useResizeObserver or bind to the window resize event instead.
🛠️ Suggested fix using VueUse
<script setup lang="ts">
-import { useTemplateRef, watch } from 'vue'
+import { useTemplateRef, watch, onUnmounted } from 'vue'
+import { useResizeObserver } from '@vueuse/core'
import AnimationControls from '@/components/load3d/controls/AnimationControls.vue'
import { useLoad3dViewer } from '@/composables/useLoad3dViewer'
const { modelUrl } = defineProps<{
modelUrl: string
}>()
const containerRef = useTemplateRef('containerRef')
const viewer = useLoad3dViewer()
+useResizeObserver(containerRef, () => {
+ viewer.handleResize()
+})
watch([containerRef, () => modelUrl], async () => {
if (!containerRef.value || !modelUrl) return
await viewer.initializeStandaloneViewer(containerRef.value, modelUrl)
})Then remove the @resize handler from the template:
<div
ref="containerRef"
class="relative w-full h-full"
`@mouseenter`="viewer.handleMouseEnter"
`@mouseleave`="viewer.handleMouseLeave"
- `@resize`="viewer.handleResize"
>📝 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.
| @resize="viewer.handleResize" | |
| <script setup lang="ts"> | |
| import { useTemplateRef, watch, onUnmounted } from 'vue' | |
| import { useResizeObserver } from '@vueuse/core' | |
| import AnimationControls from '@/components/load3d/controls/AnimationControls.vue' | |
| import { useLoad3dViewer } from '@/composables/useLoad3dViewer' | |
| const { modelUrl } = defineProps<{ | |
| modelUrl: string | |
| }>() | |
| const containerRef = useTemplateRef('containerRef') | |
| const viewer = useLoad3dViewer() | |
| useResizeObserver(containerRef, () => { | |
| viewer.handleResize() | |
| }) | |
| watch([containerRef, () => modelUrl], async () => { | |
| if (!containerRef.value || !modelUrl) return | |
| await viewer.initializeStandaloneViewer(containerRef.value, modelUrl) | |
| }) | |
| </script> | |
| <template> | |
| <div | |
| ref="containerRef" | |
| class="relative w-full h-full" | |
| `@mouseenter`="viewer.handleMouseEnter" | |
| `@mouseleave`="viewer.handleMouseLeave" | |
| > | |
| <AnimationControls :controls="viewer.controls.value" /> | |
| </div> | |
| </template> |
🤖 Prompt for AI Agents
In `@src/renderer/extensions/linearMode/Preview3d.vue` at line 29, The template
uses `@resize` on a div which never fires; replace that with a ResizeObserver and
call the same handler: in Preview3d.vue remove the `@resize` binding and in the
script setup (or component) import/use VueUse's useResizeObserver (or
window.addEventListener('resize') as an alternative) to observe the div element
and invoke viewer.handleResize whenever size changes; ensure you reference the
div's ref (e.g., containerRef) when calling useResizeObserver and clean up the
observer on unmount.
| <a | ||
| href="https://form.typeform.com/to/gmVqFi8l" | ||
| v-text="t('linearMode.beta')" | ||
| /> | ||
| <TypeformPopoverButton data-tf-widget="gmVqFi8l" /> |
There was a problem hiding this comment.
External link should include security attributes.
The anchor tag linking to Typeform should include rel="noopener noreferrer" and target="_blank" for security when opening external URLs.
🔒 Proposed fix
<a
href="https://form.typeform.com/to/gmVqFi8l"
+ target="_blank"
+ rel="noopener noreferrer"
v-text="t('linearMode.beta')"
/>🤖 Prompt for AI Agents
In `@src/views/LinearView.vue` around lines 88 - 92, The external anchor in
LinearView.vue (the <a> element rendering t('linearMode.beta') next to
TypeformPopoverButton) must open in a new tab and avoid window.opener risks; add
target="_blank" and rel="noopener noreferrer" to that <a> tag so the Typeform
link is opened securely.
…wing node bounds (#8082) Backport of #8070 to `cloud/1.37` Automatically created by backport workflow. ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-8082-backport-cloud-1-37-fix-prevent-Record-Audio-waveform-from-overflowing-node-bounds-2e96d73d36508112b881df2c4bf5fd3c) by [Unito](https://www.unito.io) Co-authored-by: Terry Jia <terryjia88@gmail.com>
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/renderer/extensions/vueNodes/widgets/components/WidgetRecordAudio.vue (1)
45-67: Use shared button components for the icon-only controls.Prefer the common button components from
src/components/button/(e.g.,IconButton) for consistent theming and behaviors; supply an accessible label via the component’s props for these icon-only actions. Based on learnings, prefer shared button components for consistency.
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
src/renderer/extensions/vueNodes/widgets/components/WidgetRecordAudio.vue
🧰 Additional context used
📓 Path-based instructions (7)
src/**/*.vue
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
src/**/*.vue: Use the Vue 3 Composition API instead of the Options API when writing Vue components (exception: when overriding or extending PrimeVue components for compatibility)
Use setup() function for component logic
Utilize ref and reactive for reactive state
Implement computed properties with computed()
Use watch and watchEffect for side effects
Implement lifecycle hooks with onMounted, onUpdated, etc.
Utilize provide/inject for dependency injection
Use vue 3.5 style of default prop declaration
Use Tailwind CSS for styling
Implement proper props and emits definitions
Utilize Vue 3's Teleport component when needed
Use Suspense for async components
Follow Vue 3 style guide and naming conventions
src/**/*.vue: Use Vue 3 Single File Components (SFCs) with Composition API only
Use<script setup lang="ts">for component logic in Vue SFCs
Avoid<style>blocks in Vue components - use Tailwind 4 styling instead
Use vue-i18n for all string literals in Vue components - place translation entries insrc/locales/en/main.json
Use Tailwind utility classes instead ofdark:variant - use semantic values fromstyle.csstheme (e.g.,bg-node-component-surface)
Usecn()utility from@/utils/tailwindUtilfor merging Tailwind class names instead of:class="[]"or hardcoding
Never use!importantor!Tailwind prefix - fix interfering classes instead
Use Tailwind fraction utilities instead of arbitrary percentage values (e.g.,w-4/5instead ofw-[80%])
Use TypeScript Vue 3.5 style default prop declaration with reactive props destructuring - avoidwithDefaultsor runtime props
PreferdefineModelover separately defining a prop and emit for v-model bindings
Define slots via template usage, not viadefineSlots
Use same-name shorthand for slot prop bindings (e.g.,:isExpandedinstead of:is-expanded="isExpanded")
Do not import Vue macros unnecessarily
Avoid new usage of PrimeVue components
Use Tailwind's plurals system via i18n instead of hardcoding ...
Files:
src/renderer/extensions/vueNodes/widgets/components/WidgetRecordAudio.vue
src/**/*.{vue,ts}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
src/**/*.{vue,ts}: Leverage VueUse functions for performance-enhancing styles
Implement proper error handling
Use vue-i18n in composition API for any string literals. Place new translation entries in src/locales/en/main.json
Files:
src/renderer/extensions/vueNodes/widgets/components/WidgetRecordAudio.vue
src/**/*.{ts,tsx,vue}
📄 CodeRabbit inference engine (src/CLAUDE.md)
src/**/*.{ts,tsx,vue}: Sanitize HTML with DOMPurify to prevent XSS attacks
Avoid using@ts-expect-error; use proper TypeScript types instead
Use es-toolkit for utility functions instead of other utility libraries
Implement proper TypeScript types throughout the codebase
src/**/*.{ts,tsx,vue}: Use separateimport typestatements instead of inlinetypein mixed imports
Apply Prettier formatting with 2-space indentation, single quotes, no trailing semicolons, 80-character width
Sort and group imports by plugin, runpnpm formatbefore committing
Never useanytype - use proper TypeScript types
Never useas anytype assertions - fix the underlying type issue
Write code that is expressive and self-documenting - avoid unnecessary comments
Do not add or retain redundant comments - clean as you go
Avoid mutable state - prefer immutability and assignment at point of declaration
Watch out for Code Smells and refactor to avoid them
Files:
src/renderer/extensions/vueNodes/widgets/components/WidgetRecordAudio.vue
src/**/{composables,components}/**/*.{ts,tsx,vue}
📄 CodeRabbit inference engine (src/CLAUDE.md)
Clean up subscriptions in state management to prevent memory leaks
Files:
src/renderer/extensions/vueNodes/widgets/components/WidgetRecordAudio.vue
src/**/*.{vue,ts,tsx}
📄 CodeRabbit inference engine (src/CLAUDE.md)
Follow Vue 3 composition API style guide
Files:
src/renderer/extensions/vueNodes/widgets/components/WidgetRecordAudio.vue
src/**/{components,composables}/**/*.{ts,tsx,vue}
📄 CodeRabbit inference engine (src/CLAUDE.md)
Use vue-i18n for ALL user-facing strings by adding them to
src/locales/en/main.json
Files:
src/renderer/extensions/vueNodes/widgets/components/WidgetRecordAudio.vue
src/**/*.{ts,vue}
📄 CodeRabbit inference engine (AGENTS.md)
src/**/*.{ts,vue}: Usereffor reactive state,computed()for derived values, andwatch/watchEffectfor side effects in Composition API
Avoid usingrefwithwatchif acomputedwould suffice - minimize refs and derived state
Useprovide/injectfor dependency injection only when simpler alternatives (Store or shared composable) won't work
Leverage VueUse functions for performance-enhancing composables
Use VueUse function for useI18n in composition API for string literals
Files:
src/renderer/extensions/vueNodes/widgets/components/WidgetRecordAudio.vue
🧠 Learnings (11)
📚 Learning: 2025-12-09T03:49:52.828Z
Learnt from: christian-byrne
Repo: Comfy-Org/ComfyUI_frontend PR: 6300
File: src/platform/updates/components/WhatsNewPopup.vue:5-13
Timestamp: 2025-12-09T03:49:52.828Z
Learning: In Vue files across the ComfyUI_frontend repo, when a button is needed, prefer the repo's common button components from src/components/button/ (IconButton.vue, TextButton.vue, IconTextButton.vue) over plain HTML <button> elements. These components wrap PrimeVue with the project’s design system styling. Use only the common button components for consistency and theming, and import them from src/components/button/ as needed.
Applied to files:
src/renderer/extensions/vueNodes/widgets/components/WidgetRecordAudio.vue
📚 Learning: 2025-12-09T21:40:12.361Z
Learnt from: benceruleanlu
Repo: Comfy-Org/ComfyUI_frontend PR: 7297
File: src/components/actionbar/ComfyActionbar.vue:33-43
Timestamp: 2025-12-09T21:40:12.361Z
Learning: In Vue single-file components, allow inline Tailwind CSS class strings for static classes and avoid extracting them into computed properties solely for readability. Prefer keeping static class names inline for simplicity and performance. For dynamic or conditional classes, use Vue bindings (e.g., :class) to compose classes.
Applies to all Vue files in the repository (e.g., src/**/*.vue) where Tailwind utilities are used for static styling.
Applied to files:
src/renderer/extensions/vueNodes/widgets/components/WidgetRecordAudio.vue
📚 Learning: 2025-12-16T22:26:49.463Z
Learnt from: DrJKL
Repo: Comfy-Org/ComfyUI_frontend PR: 7537
File: src/components/ui/button/Button.vue:17-17
Timestamp: 2025-12-16T22:26:49.463Z
Learning: In Vue 3.5+ with <script setup>, when using defineProps<Props>() with partial destructuring (e.g., const { as = 'button', class: customClass = '' } = defineProps<Props>() ), props that are not destructured (e.g., variant, size) stay accessible by name in the template scope. This pattern is valid: you can destructure only a subset of props for convenience while referencing the remaining props directly in template expressions. Apply this guideline to Vue components across the codebase (all .vue files).
Applied to files:
src/renderer/extensions/vueNodes/widgets/components/WidgetRecordAudio.vue
📚 Learning: 2025-12-22T21:36:08.369Z
Learnt from: DrJKL
Repo: Comfy-Org/ComfyUI_frontend PR: 7649
File: src/platform/cloud/subscription/components/PricingTable.vue:185-201
Timestamp: 2025-12-22T21:36:08.369Z
Learning: In Vue components, avoid creating single-use variants for common UI components (e.g., Button and other shared components). Aim for reusable variants that cover multiple use cases. It’s acceptable to temporarily mix variant props with inline Tailwind classes when a styling need is unique to one place, but plan and consolidate into shared, reusable variants as patterns emerge across the codebase.
Applied to files:
src/renderer/extensions/vueNodes/widgets/components/WidgetRecordAudio.vue
📚 Learning: 2026-01-08T02:26:18.357Z
Learnt from: DrJKL
Repo: Comfy-Org/ComfyUI_frontend PR: 7893
File: src/components/button/IconGroup.vue:5-6
Timestamp: 2026-01-08T02:26:18.357Z
Learning: In components that use the cn utility from '@/utils/tailwindUtil' with tailwind-merge, rely on the behavior that conflicting Tailwind classes are resolved by keeping the last one. For example, cn('base-classes bg-default', propClass) will have any conflicting background class from propClass override bg-default. This additive pattern is intentional and aligns with the shadcn-ui convention; ensure you document or review expectations accordingly in Vue components.
Applied to files:
src/renderer/extensions/vueNodes/widgets/components/WidgetRecordAudio.vue
📚 Learning: 2025-12-11T03:55:51.755Z
Learnt from: simula-r
Repo: Comfy-Org/ComfyUI_frontend PR: 7252
File: src/renderer/extensions/vueNodes/components/ImagePreview.vue:151-158
Timestamp: 2025-12-11T03:55:51.755Z
Learning: In Vue components under src/renderer/extensions/vueNodes (e.g., ImagePreview.vue and LGraphNode.vue), implement image gallery keyboard navigation so that it responds to the node's focus state rather than requiring focus inside the image preview wrapper. Achieve this by wiring keyEvent handling at the node focus level and injecting or propagating key events (arrow keys) to the gallery when the node is focused/selected. This improves accessibility and aligns navigation with node-level focus behavior.
Applied to files:
src/renderer/extensions/vueNodes/widgets/components/WidgetRecordAudio.vue
📚 Learning: 2025-12-11T12:25:15.470Z
Learnt from: christian-byrne
Repo: Comfy-Org/ComfyUI_frontend PR: 7358
File: src/components/dialog/content/signin/SignUpForm.vue:45-54
Timestamp: 2025-12-11T12:25:15.470Z
Learning: This repository uses CI automation to format code (pnpm format). Do not include manual formatting suggestions in code reviews for Comfy-Org/ComfyUI_frontend. If formatting issues are detected, rely on the CI formatter or re-run pnpm format. Focus reviews on correctness, readability, performance, accessibility, and maintainability rather than style formatting.
Applied to files:
src/renderer/extensions/vueNodes/widgets/components/WidgetRecordAudio.vue
📚 Learning: 2025-12-18T02:07:38.870Z
Learnt from: DrJKL
Repo: Comfy-Org/ComfyUI_frontend PR: 7598
File: src/components/sidebar/tabs/AssetsSidebarTab.vue:131-131
Timestamp: 2025-12-18T02:07:38.870Z
Learning: Tailwind CSS v4 safe utilities (e.g., items-center-safe, justify-*-safe, place-*-safe) are allowed in Vue components under src/ and in story files. Do not flag these specific safe variants as invalid when reviewing code in src/**/*.vue or related stories.
Applied to files:
src/renderer/extensions/vueNodes/widgets/components/WidgetRecordAudio.vue
📚 Learning: 2025-12-18T21:15:46.862Z
Learnt from: DrJKL
Repo: Comfy-Org/ComfyUI_frontend PR: 7603
File: src/components/queue/QueueOverlayHeader.vue:49-59
Timestamp: 2025-12-18T21:15:46.862Z
Learning: In the ComfyUI_frontend repository, for Vue components, do not add aria-label to buttons that have visible text content (e.g., buttons containing <span> text). The visible text provides the accessible name. Use aria-label only for elements without visible labels (e.g., icon-only buttons). If a button has no visible label, provide a clear aria-label or associate with an aria-labelledby describing its action.
Applied to files:
src/renderer/extensions/vueNodes/widgets/components/WidgetRecordAudio.vue
📚 Learning: 2025-12-21T01:06:02.786Z
Learnt from: DrJKL
Repo: Comfy-Org/ComfyUI_frontend PR: 7649
File: src/components/graph/selectionToolbox/ColorPickerButton.vue:15-18
Timestamp: 2025-12-21T01:06:02.786Z
Learning: In Comfy-Org/ComfyUI_frontend, in Vue component files, when a filled icon is required (e.g., 'pi pi-circle-fill'), you may mix PrimeIcons with Lucide icons since Lucide lacks filled variants. This mixed usage is acceptable when one icon library does not provide an equivalent filled icon. Apply consistently across Vue components in the src directory where icons are used, and document the rationale when a mixed approach is chosen.
Applied to files:
src/renderer/extensions/vueNodes/widgets/components/WidgetRecordAudio.vue
📚 Learning: 2026-01-12T17:39:27.738Z
Learnt from: DrJKL
Repo: Comfy-Org/ComfyUI_frontend PR: 7906
File: src/components/sidebar/tabs/AssetsSidebarTab.vue:545-552
Timestamp: 2026-01-12T17:39:27.738Z
Learning: In Vue/TypeScript files (src/**/*.{ts,tsx,vue}), prefer if/else statements over ternary operators when performing side effects or actions (e.g., mutating state, calling methods with side effects). Ternaries should be reserved for computing and returning values.
Applied to files:
src/renderer/extensions/vueNodes/widgets/components/WidgetRecordAudio.vue
🔇 Additional comments (1)
src/renderer/extensions/vueNodes/widgets/components/WidgetRecordAudio.vue (1)
13-42: Layout/overflow tweaks look good.The min-w-0 / shrink-0 adjustments plus overflow-hidden should prevent waveform spill while keeping the flex layout stable.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
Manual backport since the bot is slow ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-8084-backport-cloud-1-37-Further-linear-fixes-2e96d73d365081878a02d23ee2e848be) by [Unito](https://www.unito.io)
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/renderer/extensions/vueNodes/widgets/composables/useComboWidget.test.ts (1)
214-220: AssertopenModalis actually provided (not just “any object”).
expect.any(Object)allows regressions where the options object lacksopenModal. Tighten the assertion to match the new API contract.🧪 Suggested assertion tightening
- expect.any(Object) + expect.objectContaining({ openModal: expect.any(Function) })- expect.any(Object) + expect.objectContaining({ openModal: expect.any(Function) })- expect.any(Object) + expect.objectContaining({ openModal: expect.any(Function) })Also applies to: 250-256, 281-287
🤖 Fix all issues with AI agents
In `@src/renderer/extensions/linearMode/LinearControls.vue`:
- Around line 290-304: The toast is shown on mount because jobToastTimeout (the
ready ref from useTimeout) is false; add a new ref flag (e.g., hasQueuedJob) and
include it in the Teleport condition so the toast requires a queued job: change
the Teleport v-if to require hasQueuedJob && ((!jobToastTimeout ||
!jobFinishedQueue) && toastTo) and set hasQueuedJob.value = true inside the
function that actually queues a job (e.g., runButtonClick() or the job-enqueue
helper) and clear/reset hasQueuedJob when the job queue is emptied or cancelled
so the toast only appears after a job is queued.
In `@src/renderer/extensions/vueNodes/widgets/composables/useComboWidget.ts`:
- Around line 97-142: The openModal function is using the wrong this binding
(declared as this: IBaseWidget) so assigning this.value mutates the options
object instead of the widget; remove the this parameter from openModal, stop
using this.value, and assign to widget.value directly, then call
node.onWidgetChanged with widget.name, widget.value, oldValue, widget (leave
asset validation and filename logic unchanged); update the options creation
(IWidgetAssetOptions { openModal }) to reference the corrected openModal
function.
♻️ Duplicate comments (20)
src/renderer/extensions/linearMode/LinearPreview.vue (4)
54-65: Avoid empty stat entries from the media-type fallback.
?? {}yields a truthy object, so the filter keeps an empty entry and can render a blank stat.🧩 Suggested fix
- return [ - { content: formatTime(selectedItem.created_at) }, - { content: formatDuration(user_metadata.executionTimeInSeconds) }, - allOutputs && { content: t('g.asset', allOutputs.length) }, - (selectedOutput && mediaTypes[getMediaType(selectedOutput)]) ?? {} - ].filter((i) => !!i) + const mediaEntry = selectedOutput + ? mediaTypes[getMediaType(selectedOutput)] + : undefined + return [ + { content: formatTime(selectedItem.created_at) }, + { content: formatDuration(user_metadata.executionTimeInSeconds) }, + allOutputs && { content: t('g.asset', allOutputs.length) }, + mediaEntry + ].filter((i): i is StatItem => !!i && !!i.content)
87-95: Replace the fixed timeout with deterministic sync.
The 500ms delay is fragile and can still race widget updates.
124-133: Icon-only download button needs an accessible label.
Add an aria-label so screen readers can announce the action. Based on learnings, icon-only buttons require aria-labels.♿ Suggested fix
<Button size="icon" + :aria-label="t('g.download')" `@click`=" () => { if (selectedOutput?.url) downloadFile(selectedOutput.url) } " >
154-178: Guard selectedOutput before dereferencing.
The non-null assertions can still throw if media detection ever returns a value for undefined.🧩 Suggested fix
<ImagePreview v-if="latentPreview ?? getMediaType(selectedOutput) === 'images'" :mobile - :src="latentPreview ?? selectedOutput!.url" + :src="latentPreview ?? selectedOutput?.url" /> <VideoPreview v-else-if="getMediaType(selectedOutput) === 'video'" - :src="selectedOutput!.url" + :src="selectedOutput?.url ?? ''" class="object-contain flex-1 md:contain-size" /> <audio v-else-if="getMediaType(selectedOutput) === 'audio'" class="w-full m-auto" controls - :src="selectedOutput!.url" + :src="selectedOutput?.url" /> <article v-else-if="getMediaType(selectedOutput) === 'text'" class="w-full max-w-128 m-auto my-12 overflow-y-auto" - v-text="selectedOutput!.url" + v-text="selectedOutput?.url" /> <Preview3d v-else-if="getMediaType(selectedOutput) === '3d'" - :model-url="selectedOutput!.url" + :model-url="selectedOutput?.url ?? ''" />src/renderer/extensions/linearMode/DropZone.vue (2)
20-35: Add accessible labeling for the drop target.
Screen readers need a role/label to announce the drop region.♿ Suggested fix
<div v-if="onDragOver && onDragDrop" + role="region" + :aria-label="dropIndicator?.label" :class=" cn( 'rounded-lg ring-inset ring-primary-500', canAcceptDrop && 'ring-4 bg-primary-500/10' ) "
30-34: Await async onDragDrop before resetting state.
Otherwise failed async drops still clear the UI state.🧩 Suggested fix
`@drop.stop.prevent`=" - (e: DragEvent) => { - onDragDrop!(e) - canAcceptDrop = false - } + async (e: DragEvent) => { + try { + await onDragDrop!(e) + } finally { + canAcceptDrop = false + } + } "src/renderer/extensions/linearMode/LinearControls.vue (5)
72-75: Avoid mutating widget objects while mapping.
This mutates shared widget objects and can leak state across renders. As per coding guidelines, prefer immutability.🧩 Suggested fix
function nodeToNodeData(node: LGraphNode) { const dropIndicator = getDropIndicator(node) const nodeData = extractVueNodeData(node) - for (const widget of nodeData.widgets ?? []) widget.slotMetadata = undefined + const widgets = nodeData.widgets?.map((widget) => ({ + ...widget, + slotMetadata: undefined + })) return { - ...nodeData, + ...nodeData, + widgets, //note lastNodeErrors uses exeuctionid, node.id is execution for root hasErrors: !!executionStore.lastNodeErrors?.[node.id],
87-98: Make note-widget read_only changes immutable.
Mutating widget options inside the computed can cause reactive side effects. As per coding guidelines, prefer immutability.🧩 Suggested fix
const partitionedNodes = computed(() => { const parts = partition( graphNodes.value .filter((node) => node.mode === 0 && node.widgets?.length) .map(nodeToNodeData) .reverse(), (node) => ['MarkdownNote', 'Note'].includes(node.type) ) - for (const noteNode of parts[0]) { - for (const widget of noteNode.widgets ?? []) - widget.options = { ...widget.options, read_only: true } - } - return parts + const notesWithReadOnly = parts[0].map((noteNode) => ({ + ...noteNode, + widgets: noteNode.widgets?.map((widget) => ({ + ...widget, + options: { ...widget.options, read_only: true } + })) + })) + return [notesWithReadOnly, parts[1]] })
166-174: Icon-only interrupt buttons need accessible labels.
Add aria-labels so screen readers can announce the action. Based on learnings, icon-only buttons require aria-labels.♿ Suggested fix
<Button v-if="!executionStore.isIdle" variant="destructive" size="lg" class="w-10 p-2" + :aria-label="t('menu.interrupt')" `@click`="commandStore.execute('Comfy.Interrupt')" ><Button v-if="!executionStore.isIdle" variant="destructive" size="lg" class="w-10 p-2" + :aria-label="t('menu.interrupt')" `@click`="commandStore.execute('Comfy.Interrupt')" >Also applies to: 277-285
216-216: Remove dead publish button gated byv-if="false".
Either delete it or wire it to a real feature flag.
306-310: Remove dead Teleport block withv-if="false".
It will never render and adds maintenance noise.src/views/LinearView.vue (3)
74-84: Extract the repeated@update-selectionhandler.The same inline handler appears three times; pull it into a single function in
<script setup>to reduce duplication and keep updates consistent.Also applies to: 110-117, 175-182
88-91: Harden the external Typeform link.Open in a new tab and prevent
window.openeraccess.🛠️ Suggested fix
<a href="https://form.typeform.com/to/gmVqFi8l" + target="_blank" + rel="noopener noreferrer" v-text="t('linearMode.beta')" />
125-125: Avoid empty placeholder divs.If these are layout spacers, replace them with a semantic spacer (e.g.,
flex-1oraria-hiddenelement). Otherwise, remove them.Also applies to: 184-184
src/renderer/extensions/linearMode/OutputHistory.vue (6)
35-38: Use Vue 3.5 reactive props destructuring with defaults.This aligns with the repo’s Composition API guidelines and keeps props strongly typed. As per coding guidelines, prefer reactive destructuring with defaults.
♻️ Suggested refactor
-defineProps<{ - scrollResetButtonTo?: string | HTMLElement - mobile?: boolean -}>() +const { scrollResetButtonTo, mobile = false } = defineProps<{ + scrollResetButtonTo?: string | HTMLElement + mobile?: boolean +}>()
49-80: Consolidate the twowatch(selectedIndex)blocks.Merging them into a single watcher keeps side effects centralized and easier to reason about.
150-151: Avoid module‑scope DOM access and non‑reactive scroll state.
document.bodyat module scope can break SSR/tests, andscrollOffsetbypasses Vue reactivity.🛠️ Suggested fix
-let pointer = new CanvasPointer(document.body) -let scrollOffset = 0 +const scrollOffset = ref(0) +let pointer: CanvasPointer | null = null + +onMounted(() => { + pointer = new CanvasPointer(document.body) +})- if (!pointer.isTrackpadGesture(e)) { + if (!pointer?.isTrackpadGesture(e)) { if (e.deltaY > 0) gotoNextOutput() else gotoPreviousOutput() return } - scrollOffset += e.deltaY - while (scrollOffset >= 60) { - scrollOffset -= 60 + scrollOffset.value += e.deltaY + while (scrollOffset.value >= 60) { + scrollOffset.value -= 60 gotoNextOutput() } - while (scrollOffset <= -60) { - scrollOffset += 60 + while (scrollOffset.value <= -60) { + scrollOffset.value += 60 gotoPreviousOutput() }
250-252: Use a stable key for outputs.Index keys can break DOM reuse when items are inserted/removed; prefer a stable identifier.
🛠️ Suggested fix
- <section - v-for="(item, index) in outputs.media.value" - :key="index" + <section + v-for="(item, index) in outputs.media.value" + :key="item.id"
257-268: Add descriptivealttext for output images.Images need accessible text; use output indices and a localized fallback. As per coding guidelines, use i18n for new strings.
🛠️ Suggested fix
+import { t } from '@/i18n'<img v-if="getMediaType(output) === 'images'" :class=" cn( 'p-1 rounded-lg aspect-square object-cover', index === selectedIndex[0] && key === selectedIndex[1] && 'border-2' ) " :src="output.url" + :alt="t('linearMode.outputImage', { index: index + 1, output: key + 1 })" `@click`="selectedIndex = [index, key]" />Add the
linearMode.outputImageentry tosrc/locales/en/main.json.
296-307: Add an accessible label for the icon‑only scroll button.Icon‑only buttons need a label; use a localized string. As per coding guidelines, use i18n for new strings.
🛠️ Suggested fix
<Button :class=" cn( 'p-3 size-10 bg-base-foreground', settingStore.get('Comfy.Sidebar.Location') === 'left' ? 'left-4' : 'right-4' ) " + :aria-label="t('linearMode.scrollToTop')" `@click`="resetOutputsScroll" >Add the
linearMode.scrollToTopentry tosrc/locales/en/main.json(reusing thetimport from above).
| <Teleport | ||
| v-if="(!jobToastTimeout || !jobFinishedQueue) && toastTo" | ||
| defer | ||
| :to="toastTo" | ||
| > | ||
| <div | ||
| class="bg-base-foreground text-base-background rounded-sm flex h-8 p-1 pr-2 gap-2 items-center" | ||
| > | ||
| <i | ||
| v-if="jobFinishedQueue" | ||
| class="icon-[lucide--check] size-5 bg-success-background" | ||
| /> | ||
| <i v-else class="icon-[lucide--loader-circle] size-4 animate-spin" /> | ||
| <span v-text="t('queue.jobAddedToQueue')" /> | ||
| </div> |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's examine the LinearControls.vue file to understand the context
fd LinearControls.vueRepository: Comfy-Org/ComfyUI_frontend
Length of output: 299
🏁 Script executed:
# Read the file to understand the useTimeout setup and toast logic
cat -n src/renderer/extensions/linearMode/LinearControls.vue | head -150Repository: Comfy-Org/ComfyUI_frontend
Length of output: 299
🏁 Script executed:
# Get more context around the areas mentioned in the review
cat -n src/renderer/extensions/linearMode/LinearControls.vue | sed -n '30,50p'Repository: Comfy-Org/ComfyUI_frontend
Length of output: 299
🏁 Script executed:
# Check the useTimeout initialization and related refs
cat -n src/renderer/extensions/linearMode/LinearControls.vue | sed -n '100,145p'Repository: Comfy-Org/ComfyUI_frontend
Length of output: 299
🌐 Web query:
VueUse useTimeout immediate false semantics ready ref
💡 Result:
Short answer: useTimeout(interval, immediate = true) returns a ready Ref that is false until the timeout finishes. If you pass immediate: false, the timer does NOT start automatically and ready stays false until you call start(). When the timer runs and completes, ready.value becomes true; isActive (or isPending for useTimeoutFn) is true while the timeout is active. start() (re)starts the timer (resetting ready to false until it completes); stop() cancels it. See the API docs. [1][2]
Sources:
- useTimeout docs. [1]
- useTimeoutFn docs (controls / isPending). [2]
Toast renders before any job is queued.
With useTimeout(..., immediate: false), the ready ref (aliased as jobToastTimeout) is false on mount until start() is called. The toast condition (!jobToastTimeout || !jobFinishedQueue) && toastTo evaluates to true when the component mounts because !jobToastTimeout is true, allowing the toast to display before runButtonClick() is invoked. Add a flag to track whether a job has been queued to prevent this.
🧩 Suggested fix
-const jobFinishedQueue = ref(true)
+const jobFinishedQueue = ref(true)
+const hasQueuedJob = ref(false)
const { ready: jobToastTimeout, start: resetJobToastTimeout } = useTimeout(
5000,
{ controls: true, immediate: false }
)
async function runButtonClick(e: Event) {
if (!jobFinishedQueue.value) return
try {
+ hasQueuedJob.value = true
jobFinishedQueue.value = false
resetJobToastTimeout()
const isShiftPressed = 'shiftKey' in e && e.shiftKey
const commandId = isShiftPressed
? 'Comfy.QueuePromptFront'
: 'Comfy.QueuePrompt'
@@
- <Teleport
- v-if="(!jobToastTimeout || !jobFinishedQueue) && toastTo"
+ <Teleport
+ v-if="hasQueuedJob && (!jobToastTimeout || !jobFinishedQueue) && toastTo"
defer
:to="toastTo"
>🤖 Prompt for AI Agents
In `@src/renderer/extensions/linearMode/LinearControls.vue` around lines 290 -
304, The toast is shown on mount because jobToastTimeout (the ready ref from
useTimeout) is false; add a new ref flag (e.g., hasQueuedJob) and include it in
the Teleport condition so the toast requires a queued job: change the Teleport
v-if to require hasQueuedJob && ((!jobToastTimeout || !jobFinishedQueue) &&
toastTo) and set hasQueuedJob.value = true inside the function that actually
queues a job (e.g., runButtonClick() or the job-enqueue helper) and clear/reset
hasQueuedJob when the job queue is emptied or cancelled so the toast only
appears after a job is queued.
src/renderer/extensions/vueNodes/widgets/composables/useComboWidget.ts
Outdated
Show resolved
Hide resolved
#8088) Manual backport of #8086 (this time to the correct target branch) ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-8088-backport-cloud-1-37-Make-sure-toggle-visibility-checks-remote-config-2ea6d73d3650813b8207d12ed42541f5) by [Unito](https://www.unito.io)
Backport of #8094 to `cloud/1.37` Automatically created by backport workflow. ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-8096-backport-cloud-1-37-Fix-copypasted-primitives-inside-subgraphs-2ea6d73d3650812e8692eb76149d8156) by [Unito](https://www.unito.io) Co-authored-by: AustinMroz <austin@comfy.org>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/lib/litegraph/src/LGraphCanvas.ts`:
- Around line 4044-4046: The current calls use forEachNode(graph, ...) which
iterates the whole graph and fires onGraphConfigured/onAfterGraphConfigured on
unrelated nodes; change this to only traverse the newly pasted items by
iterating over the created array and invoking the hooks for each created node
and any nodes inside its subgraph(s) (e.g., for each item in created, call a
traversal like forEachNode(item, n => n.onGraphConfigured?.()) and similarly for
onAfterGraphConfigured) so only newly created nodes and their nested contents
receive the hooks; keep the selectItems(created) call as-is.
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
src/lib/litegraph/src/LGraphCanvas.ts
🧰 Additional context used
📓 Path-based instructions (8)
src/**/*.{vue,ts}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
src/**/*.{vue,ts}: Leverage VueUse functions for performance-enhancing styles
Implement proper error handling
Use vue-i18n in composition API for any string literals. Place new translation entries in src/locales/en/main.json
Files:
src/lib/litegraph/src/LGraphCanvas.ts
src/**/*.ts
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
src/**/*.ts: Use es-toolkit for utility functions
Use TypeScript for type safety
src/**/*.ts: Derive component types usingvue-component-type-helpers(ComponentProps,ComponentSlots) instead of separate type files
Use es-toolkit for utility functions
Minimize the surface area (exported values) of each module and composable
Favor pure functions, especially testable ones
Files:
src/lib/litegraph/src/LGraphCanvas.ts
src/**/*.{ts,tsx,vue}
📄 CodeRabbit inference engine (src/CLAUDE.md)
src/**/*.{ts,tsx,vue}: Sanitize HTML with DOMPurify to prevent XSS attacks
Avoid using@ts-expect-error; use proper TypeScript types instead
Use es-toolkit for utility functions instead of other utility libraries
Implement proper TypeScript types throughout the codebase
src/**/*.{ts,tsx,vue}: Use separateimport typestatements instead of inlinetypein mixed imports
Apply Prettier formatting with 2-space indentation, single quotes, no trailing semicolons, 80-character width
Sort and group imports by plugin, runpnpm formatbefore committing
Never useanytype - use proper TypeScript types
Never useas anytype assertions - fix the underlying type issue
Write code that is expressive and self-documenting - avoid unnecessary comments
Do not add or retain redundant comments - clean as you go
Avoid mutable state - prefer immutability and assignment at point of declaration
Watch out for Code Smells and refactor to avoid them
Files:
src/lib/litegraph/src/LGraphCanvas.ts
src/**/*.{vue,ts,tsx}
📄 CodeRabbit inference engine (src/CLAUDE.md)
Follow Vue 3 composition API style guide
Files:
src/lib/litegraph/src/LGraphCanvas.ts
src/lib/litegraph/**/*.{js,ts,jsx,tsx}
📄 CodeRabbit inference engine (src/lib/litegraph/CLAUDE.md)
src/lib/litegraph/**/*.{js,ts,jsx,tsx}: Run ESLint instead of manually figuring out whitespace fixes or other trivial style concerns using thepnpm lint:fixcommand
Take advantage ofTypedArraysubarraywhen appropriate
Thesizeandposproperties ofRectangleshare the same array buffer (subarray); they may be used to set the rectangle's size and position
Prefer single lineifsyntax over adding curly braces, when the statement has a very concise expression and concise, single line statement
Do not replace&&=or||=with=when there is no reason to do so. If you do find a reason to remove either&&=or||=, leave a comment explaining why the removal occurred
When writing methods, prefer returning idiomatic JavaScriptundefinedovernull
Files:
src/lib/litegraph/src/LGraphCanvas.ts
src/lib/litegraph/**/*.{ts,tsx}
📄 CodeRabbit inference engine (src/lib/litegraph/CLAUDE.md)
Type assertions are an absolute last resort. In almost all cases, they are a crutch that leads to brittle code
Files:
src/lib/litegraph/src/LGraphCanvas.ts
src/**/*.{ts,vue}
📄 CodeRabbit inference engine (AGENTS.md)
src/**/*.{ts,vue}: Usereffor reactive state,computed()for derived values, andwatch/watchEffectfor side effects in Composition API
Avoid usingrefwithwatchif acomputedwould suffice - minimize refs and derived state
Useprovide/injectfor dependency injection only when simpler alternatives (Store or shared composable) won't work
Leverage VueUse functions for performance-enhancing composables
Use VueUse function for useI18n in composition API for string literals
Files:
src/lib/litegraph/src/LGraphCanvas.ts
src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
src/**/*.{ts,tsx}: Keep functions short and functional
Minimize nesting (if statements, for loops, etc.)
Use function declarations instead of function expressions when possible
Files:
src/lib/litegraph/src/LGraphCanvas.ts
🧠 Learnings (8)
📚 Learning: 2025-11-24T19:47:56.371Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: src/lib/litegraph/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:47:56.371Z
Learning: Applies to src/lib/litegraph/**/*.{test,spec}.{ts,tsx} : Use provided test helpers `createTestSubgraph` and `createTestSubgraphNode` from `./fixtures/subgraphHelpers` for consistent subgraph test setup
Applied to files:
src/lib/litegraph/src/LGraphCanvas.ts
📚 Learning: 2025-11-24T19:47:56.371Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: src/lib/litegraph/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:47:56.371Z
Learning: Applies to src/lib/litegraph/**/*.{test,spec}.{js,ts,jsx,tsx} : When adding features, always write vitest unit tests using cursor rules in @.cursor
Applied to files:
src/lib/litegraph/src/LGraphCanvas.ts
📚 Learning: 2025-12-09T03:39:54.501Z
Learnt from: DrJKL
Repo: Comfy-Org/ComfyUI_frontend PR: 7169
File: src/platform/remote/comfyui/jobs/jobTypes.ts:1-107
Timestamp: 2025-12-09T03:39:54.501Z
Learning: In the ComfyUI_frontend project, Zod is on v3.x. Do not suggest Zod v4 standalone validators (z.uuid, z.ulid, z.cuid2, z.nanoid) until an upgrade to Zod 4 is performed. When reviewing TypeScript files (e.g., src/platform/remote/comfyui/jobs/jobTypes.ts) validate against Zod 3 capabilities and avoid introducing v4-specific features; flag any proposal to upgrade or incorporate v4-only validators and propose staying with compatible 3.x patterns.
Applied to files:
src/lib/litegraph/src/LGraphCanvas.ts
📚 Learning: 2025-12-13T11:03:11.264Z
Learnt from: christian-byrne
Repo: Comfy-Org/ComfyUI_frontend PR: 7416
File: src/stores/imagePreviewStore.ts:5-7
Timestamp: 2025-12-13T11:03:11.264Z
Learning: In the ComfyUI_frontend repository, lint rules require keeping 'import type' statements separate from non-type imports, even if importing from the same module. Do not suggest consolidating them into a single import statement. Ensure type imports remain on their own line (import type { ... } from 'module') and regular imports stay on separate lines.
Applied to files:
src/lib/litegraph/src/LGraphCanvas.ts
📚 Learning: 2025-12-17T00:40:09.635Z
Learnt from: DrJKL
Repo: Comfy-Org/ComfyUI_frontend PR: 7537
File: src/components/ui/button/Button.stories.ts:45-55
Timestamp: 2025-12-17T00:40:09.635Z
Learning: Prefer pure function declarations over function expressions (e.g., use function foo() { ... } instead of const foo = () => { ... }) for pure functions in the repository. Function declarations are more functional-leaning, offer better hoisting clarity, and can improve readability and tooling consistency. Apply this guideline across TypeScript files in Comfy-Org/ComfyUI_frontend, including story and UI component code, except where a function expression is semantically required (e.g., callbacks, higher-order functions with closures).
Applied to files:
src/lib/litegraph/src/LGraphCanvas.ts
📚 Learning: 2025-12-30T22:22:33.836Z
Learnt from: kaili-yang
Repo: Comfy-Org/ComfyUI_frontend PR: 7805
File: src/composables/useCoreCommands.ts:439-439
Timestamp: 2025-12-30T22:22:33.836Z
Learning: When accessing reactive properties from Pinia stores in TypeScript files, avoid using .value on direct property access (e.g., useStore().isOverlayExpanded). Pinia auto-wraps refs when accessed directly, returning the primitive value. The .value accessor is only needed when destructuring store properties or when using storeToRefs().
Applied to files:
src/lib/litegraph/src/LGraphCanvas.ts
📚 Learning: 2025-12-11T12:25:15.470Z
Learnt from: christian-byrne
Repo: Comfy-Org/ComfyUI_frontend PR: 7358
File: src/components/dialog/content/signin/SignUpForm.vue:45-54
Timestamp: 2025-12-11T12:25:15.470Z
Learning: This repository uses CI automation to format code (pnpm format). Do not include manual formatting suggestions in code reviews for Comfy-Org/ComfyUI_frontend. If formatting issues are detected, rely on the CI formatter or re-run pnpm format. Focus reviews on correctness, readability, performance, accessibility, and maintainability rather than style formatting.
Applied to files:
src/lib/litegraph/src/LGraphCanvas.ts
📚 Learning: 2026-01-12T17:39:27.738Z
Learnt from: DrJKL
Repo: Comfy-Org/ComfyUI_frontend PR: 7906
File: src/components/sidebar/tabs/AssetsSidebarTab.vue:545-552
Timestamp: 2026-01-12T17:39:27.738Z
Learning: In Vue/TypeScript files (src/**/*.{ts,tsx,vue}), prefer if/else statements over ternary operators when performing side effects or actions (e.g., mutating state, calling methods with side effects). Ternaries should be reserved for computing and returning values.
Applied to files:
src/lib/litegraph/src/LGraphCanvas.ts
🧬 Code graph analysis (1)
src/lib/litegraph/src/LGraphCanvas.ts (2)
src/utils/graphTraversalUtil.ts (1)
forEachNode(156-169)src/scripts/app.ts (1)
graph(165-167)
🔇 Additional comments (1)
src/lib/litegraph/src/LGraphCanvas.ts (1)
11-11: LGTM – traversal util import is used.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
| this.selectItems(created) | ||
| forEachNode(graph, (n) => n.onGraphConfigured?.()) | ||
| forEachNode(graph, (n) => n.onAfterGraphConfigured?.()) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's examine the actual code around lines 4044-4046
sed -n '4030,4060p' src/lib/litegraph/src/LGraphCanvas.tsRepository: Comfy-Org/ComfyUI_frontend
Length of output: 851
🏁 Script executed:
# Check the forEachNode implementation
rg -A 15 "function forEachNode|export.*forEachNode" src/lib/litegraphRepository: Comfy-Org/ComfyUI_frontend
Length of output: 52
🏁 Script executed:
# Understand the context - look at the broader paste operation
sed -n '4000,4070p' src/lib/litegraph/src/LGraphCanvas.tsRepository: Comfy-Org/ComfyUI_frontend
Length of output: 1931
🏁 Script executed:
# Search for other uses of these hooks (onGraphConfigured, onAfterGraphConfigured) in the codebase
rg "onGraphConfigured|onAfterGraphConfigured" src/lib/litegraph --context 3Repository: Comfy-Org/ComfyUI_frontend
Length of output: 1171
🏁 Script executed:
# Check the signature of 'created' variable in the paste context
sed -n '3950,4050p' src/lib/litegraph/src/LGraphCanvas.ts | head -150Repository: Comfy-Org/ComfyUI_frontend
Length of output: 2926
🏁 Script executed:
# Find where forEachNode is defined
rg "forEachNode" src/lib/litegraph --type ts --type tsxRepository: Comfy-Org/ComfyUI_frontend
Length of output: 97
🏁 Script executed:
# Look for the function definition more broadly
rg -B 2 -A 10 "forEachNode\s*=" src/lib/litegraphRepository: Comfy-Org/ComfyUI_frontend
Length of output: 52
🏁 Script executed:
# Check imports in LGraphCanvas.ts to see where forEachNode comes from
head -100 src/lib/litegraph/src/LGraphCanvas.ts | grep -E "import|from"Repository: Comfy-Org/ComfyUI_frontend
Length of output: 2623
🏁 Script executed:
# Look at the broader context of _pasteFromClipboard to see what method we're in
sed -n '3900,4000p' src/lib/litegraph/src/LGraphCanvas.ts | head -150Repository: Comfy-Org/ComfyUI_frontend
Length of output: 3082
🏁 Script executed:
# Check if there are existing patterns of selective node configuration in the codebase
rg "\.onGraphConfigured\|\.onAfterGraphConfigured" src/lib/litegraph -B 3 -A 1Repository: Comfy-Org/ComfyUI_frontend
Length of output: 52
🏁 Script executed:
# Find forEachNode in graphTraversalUtil
find src -name "*graphTraversal*" -type fRepository: Comfy-Org/ComfyUI_frontend
Length of output: 138
🏁 Script executed:
# Check the implementation of forEachNode
cat src/utils/graphTraversalUtil.tsRepository: Comfy-Org/ComfyUI_frontend
Length of output: 17233
🏁 Script executed:
# Let's also look at LGraph.ts to see how it uses forEachNode in the subgraph configuration
sed -n '1,100p' src/lib/litegraph/src/LGraph.ts | grep -E "forEachNode|import"Repository: Comfy-Org/ComfyUI_frontend
Length of output: 1553
🏁 Script executed:
# Look for the broader paste method signature to understand the full context
sed -n '3900,3950p' src/lib/litegraph/src/LGraphCanvas.tsRepository: Comfy-Org/ComfyUI_frontend
Length of output: 1690
🏁 Script executed:
# Search for other uses of onGraphConfigured and onAfterGraphConfigured to understand the intended pattern
rg "onGraphConfigured|onAfterGraphConfigured" src --context 4 -A 2 -B 2Repository: Comfy-Org/ComfyUI_frontend
Length of output: 4375
🏁 Script executed:
# Check if there are any tests or documentation about the expected behavior of these hooks
find src -name "*.test.ts" -o -name "*.spec.ts" | xargs grep -l "onGraphConfigured\|onAfterGraphConfigured" 2>/dev/nullRepository: Comfy-Org/ComfyUI_frontend
Length of output: 54
🏁 Script executed:
# Look at the method signature and surrounding context to see if there's any documentation or comments
sed -n '3880,3920p' src/lib/litegraph/src/LGraphCanvas.tsRepository: Comfy-Org/ComfyUI_frontend
Length of output: 1565
🏁 Script executed:
# Check what LGraphNode.onGraphConfigured and onAfterGraphConfigured are used for
rg "onGraphConfigured\s*=" src/lib/litegraph -B 2 -A 5Repository: Comfy-Org/ComfyUI_frontend
Length of output: 52
Scope configuration hooks to newly pasted nodes only.
forEachNode(graph, …) traverses the entire graph including all nested subgraphs, so pasting fires hooks on unrelated existing nodes. This causes duplicate side effects and unnecessary work. Fire hooks only on the newly created items (created array) and their subgraph contents, consistent with how LGraph.ts handles subgraph node configuration.
💡 Suggested fix
- this.selectItems(created)
- forEachNode(graph, (n) => n.onGraphConfigured?.())
- forEachNode(graph, (n) => n.onAfterGraphConfigured?.())
+ this.selectItems(created)
+
+ const configuredNodes = new Set<LGraphNode>()
+ function collectNode(node: LGraphNode) {
+ if (configuredNodes.has(node)) return
+ configuredNodes.add(node)
+ if (node.isSubgraphNode?.() && node.subgraph) {
+ forEachNode(node.subgraph, collectNode)
+ }
+ }
+
+ for (const item of created) {
+ if (item instanceof LGraphNode) collectNode(item)
+ }
+
+ for (const node of configuredNodes) node.onGraphConfigured?.()
+ for (const node of configuredNodes) node.onAfterGraphConfigured?.()📝 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.
| this.selectItems(created) | |
| forEachNode(graph, (n) => n.onGraphConfigured?.()) | |
| forEachNode(graph, (n) => n.onAfterGraphConfigured?.()) | |
| this.selectItems(created) | |
| const configuredNodes = new Set<LGraphNode>() | |
| function collectNode(node: LGraphNode) { | |
| if (configuredNodes.has(node)) return | |
| configuredNodes.add(node) | |
| if (node.isSubgraphNode?.() && node.subgraph) { | |
| forEachNode(node.subgraph, collectNode) | |
| } | |
| } | |
| for (const item of created) { | |
| if (item instanceof LGraphNode) collectNode(item) | |
| } | |
| for (const node of configuredNodes) node.onGraphConfigured?.() | |
| for (const node of configuredNodes) node.onAfterGraphConfigured?.() |
🤖 Prompt for AI Agents
In `@src/lib/litegraph/src/LGraphCanvas.ts` around lines 4044 - 4046, The current
calls use forEachNode(graph, ...) which iterates the whole graph and fires
onGraphConfigured/onAfterGraphConfigured on unrelated nodes; change this to only
traverse the newly pasted items by iterating over the created array and invoking
the hooks for each created node and any nodes inside its subgraph(s) (e.g., for
each item in created, call a traversal like forEachNode(item, n =>
n.onGraphConfigured?.()) and similarly for onAfterGraphConfigured) so only newly
created nodes and their nested contents receive the hooks; keep the
selectItems(created) call as-is.
…utton metadata (#8108) Manual backport of #8107 to `cloud/1.37` ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-8108-backport-cloud-1-37-Improve-linear-compatibility-with-Safari-run-button-metadata-2ea6d73d365081e79cc9f920f852a8a2) by [Unito](https://www.unito.io)
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@src/components/sidebar/ModeToggle.vue`:
- Around line 16-39: The icon-only Button components in ModeToggle.vue lack
accessible labels; update both Button elements to include descriptive aria-label
attributes (e.g., aria-label="Switch to linear mode" and aria-label="Switch to
graph mode") that reflect their actions, keeping existing props and the
`@click`="toggleLinearMode" handler and canvasStore.linearMode variants unchanged
so screen readers can announce the purpose of the buttons.
In `@src/renderer/extensions/linearMode/LinearPreview.vue`:
- Around line 134-151: The Popover entry actions currently dereference
selectedItem with a non-null assertion (selectedItem!) which can throw if
selectedItem becomes undefined later; update the action callbacks for
downloadAsset and mediaActions.confirmDelete inside the Popover entries to
defensively check that selectedItem is truthy before calling
downloadAsset(selectedItem) or mediaActions.confirmDelete(selectedItem), and
fail gracefully (no-op or show an error/disable the menu) when selectedItem is
missing; locate these in the Popover declaration referencing selectedItem,
downloadAsset, and mediaActions.confirmDelete and add the guard logic inside the
arrow functions.
♻️ Duplicate comments (11)
src/components/sidebar/ModeToggle.vue (1)
3-3: UseuseI18n()composable instead of directtimport.Per coding guidelines, use the vue-i18n composable pattern for i18n in Composition API:
-import { t } from '@/i18n' +import { useI18n } from 'vue-i18n'Then in the script:
const { t } = useI18n()src/renderer/extensions/linearMode/OutputHistory.vue (5)
44-47: Consider Vue 3.5 reactive props destructuring.Per coding guidelines, prefer destructuring props with defaults for cleaner template usage.
♻️ Suggested refactor
-defineProps<{ - scrollResetButtonTo?: string | HTMLElement - mobile?: boolean -}>() +const { scrollResetButtonTo, mobile = false } = defineProps<{ + scrollResetButtonTo?: string | HTMLElement + mobile?: boolean +}>()
161-162: SSR hazard and mutable state outside reactivity.
CanvasPointer(document.body)at module scope fails during SSR/tests wheredocumentis unavailable.scrollOffsetas a plainletis mutable state outside Vue's reactive system.🐛 Proposed fix
-let pointer = new CanvasPointer(document.body) -let scrollOffset = 0 +import { onMounted } from 'vue' + +const scrollOffset = ref(0) +let pointer: CanvasPointer | null = null + +onMounted(() => { + pointer = new CanvasPointer(document.body) +}) + function onWheel(e: WheelEvent) { if (!e.ctrlKey && !e.metaKey) return e.preventDefault() e.stopPropagation() - if (!pointer.isTrackpadGesture(e)) { + if (!pointer?.isTrackpadGesture(e)) { if (e.deltaY > 0) gotoNextOutput() else gotoPreviousOutput() return } - scrollOffset += e.deltaY - while (scrollOffset >= 60) { - scrollOffset -= 60 + scrollOffset.value += e.deltaY + while (scrollOffset.value >= 60) { + scrollOffset.value -= 60 gotoNextOutput() } - while (scrollOffset <= -60) { - scrollOffset += 60 + while (scrollOffset.value <= -60) { + scrollOffset.value += 60 gotoPreviousOutput() } }
273-276: Use stable identifier forv-forkey.Using
indexas the key causes rendering issues when items are added, removed, or reordered. The item has anidproperty (used on line 120-121).🐛 Proposed fix
- <template v-for="(item, index) in outputs.media.value" :key="index"> + <template v-for="(item, index) in outputs.media.value" :key="item.id">
278-291: Addaltattribute for accessibility.Images should have descriptive
alttext for screen readers.♿ Proposed fix
<img v-if="getMediaType(output) === 'images'" :class=" cn( 'p-1 rounded-lg aspect-square object-cover not-md:h-20 md:w-full', index === selectedIndex[0] && key === selectedIndex[1] && 'border-2' ) " :data-output-index="index" :src="output.url" + :alt="t('linearMode.outputThumbnail', { index: index + 1, key: key + 1 })" `@click`="selectedIndex = [index, key]" />
319-331: Addaria-labelfor icon-only button.Per accessibility guidelines, icon-only buttons need an accessible label.
♿ Proposed fix
<Button :class=" cn( 'p-3 size-10 bg-base-foreground', settingStore.get('Comfy.Sidebar.Location') === 'left' ? 'left-4' : 'right-4' ) " + :aria-label="t('linearMode.scrollToTop')" `@click`="resetOutputsScroll" > <i class="icon-[lucide--arrow-up] size-4 bg-base-background" /> </Button>src/renderer/extensions/linearMode/LinearPreview.vue (5)
29-35: Consider Vue 3.5 style reactive props destructuring.Per coding guidelines, prefer full destructuring with defaults for consistency. Currently
latentPreviewandmobileare accessed via implicit template scope while others are destructured.♻️ Suggested refactor
-const { runButtonClick, selectedItem, selectedOutput } = defineProps<{ +const { + latentPreview, + runButtonClick, + selectedItem, + selectedOutput, + mobile = false +} = defineProps<{ latentPreview?: string runButtonClick?: (e: Event) => void selectedItem?: AssetItem selectedOutput?: ResultItemImpl mobile?: boolean }>()
60-66: Empty objects pass through the filter.When
mediaTypes[getMediaType(selectedOutput)]is undefined, the fallback?? {}creates an empty object that passes the!!icheck. This may render empty stat items in the UI.🐛 Proposed fix
return [ { content: formatTime(selectedItem.created_at) }, { content: formatDuration(user_metadata.executionTimeInSeconds) }, allOutputs && { content: t('g.asset', allOutputs.length) }, - (selectedOutput && mediaTypes[getMediaType(selectedOutput)]) ?? {} - ].filter((i) => !!i) + selectedOutput && mediaTypes[getMediaType(selectedOutput)] + ].filter((i): i is StatItem => !!i && !!i.content) })
87-96: Race condition risk with hardcoded timeout.The
FIXMEcomment acknowledges this issue. The 500ms timeout is a fragile workaround that may fail under varying load conditions. The comment also notes seeds fail to update properly.Consider implementing a more robust synchronization mechanism, such as:
- Waiting for a specific event/signal that the graph is fully loaded
- Using a polling mechanism with exponential backoff
- Leveraging Vue's
nextTickcombined with graph state checksWould you like me to suggest an event-driven approach that waits for the graph to be fully loaded before executing widgets?
124-133: Addaria-labelfor icon-only download button.Per accessibility guidelines, icon-only buttons need an accessible label.
♿ Proposed fix
<Button size="icon" + :aria-label="t('g.download')" `@click`=" () => { if (selectedOutput?.url) downloadFile(selectedOutput.url) } " > <i class="icon-[lucide--download]" /> </Button>
154-178: Non-null assertions onselectedOutputmay cause runtime errors.Multiple conditional branches use
selectedOutput!after checkinggetMediaType(selectedOutput). IfgetMediaTypereturns a truthy value forundefinedinput, accessingselectedOutput!.urlwill throw. Use optional chaining for safety.🐛 Proposed fix
<ImagePreview v-if="latentPreview ?? getMediaType(selectedOutput) === 'images'" :mobile - :src="latentPreview ?? selectedOutput!.url" + :src="latentPreview ?? selectedOutput?.url" /> <VideoPreview v-else-if="getMediaType(selectedOutput) === 'video'" - :src="selectedOutput!.url" + :src="selectedOutput?.url ?? ''" class="object-contain flex-1 md:contain-size md:p-3" /> <audio v-else-if="getMediaType(selectedOutput) === 'audio'" class="w-full m-auto" controls - :src="selectedOutput!.url" + :src="selectedOutput?.url" /> <article v-else-if="getMediaType(selectedOutput) === 'text'" class="w-full max-w-128 m-auto my-12 overflow-y-auto" - v-text="selectedOutput!.url" + v-text="selectedOutput?.url" /> <Preview3d v-else-if="getMediaType(selectedOutput) === '3d'" - :model-url="selectedOutput!.url" + :model-url="selectedOutput?.url ?? ''" />
| <Button | ||
| v-tooltip="{ | ||
| value: t('linearMode.linearMode'), | ||
| showDelay: 300, | ||
| hideDelay: 300 | ||
| }" | ||
| size="icon" | ||
| :variant="canvasStore.linearMode ? 'inverted' : 'secondary'" | ||
| @click="toggleLinearMode" | ||
| > | ||
| <i class="icon-[lucide--panels-top-left]" /> | ||
| </Button> | ||
| <Button | ||
| v-tooltip="{ | ||
| value: t('linearMode.graphMode'), | ||
| showDelay: 300, | ||
| hideDelay: 300 | ||
| }" | ||
| size="icon" | ||
| :variant="canvasStore.linearMode ? 'secondary' : 'inverted'" | ||
| @click="toggleLinearMode" | ||
| > | ||
| <i class="icon-[comfy--workflow]" /> | ||
| </Button> |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Add aria-label to icon-only buttons for accessibility.
These buttons have no visible text—only icons. Screen reader users need aria-label to understand the button's purpose. The tooltip alone doesn't provide accessible labeling.
♿ Proposed accessibility fix
<Button
v-tooltip="{
value: t('linearMode.linearMode'),
showDelay: 300,
hideDelay: 300
}"
size="icon"
:variant="canvasStore.linearMode ? 'inverted' : 'secondary'"
+ :aria-label="t('linearMode.linearMode')"
`@click`="toggleLinearMode"
>
<i class="icon-[lucide--panels-top-left]" />
</Button>
<Button
v-tooltip="{
value: t('linearMode.graphMode'),
showDelay: 300,
hideDelay: 300
}"
size="icon"
:variant="canvasStore.linearMode ? 'secondary' : 'inverted'"
+ :aria-label="t('linearMode.graphMode')"
`@click`="toggleLinearMode"
>
<i class="icon-[comfy--workflow]" />
</Button>Based on learnings: "Use aria-label only for elements without visible labels (e.g., icon-only buttons)."
🤖 Prompt for AI Agents
In `@src/components/sidebar/ModeToggle.vue` around lines 16 - 39, The icon-only
Button components in ModeToggle.vue lack accessible labels; update both Button
elements to include descriptive aria-label attributes (e.g., aria-label="Switch
to linear mode" and aria-label="Switch to graph mode") that reflect their
actions, keeping existing props and the `@click`="toggleLinearMode" handler and
canvasStore.linearMode variants unchanged so screen readers can announce the
purpose of the buttons.
| <Popover | ||
| :entries="[ | ||
| [ | ||
| { | ||
| icon: 'icon-[lucide--download]', | ||
| label: t('linearMode.downloadAll'), | ||
| action: () => downloadAsset(selectedItem!) | ||
| } | ||
| ], | ||
| [ | ||
| { | ||
| icon: 'icon-[lucide--trash-2]', | ||
| label: t('queue.jobMenu.deleteAsset'), | ||
| action: () => mediaActions.confirmDelete(selectedItem!) | ||
| } | ||
| ] | ||
| ]" | ||
| /> |
There was a problem hiding this comment.
Non-null assertions in Popover actions may cause runtime errors.
The actions reference selectedItem! but these callbacks could theoretically be invoked after selectedItem becomes undefined (e.g., if the asset is deleted while the popover is open). Consider defensive checks.
🐛 Proposed fix
<Popover
:entries="[
[
{
icon: 'icon-[lucide--download]',
label: t('linearMode.downloadAll'),
- action: () => downloadAsset(selectedItem!)
+ action: () => selectedItem && downloadAsset(selectedItem)
}
],
[
{
icon: 'icon-[lucide--trash-2]',
label: t('queue.jobMenu.deleteAsset'),
- action: () => mediaActions.confirmDelete(selectedItem!)
+ action: () => selectedItem && mediaActions.confirmDelete(selectedItem)
}
]
]"
/>📝 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.
| <Popover | |
| :entries="[ | |
| [ | |
| { | |
| icon: 'icon-[lucide--download]', | |
| label: t('linearMode.downloadAll'), | |
| action: () => downloadAsset(selectedItem!) | |
| } | |
| ], | |
| [ | |
| { | |
| icon: 'icon-[lucide--trash-2]', | |
| label: t('queue.jobMenu.deleteAsset'), | |
| action: () => mediaActions.confirmDelete(selectedItem!) | |
| } | |
| ] | |
| ]" | |
| /> | |
| <Popover | |
| :entries="[ | |
| [ | |
| { | |
| icon: 'icon-[lucide--download]', | |
| label: t('linearMode.downloadAll'), | |
| action: () => selectedItem && downloadAsset(selectedItem) | |
| } | |
| ], | |
| [ | |
| { | |
| icon: 'icon-[lucide--trash-2]', | |
| label: t('queue.jobMenu.deleteAsset'), | |
| action: () => selectedItem && mediaActions.confirmDelete(selectedItem) | |
| } | |
| ] | |
| ]" | |
| /> |
🤖 Prompt for AI Agents
In `@src/renderer/extensions/linearMode/LinearPreview.vue` around lines 134 - 151,
The Popover entry actions currently dereference selectedItem with a non-null
assertion (selectedItem!) which can throw if selectedItem becomes undefined
later; update the action callbacks for downloadAsset and
mediaActions.confirmDelete inside the Popover entries to defensively check that
selectedItem is truthy before calling downloadAsset(selectedItem) or
mediaActions.confirmDelete(selectedItem), and fail gracefully (no-op or show an
error/disable the menu) when selectedItem is missing; locate these in the
Popover declaration referencing selectedItem, downloadAsset, and
mediaActions.confirmDelete and add the guard logic inside the arrow functions.
…mfyMenuButton (#8288) Backport of #8268 to `cloud/1.37` Automatically created by backport workflow. ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-8288-backport-cloud-1-37-bugfix-Fix-inconsistent-menu-icon-sizes-in-ComfyMenuButton-2f26d73d365081f4a184dd5c98ec736d) by [Unito](https://www.unito.io) Co-authored-by: Jin Yi <jin12cc@gmail.com>
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/platform/assets/utils/createModelNodeFromAsset.ts (1)
72-79: Prefer type-aware filename selection to avoid false INVALID_ASSET.
||will accept any truthy value (e.g., number/object), preventing fallback tovalidAsset.nameeven when that is valid. Also, the log message now misattributes errors touser_metadata.filenameeven when fallback sources are used.✅ Suggested fix (type-aware fallback + clearer log)
- const userMetadata = validAsset.user_metadata ?? {} - - const filename = - userMetadata.filename || validAsset.metadata?.filename || validAsset.name + const userMetadata = validAsset.user_metadata ?? {} + const filenameCandidates = [ + userMetadata.filename, + validAsset.metadata?.filename, + validAsset.name + ] + const filename = filenameCandidates.find( + (value): value is string => typeof value === 'string' && value.length > 0 + ) if (typeof filename !== 'string' || filename.length === 0) { console.error( - `Asset ${validAsset.id} has invalid user_metadata.filename (expected non-empty string, got ${typeof filename})` + `Asset ${validAsset.id} has no valid filename (expected non-empty string, got ${typeof filename})` )
Backport of #8041 to `cloud/1.37`. **Original PR:** #8041 ## Changes - Consolidated ManagerDialogContent, ManagerHeader, ManagerNavSidebar, RegistrySearchBar, and SearchFilterDropdown into single ManagerDialog component - Added v-model:rightPanelOpen to BaseModalLayout for external panel state control - Removed unused useResponsiveCollapse composable, TabItem and SearchOption types - Moved action buttons (Install All/Update All) from header-right-area to contentFilter area ## Conflict Resolution - **GlobalDialog.vue**: Kept settings-dialog-workspace styles, removed manager-dialog styles (now in BaseModalLayout) - **BaseModalLayout.vue**: Kept HEAD version (from #8256 backport) which has improved grid-based layout with accessibility features ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-8306-backport-cloud-1-37-refactor-Manager-dialog-simplification-2f36d73d365081078518cc62ea736708) by [Unito](https://www.unito.io) Co-authored-by: Jin Yi <jin12cc@gmail.com> Co-authored-by: GitHub Action <action@github.com> Co-authored-by: github-actions <github-actions@github.com>
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/workbench/extensions/manager/composables/useManagerState.ts (1)
182-194:isLegacyOnlystill opens NEW_UI dialog, contradicting the doc.The comment states legacy-only should show an error instead of opening the manager, but the code still opens the All tab. Either update the behavior or the doc.
🐛 Proposed fix (don’t open manager when legacy-only)
case ManagerUIState.NEW_UI: if (options?.isLegacyOnly) { // Legacy command is not available in NEW_UI mode useToastStore().add({ severity: 'error', summary: t('g.error'), detail: t('manager.legacyMenuNotAvailable'), life: 3000 }) - await managerDialog.show(ManagerTab.All) + return } else { await managerDialog.show(options?.initialTab) } break
🤖 Fix all issues with AI agents
In `@src/components/dialog/GlobalDialog.vue`:
- Around line 72-80: The new .settings-dialog-workspace CSS should be removed
and replaced with Tailwind classes and PrimeVue pt props: delete the CSS rules
from GlobalDialog.vue, add equivalent width/max-width Tailwind utilities on the
dialog root (use cn() to compose classes) and apply content sizing via
PrimeVue's pt or a wrapper element for .p-dialog-content; ensure the dialog
component (GlobalDialog.vue) uses cn() to set classes like full width and
max-w-[1440px] and pass pt to adjust .p-dialog-content instead of adding a
<style> block.
In `@src/workbench/extensions/manager/components/manager/ManagerDialog.vue`:
- Around line 72-79: The close Button in ManagerDialog.vue is icon-only and
needs an accessible name; update the Button component (the one with
`@click`="dismissWarningBanner") to include an aria-label using the i18n translate
function (e.g. $t or equivalent) so screen readers can announce it; add a
descriptive key like "manager.dialog.close" to your locale files and bind
aria-label to that i18n key on the Button.
- Around line 65-70: Replace the clickable <p> with a real button element (or
the project's button component) so the warning banner action is
keyboard-accessible: change the element that renders "{{
$t('manager.conflicts.warningBanner.button') }}" to a <button> (or <BaseButton>)
that keeps the same classes, binds `@click`="onClickWarningLink", and adds
type="button" and an aria-label if needed; also remove cursor-pointer from
non-interactive styling and ensure focus styles remain visible. Target the
template element that currently uses the class "m-0 cursor-pointer text-sm
font-bold" and the click handler onClickWarningLink to implement this change.
♻️ Duplicate comments (2)
src/locales/en/main.json (1)
1282-1285: Remove duplicateGeneralkey insettingsCategories.
Duplicate JSON keys are overwritten by the latter entry and can confuse tooling.🧹 Suggested fix
- "General": "General",src/components/dialog/GlobalDialog.vue (1)
7-12: Usecn()instead of array class binding.This repeats the previously flagged guideline violation for class merging. Please switch the array binding to
cn()as already recommended.Based on learnings, ...
| /* Workspace mode: wider settings dialog */ | ||
| .settings-dialog-workspace { | ||
| width: 100%; | ||
| max-width: 1440px; | ||
| } | ||
|
|
||
| @media (min-width: 3000px) { | ||
| .manager-dialog { | ||
| max-width: 2200px; | ||
| max-height: 1320px; | ||
| } | ||
| .settings-dialog-workspace .p-dialog-content { | ||
| width: 100%; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Avoid new <style> rules; use Tailwind utilities/pt instead.
The new .settings-dialog-workspace CSS expands the style block, which conflicts with the repo rule to avoid <style> in Vue SFCs. Prefer moving this sizing to Tailwind utilities on the dialog root and content (via cn() and PrimeVue pt or a wrapper).
As per coding guidelines, ...
🤖 Prompt for AI Agents
In `@src/components/dialog/GlobalDialog.vue` around lines 72 - 80, The new
.settings-dialog-workspace CSS should be removed and replaced with Tailwind
classes and PrimeVue pt props: delete the CSS rules from GlobalDialog.vue, add
equivalent width/max-width Tailwind utilities on the dialog root (use cn() to
compose classes) and apply content sizing via PrimeVue's pt or a wrapper element
for .p-dialog-content; ensure the dialog component (GlobalDialog.vue) uses cn()
to set classes like full width and max-w-[1440px] and pass pt to adjust
.p-dialog-content instead of adding a <style> block.
| <p | ||
| class="m-0 cursor-pointer text-sm font-bold" | ||
| @click="onClickWarningLink" | ||
| > | ||
| <i | ||
| class="icon-[lucide--triangle-alert] text-lg text-warning-background" | ||
| /> | ||
| <div class="flex flex-1 flex-col gap-2"> | ||
| <p class="m-0 text-sm font-bold"> | ||
| {{ $t('manager.conflicts.warningBanner.title') }} | ||
| </p> | ||
| <p class="m-0 text-xs"> | ||
| {{ $t('manager.conflicts.warningBanner.message') }} | ||
| </p> | ||
| <p | ||
| class="m-0 cursor-pointer text-sm font-bold" | ||
| @click="onClickWarningLink" | ||
| > | ||
| {{ $t('manager.conflicts.warningBanner.button') }} | ||
| </p> | ||
| </div> | ||
| <Button | ||
| class="absolute top-0 right-0" | ||
| variant="textonly" | ||
| size="icon" | ||
| @click="dismissWarningBanner" | ||
| > | ||
| <i class="pi pi-times text-xs text-base-foreground"></i> | ||
| </Button> | ||
| </div> | ||
| <RegistrySearchBar | ||
| v-model:search-query="searchQuery" | ||
| v-model:search-mode="searchMode" | ||
| v-model:sort-field="sortField" | ||
| :search-results="searchResults" | ||
| :suggestions="suggestions" | ||
| :is-missing-tab="isMissingTab" | ||
| :sort-options="sortOptions" | ||
| :is-update-available-tab="isUpdateAvailableTab" | ||
| /> | ||
| <div class="flex-1 overflow-auto"> | ||
| <div | ||
| v-if="isLoading" | ||
| class="h-full scrollbar-hide w-full overflow-auto" | ||
| > | ||
| <GridSkeleton :grid-style="GRID_STYLE" :skeleton-card-count /> | ||
| </div> | ||
| <NoResultsPlaceholder | ||
| v-else-if="searchResults.length === 0" | ||
| :title=" | ||
| comfyManagerStore.error | ||
| ? $t('manager.errorConnecting') | ||
| : $t('manager.noResultsFound') | ||
| " | ||
| :message=" | ||
| comfyManagerStore.error | ||
| ? $t('manager.tryAgainLater') | ||
| : $t('manager.tryDifferentSearch') | ||
| " | ||
| /> | ||
| <div v-else class="h-full" @click="handleGridContainerClick"> | ||
| <VirtualGrid | ||
| id="results-grid" | ||
| :items="resultsWithKeys" | ||
| :buffer-rows="4" | ||
| :grid-style="GRID_STYLE" | ||
| @approach-end="onApproachEnd" | ||
| > | ||
| <template #item="{ item }"> | ||
| <PackCard | ||
| :node-pack="item" | ||
| :is-selected=" | ||
| selectedNodePacks.some((pack) => pack.id === item.id) | ||
| " | ||
| @click.stop=" | ||
| (event: MouseEvent) => selectNodePack(item, event) | ||
| " | ||
| /> | ||
| </template> | ||
| </VirtualGrid> | ||
| </div> | ||
| </div> | ||
| {{ $t('manager.conflicts.warningBanner.button') }} | ||
| </p> |
There was a problem hiding this comment.
Use a real button for the warning-banner action.
Clickable <p> isn’t keyboard accessible. Switch to a button component to keep semantics and focus behavior correct. Based on learnings, interactive elements should be real buttons for accessibility.
♿ Proposed fix
- <p
- class="m-0 cursor-pointer text-sm font-bold"
- `@click`="onClickWarningLink"
- >
- {{ $t('manager.conflicts.warningBanner.button') }}
- </p>
+ <Button
+ variant="textonly"
+ class="m-0 p-0 h-auto text-sm font-bold"
+ `@click`="onClickWarningLink"
+ >
+ {{ $t('manager.conflicts.warningBanner.button') }}
+ </Button>📝 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.
| <p | |
| class="m-0 cursor-pointer text-sm font-bold" | |
| @click="onClickWarningLink" | |
| > | |
| <i | |
| class="icon-[lucide--triangle-alert] text-lg text-warning-background" | |
| /> | |
| <div class="flex flex-1 flex-col gap-2"> | |
| <p class="m-0 text-sm font-bold"> | |
| {{ $t('manager.conflicts.warningBanner.title') }} | |
| </p> | |
| <p class="m-0 text-xs"> | |
| {{ $t('manager.conflicts.warningBanner.message') }} | |
| </p> | |
| <p | |
| class="m-0 cursor-pointer text-sm font-bold" | |
| @click="onClickWarningLink" | |
| > | |
| {{ $t('manager.conflicts.warningBanner.button') }} | |
| </p> | |
| </div> | |
| <Button | |
| class="absolute top-0 right-0" | |
| variant="textonly" | |
| size="icon" | |
| @click="dismissWarningBanner" | |
| > | |
| <i class="pi pi-times text-xs text-base-foreground"></i> | |
| </Button> | |
| </div> | |
| <RegistrySearchBar | |
| v-model:search-query="searchQuery" | |
| v-model:search-mode="searchMode" | |
| v-model:sort-field="sortField" | |
| :search-results="searchResults" | |
| :suggestions="suggestions" | |
| :is-missing-tab="isMissingTab" | |
| :sort-options="sortOptions" | |
| :is-update-available-tab="isUpdateAvailableTab" | |
| /> | |
| <div class="flex-1 overflow-auto"> | |
| <div | |
| v-if="isLoading" | |
| class="h-full scrollbar-hide w-full overflow-auto" | |
| > | |
| <GridSkeleton :grid-style="GRID_STYLE" :skeleton-card-count /> | |
| </div> | |
| <NoResultsPlaceholder | |
| v-else-if="searchResults.length === 0" | |
| :title=" | |
| comfyManagerStore.error | |
| ? $t('manager.errorConnecting') | |
| : $t('manager.noResultsFound') | |
| " | |
| :message=" | |
| comfyManagerStore.error | |
| ? $t('manager.tryAgainLater') | |
| : $t('manager.tryDifferentSearch') | |
| " | |
| /> | |
| <div v-else class="h-full" @click="handleGridContainerClick"> | |
| <VirtualGrid | |
| id="results-grid" | |
| :items="resultsWithKeys" | |
| :buffer-rows="4" | |
| :grid-style="GRID_STYLE" | |
| @approach-end="onApproachEnd" | |
| > | |
| <template #item="{ item }"> | |
| <PackCard | |
| :node-pack="item" | |
| :is-selected=" | |
| selectedNodePacks.some((pack) => pack.id === item.id) | |
| " | |
| @click.stop=" | |
| (event: MouseEvent) => selectNodePack(item, event) | |
| " | |
| /> | |
| </template> | |
| </VirtualGrid> | |
| </div> | |
| </div> | |
| {{ $t('manager.conflicts.warningBanner.button') }} | |
| </p> | |
| <Button | |
| class="m-0 p-0 h-auto text-sm font-bold" | |
| `@click`="onClickWarningLink" | |
| > | |
| {{ $t('manager.conflicts.warningBanner.button') }} | |
| </Button> |
🤖 Prompt for AI Agents
In `@src/workbench/extensions/manager/components/manager/ManagerDialog.vue` around
lines 65 - 70, Replace the clickable <p> with a real button element (or the
project's button component) so the warning banner action is keyboard-accessible:
change the element that renders "{{ $t('manager.conflicts.warningBanner.button')
}}" to a <button> (or <BaseButton>) that keeps the same classes, binds
`@click`="onClickWarningLink", and adds type="button" and an aria-label if needed;
also remove cursor-pointer from non-interactive styling and ensure focus styles
remain visible. Target the template element that currently uses the class "m-0
cursor-pointer text-sm font-bold" and the click handler onClickWarningLink to
implement this change.
| <Button | ||
| class="absolute top-0 right-0" | ||
| variant="textonly" | ||
| size="icon" | ||
| @click="dismissWarningBanner" | ||
| > | ||
| <i class="pi pi-times text-xs text-base-foreground"></i> | ||
| </Button> |
There was a problem hiding this comment.
Add an accessible label to the icon-only close button.
Icon-only buttons need an accessible name. Use i18n for the label. Based on learnings, icon-only buttons require aria-labels.
♿ Proposed fix
- <Button
+ <Button
+ :aria-label="$t('g.close')"
class="absolute top-0 right-0"
variant="textonly"
size="icon"
`@click`="dismissWarningBanner"
>🤖 Prompt for AI Agents
In `@src/workbench/extensions/manager/components/manager/ManagerDialog.vue` around
lines 72 - 79, The close Button in ManagerDialog.vue is icon-only and needs an
accessible name; update the Button component (the one with
`@click`="dismissWarningBanner") to include an aria-label using the i18n translate
function (e.g. $t or equivalent) so screen readers can announce it; add a
descriptive key like "manager.dialog.close" to your locale files and bind
aria-label to that i18n key on the Button.
Backport of #8256 to cloud/1.37 ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-8308-backport-cloud-1-37-Updates-More-Modal-Modification-2f36d73d365081939779c594614e3a1b) by [Unito](https://www.unito.io) Co-authored-by: Amp <amp@ampcode.com>
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (8)
src/platform/assets/components/AssetBadgeGroup.vue (1)
8-12: Optional: Simplify static class binding.
cn()is useful for merging and resolving conflicting Tailwind classes, but here it wraps a single static string with no dynamic or conditional classes. You can apply the classes directly to streamline the code.♻️ Suggested simplification
<span v-for="badge in badges" :key="badge.label" - :class=" - cn( - 'px-2 py-1 rounded text-xs font-bold uppercase tracking-wider text-modal-card-tag-foreground bg-modal-card-tag-background break-all' - ) - " + class="px-2 py-1 rounded text-xs font-bold uppercase tracking-wider text-modal-card-tag-foreground bg-modal-card-tag-background break-all" >This also allows removing the
cnimport if unused elsewhere.browser_tests/tests/templates.spec.ts (1)
106-106: Prefer Playwright's auto-retrying assertion pattern.Using
expect(await ...isVisible()).toBe(true)bypasses Playwright's built-in retry mechanism. The auto-retrying pattern is more resilient to timing issues.♻️ Suggested fix
- expect(await comfyPage.templates.content.isVisible()).toBe(true) + await expect(comfyPage.templates.content).toBeVisible()src/components/widget/layout/BaseModalLayout.stories.ts (4)
67-67: Prefer function declaration over function expression.Per project guidelines, pure function declarations are preferred over function expressions for better hoisting clarity and readability.
♻️ Suggested refactor
-const createStoryTemplate = (args: StoryArgs) => ({ +function createStoryTemplate(args: StoryArgs) { + return { components: { ... }, ... - }) + } +}Based on learnings, function declarations are preferred over function expressions.
82-82: Remove unused variablet.The translation function
tis defined but never referenced in the template. Remove it to avoid dead code.♻️ Suggested fix
setup() { - const t = (k: string) => k - const onClose = () => {
205-227: Inconsistent Button API usage between branches.The two template branches use different patterns for the
MoreButtonchildren:
- Lines 207-225: Uses
labelprop with#iconslot- Lines 330-344: Uses inline
<span>and<i>childrenThis inconsistency makes it unclear which API pattern is preferred for the
Buttoncomponent and reduces the value of these stories as documentation.Align both branches to use the same Button API pattern for consistency.
Also applies to: 328-346
421-445:DefaultandBothPanelsstories have identical args.Both stories use exactly the same configuration, making
BothPanelsredundant. Consider differentiating them or removing one.♻️ Suggested fix: Remove duplicate or differentiate
Either remove
BothPanelssince it's identical toDefault, or if intentional for documentation purposes, add a comment explaining the distinction.export const BothPanels: Story = { + // Explicit demonstration of both panels enabled (same as Default) render: (args: StoryArgs) => createStoryTemplate(args), args: {src/workbench/extensions/manager/components/manager/ManagerDialog.vue (2)
575-584: Consider immutable array updates instead of splice.The
splicecalls mutateselectedNodePacksanddisplayPacksin place. Per coding guidelines, prefer immutability. However, this pattern is common for preserving array reference reactivity.♻️ Immutable alternative
if (packIndex !== -1) { - selectedNodePacks.value.splice(packIndex, 1, mergedPack) + selectedNodePacks.value = [ + ...selectedNodePacks.value.slice(0, packIndex), + mergedPack, + ...selectedNodePacks.value.slice(packIndex + 1) + ] } const idx = displayPacks.value.findIndex((p) => p.id === mergedPack.id) if (idx !== -1) { - displayPacks.value.splice(idx, 1, mergedPack) + displayPacks.value = [ + ...displayPacks.value.slice(0, idx), + mergedPack, + ...displayPacks.value.slice(idx + 1) + ] }
588-598: Consider using a template ref instead of getElementById.Direct DOM access via
getElementByIdworks but using a Vue template ref would be more idiomatic and type-safe.♻️ Template ref approach
In template, add ref to the VirtualGrid wrapper:
<div v-else ref="gridContainerRef" class="h-full" `@click`="handleGridContainerClick">In script:
-let gridContainer: HTMLElement | null = null -onMounted(() => { - gridContainer = document.getElementById('results-grid') -}) +const gridContainerRef = ref<HTMLElement | null>(null) + watch([searchQuery, selectedNavId], () => { - gridContainer ??= document.getElementById('results-grid') - if (gridContainer) { + if (gridContainerRef.value) { pageNumber.value = 0 - gridContainer.scrollTop = 0 + gridContainerRef.value.scrollTop = 0 } })
🤖 Fix all issues with AI agents
In `@src/platform/assets/components/modelInfo/ModelInfoPanel.vue`:
- Around line 172-197: There are two ModelInfoField blocks showing source
description and editable user description simultaneously which can confuse
users; update the UI so that when the prop/variable description (the read-only
source) exists you either hide the editable field or clearly separate them —
e.g., conditionally render the editable block only when description is empty or
add a distinct label/header for the editable field; adjust the template logic
around the ModelInfoField instances (the one using description and the one
binding v-model="userDescription" and ref="descriptionTextarea") and ensure
isImmutable behavior and placeholders remain correct.
In `@src/stores/assetsStore.ts`:
- Around line 481-502: The updateAssetMetadata function swallows errors (logs
and rolls back) so callers can’t detect failure; change its signature to return
a Promise<boolean> (or rethrow) and on success return true, and in the catch
block after calling updateAssetInCache to rollback either return false or
rethrow the caught error; update callers of updateAssetMetadata (e.g., places
invoking updateAssetMetadata) to handle the boolean result or catch the rethrown
error and show UI feedback (toast/retry) accordingly.
In `@src/workbench/extensions/manager/components/manager/ManagerDialog.vue`:
- Around line 22-42: The AutoCompletePlus in ManagerDialog.vue currently uses an
extremely low debounce (:delay="8") which can trigger excessive API calls;
update the AutoCompletePlus prop :delay to a more typical debounce (e.g.,
150–300 ms, recommend 200) to reduce churn on searchQuery and API traffic, and
ensure the change is applied where AutoCompletePlus is instantiated (look for
the component tag with v-model.lazy="searchQuery" and the :delay prop).
- Around line 500-507: Replace the ref+watch pair for isRightPanelOpen with a
computed property derived from selectedNodePacks: if the panel is strictly
derived (cannot be closed independently), use computed(() =>
selectedNodePacks.value.length > 0); if the panel can be closed by the user via
v-model:right-panel-open, implement a writable computed with get returning
selectedNodePacks.value.length > 0 and set(false) clearing selectedNodePacks (or
otherwise deselecting nodes) so v-model:right-panel-open works correctly; update
usages of isRightPanelOpen accordingly.
♻️ Duplicate comments (6)
src/components/widget/layout/BaseModalLayout.vue (1)
183-187: Consider extracting magic numbers for grid column widths.The grid column widths (
14rem,18rem) are hardcoded in the computed property. Consider extracting these as named constants for better maintainability.src/platform/assets/components/AssetBrowserModal.vue (1)
59-71: Use.selfmodifier to prevent clearing focus on bubbled child clicks.On line 69, the
@clickhandler will fire for any click event bubbling from child elements (like the delete button in AssetCard). This will clear the focused asset unintentionally. Use.selfto respond only to direct clicks on the AssetGrid container, consistent with the header (line 27) and filter bar (line 55).🔧 Proposed fix
<AssetGrid :assets="filteredAssets" :loading="isLoading" :focused-asset-id="focusedAsset?.id" :empty-message `@asset-focus`="handleAssetFocus" `@asset-select`="handleAssetSelectAndEmit" `@asset-deleted`="refreshAssets" `@asset-show-info`="handleShowInfo" - `@click`="focusedAsset = null" + `@click.self`="focusedAsset = null" />src/workbench/extensions/manager/components/manager/ManagerDialog.vue (2)
62-67: Use a real button for the warning-banner action.The clickable
<p>element is not keyboard accessible. This should be replaced with a proper button component to ensure correct semantics and focus behavior.♿ Proposed fix
- <p - class="m-0 cursor-pointer text-sm font-bold" - `@click`="onClickWarningLink" - > - {{ $t('manager.conflicts.warningBanner.button') }} - </p> + <Button + variant="textonly" + class="m-0 p-0 h-auto text-sm font-bold" + `@click`="onClickWarningLink" + > + {{ $t('manager.conflicts.warningBanner.button') }} + </Button>
69-76: Add an accessible label to the icon-only close button.Icon-only buttons require an accessible name for screen readers. Add an
aria-labelusing i18n.♿ Proposed fix
<Button + :aria-label="$t('g.close')" class="absolute top-0 right-0" variant="textonly" size="icon" `@click`="dismissWarningBanner" >src/stores/assetsStore.ts (1)
510-542: The partial failure concern (ifremoveAssetTagssucceeds butaddAssetTagsfails) has already been identified in a previous review.src/components/widget/nav/NavItem.vue (1)
3-16: Compute overflow on mount/resize so the tooltip works on first hover.
isOverflowingonly updates onmouseenter, so the first hover won’t show a tooltip and resizes/content changes won’t refresh the overflow state.🛠️ Proposed fix
-import { computed, ref } from 'vue' +import { computed, onMounted, ref } from 'vue' +import { useResizeObserver } from '@vueuse/core' @@ const checkOverflow = () => { if (!textRef.value) return isOverflowing.value = textRef.value.scrollWidth > textRef.value.clientWidth + 1 } + +onMounted(checkOverflow) +useResizeObserver(textRef, checkOverflow)#!/bin/bash # Verify `@vueuse/core` is available for useResizeObserver. rg -n '"@vueuse/core"' -g 'package.json'As per coding guidelines, prefer VueUse utilities for observer-based updates.
Also applies to: 51-57
| <ModelInfoField | ||
| v-if="description" | ||
| :label="t('assetBrowser.modelInfo.description')" | ||
| > | ||
| <p class="text-sm whitespace-pre-wrap">{{ description }}</p> | ||
| </ModelInfoField> | ||
| <ModelInfoField :label="t('assetBrowser.modelInfo.description')"> | ||
| <textarea | ||
| ref="descriptionTextarea" | ||
| v-model="userDescription" | ||
| :disabled="isImmutable" | ||
| :placeholder=" | ||
| isImmutable | ||
| ? t('assetBrowser.modelInfo.descriptionNotSet') | ||
| : t('assetBrowser.modelInfo.descriptionPlaceholder') | ||
| " | ||
| rows="3" | ||
| :class=" | ||
| cn( | ||
| 'w-full resize-y rounded-lg border border-transparent bg-transparent px-3 py-2 text-sm text-component-node-foreground outline-none transition-colors focus:bg-component-node-widget-background', | ||
| isImmutable && 'cursor-not-allowed' | ||
| ) | ||
| " | ||
| @keydown.escape.stop="descriptionTextarea?.blur()" | ||
| /> | ||
| </ModelInfoField> |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Clarify the two description sections.
There are two ModelInfoField components with description-related labels:
- Lines 172-177: Read-only source description (conditionally shown)
- Lines 178-197: Editable user description (always shown)
This dual-description pattern makes sense for separating source metadata from user notes, but consider whether both should be visible simultaneously when the source description exists, as users might find this confusing.
🤖 Prompt for AI Agents
In `@src/platform/assets/components/modelInfo/ModelInfoPanel.vue` around lines 172
- 197, There are two ModelInfoField blocks showing source description and
editable user description simultaneously which can confuse users; update the UI
so that when the prop/variable description (the read-only source) exists you
either hide the editable field or clearly separate them — e.g., conditionally
render the editable block only when description is empty or add a distinct
label/header for the editable field; adjust the template logic around the
ModelInfoField instances (the one using description and the one binding
v-model="userDescription" and ref="descriptionTextarea") and ensure isImmutable
behavior and placeholders remain correct.
| async function updateAssetMetadata( | ||
| asset: AssetItem, | ||
| userMetadata: Record<string, unknown>, | ||
| cacheKey?: string | ||
| ) { | ||
| const originalMetadata = asset.user_metadata | ||
| updateAssetInCache(asset.id, { user_metadata: userMetadata }, cacheKey) | ||
|
|
||
| try { | ||
| const updatedAsset = await assetService.updateAsset(asset.id, { | ||
| user_metadata: userMetadata | ||
| }) | ||
| updateAssetInCache(asset.id, updatedAsset, cacheKey) | ||
| } catch (error) { | ||
| console.error('Failed to update asset metadata:', error) | ||
| updateAssetInCache( | ||
| asset.id, | ||
| { user_metadata: originalMetadata }, | ||
| cacheKey | ||
| ) | ||
| } | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider: Callers cannot detect operation failure.
The error is caught, logged, and rolled back, but not propagated to callers. If the caller needs to show a toast notification or retry logic, they have no way to know the server operation failed.
If this is intentional (the visual rollback is sufficient user feedback), consider documenting this behavior. Otherwise, consider returning a success boolean or rethrowing after rollback:
💡 Optional: Return success indicator
async function updateAssetMetadata(
asset: AssetItem,
userMetadata: Record<string, unknown>,
cacheKey?: string
- ) {
+ ): Promise<boolean> {
const originalMetadata = asset.user_metadata
updateAssetInCache(asset.id, { user_metadata: userMetadata }, cacheKey)
try {
const updatedAsset = await assetService.updateAsset(asset.id, {
user_metadata: userMetadata
})
updateAssetInCache(asset.id, updatedAsset, cacheKey)
+ return true
} catch (error) {
console.error('Failed to update asset metadata:', error)
updateAssetInCache(
asset.id,
{ user_metadata: originalMetadata },
cacheKey
)
+ return false
}
}📝 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.
| async function updateAssetMetadata( | |
| asset: AssetItem, | |
| userMetadata: Record<string, unknown>, | |
| cacheKey?: string | |
| ) { | |
| const originalMetadata = asset.user_metadata | |
| updateAssetInCache(asset.id, { user_metadata: userMetadata }, cacheKey) | |
| try { | |
| const updatedAsset = await assetService.updateAsset(asset.id, { | |
| user_metadata: userMetadata | |
| }) | |
| updateAssetInCache(asset.id, updatedAsset, cacheKey) | |
| } catch (error) { | |
| console.error('Failed to update asset metadata:', error) | |
| updateAssetInCache( | |
| asset.id, | |
| { user_metadata: originalMetadata }, | |
| cacheKey | |
| ) | |
| } | |
| } | |
| async function updateAssetMetadata( | |
| asset: AssetItem, | |
| userMetadata: Record<string, unknown>, | |
| cacheKey?: string | |
| ): Promise<boolean> { | |
| const originalMetadata = asset.user_metadata | |
| updateAssetInCache(asset.id, { user_metadata: userMetadata }, cacheKey) | |
| try { | |
| const updatedAsset = await assetService.updateAsset(asset.id, { | |
| user_metadata: userMetadata | |
| }) | |
| updateAssetInCache(asset.id, updatedAsset, cacheKey) | |
| return true | |
| } catch (error) { | |
| console.error('Failed to update asset metadata:', error) | |
| updateAssetInCache( | |
| asset.id, | |
| { user_metadata: originalMetadata }, | |
| cacheKey | |
| ) | |
| return false | |
| } | |
| } |
🤖 Prompt for AI Agents
In `@src/stores/assetsStore.ts` around lines 481 - 502, The updateAssetMetadata
function swallows errors (logs and rolls back) so callers can’t detect failure;
change its signature to return a Promise<boolean> (or rethrow) and on success
return true, and in the catch block after calling updateAssetInCache to rollback
either return false or rethrow the caught error; update callers of
updateAssetMetadata (e.g., places invoking updateAssetMetadata) to handle the
boolean result or catch the rethrown error and show UI feedback (toast/retry)
accordingly.
| <AutoCompletePlus | ||
| v-model.lazy="searchQuery" | ||
| :suggestions="suggestions" | ||
| :placeholder="$t('manager.searchPlaceholder')" | ||
| :complete-on-focus="false" | ||
| :delay="8" | ||
| option-label="query" | ||
| class="w-full min-w-md max-w-lg" | ||
| :pt="{ | ||
| pcInputText: { | ||
| root: { | ||
| autofocus: true, | ||
| class: 'w-full rounded-lg h-10' | ||
| } | ||
| }, | ||
| loader: { style: 'display: none' } | ||
| }" | ||
| :show-empty-message="false" | ||
| @complete="stubTrue" | ||
| @option-select="onOptionSelect" | ||
| /> |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Very short autocomplete delay may cause excessive API calls.
The delay of 8ms is extremely aggressive for debouncing search requests. Consider increasing this to a more typical value (e.g., 150-300ms) to reduce unnecessary API calls during typing.
♻️ Proposed fix
<AutoCompletePlus
v-model.lazy="searchQuery"
:suggestions="suggestions"
:placeholder="$t('manager.searchPlaceholder')"
:complete-on-focus="false"
- :delay="8"
+ :delay="200"
option-label="query"📝 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.
| <AutoCompletePlus | |
| v-model.lazy="searchQuery" | |
| :suggestions="suggestions" | |
| :placeholder="$t('manager.searchPlaceholder')" | |
| :complete-on-focus="false" | |
| :delay="8" | |
| option-label="query" | |
| class="w-full min-w-md max-w-lg" | |
| :pt="{ | |
| pcInputText: { | |
| root: { | |
| autofocus: true, | |
| class: 'w-full rounded-lg h-10' | |
| } | |
| }, | |
| loader: { style: 'display: none' } | |
| }" | |
| :show-empty-message="false" | |
| @complete="stubTrue" | |
| @option-select="onOptionSelect" | |
| /> | |
| <AutoCompletePlus | |
| v-model.lazy="searchQuery" | |
| :suggestions="suggestions" | |
| :placeholder="$t('manager.searchPlaceholder')" | |
| :complete-on-focus="false" | |
| :delay="200" | |
| option-label="query" | |
| class="w-full min-w-md max-w-lg" | |
| :pt="{ | |
| pcInputText: { | |
| root: { | |
| autofocus: true, | |
| class: 'w-full rounded-lg h-10' | |
| } | |
| }, | |
| loader: { style: 'display: none' } | |
| }" | |
| :show-empty-message="false" | |
| `@complete`="stubTrue" | |
| `@option-select`="onOptionSelect" | |
| /> |
🤖 Prompt for AI Agents
In `@src/workbench/extensions/manager/components/manager/ManagerDialog.vue` around
lines 22 - 42, The AutoCompletePlus in ManagerDialog.vue currently uses an
extremely low debounce (:delay="8") which can trigger excessive API calls;
update the AutoCompletePlus prop :delay to a more typical debounce (e.g.,
150–300 ms, recommend 200) to reduce churn on searchQuery and API traffic, and
ensure the change is applied where AutoCompletePlus is instantiated (look for
the component tag with v-model.lazy="searchQuery" and the :delay prop).
| const isRightPanelOpen = ref(false) | ||
|
|
||
| watch( | ||
| () => selectedNodePacks.value.length, | ||
| (length) => { | ||
| isRightPanelOpen.value = length > 0 | ||
| } | ||
| ) |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Prefer computed over ref + watch for derived state.
isRightPanelOpen is derived from selectedNodePacks.value.length > 0. Using a watch to sync a separate ref is more complex than necessary. However, since this is used with v-model:right-panel-open, a writable computed would be needed if the panel can be closed independently.
If the right panel cannot be closed independently (only through deselection), consider:
♻️ Proposed simplification
-const isRightPanelOpen = ref(false)
-
-watch(
- () => selectedNodePacks.value.length,
- (length) => {
- isRightPanelOpen.value = length > 0
- }
-)
+const isRightPanelOpen = computed({
+ get: () => selectedNodePacks.value.length > 0,
+ set: (open) => {
+ if (!open) selectedNodePacks.value = []
+ }
+})🤖 Prompt for AI Agents
In `@src/workbench/extensions/manager/components/manager/ManagerDialog.vue` around
lines 500 - 507, Replace the ref+watch pair for isRightPanelOpen with a computed
property derived from selectedNodePacks: if the panel is strictly derived
(cannot be closed independently), use computed(() =>
selectedNodePacks.value.length > 0); if the panel can be closed by the user via
v-model:right-panel-open, implement a writable computed with get returning
selectedNodePacks.value.length > 0 and set(false) clearing selectedNodePacks (or
otherwise deselecting nodes) so v-model:right-panel-open works correctly; update
usages of isRightPanelOpen accordingly.
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/platform/assets/utils/createModelNodeFromAsset.ts (1)
73-86: Update error message to reflect the new fallback logic.The error message on line 76 references
user_metadata.filenamebut the filename now comes from a fallback chain (user_metadata.filename→metadata.filename→asset.name). Additionally, sincegetAssetFilenamealways returns a string,typeof filenamewill always be"string", making the error message misleading.🔧 Proposed fix
const filename = getAssetFilename(validAsset) if (filename.length === 0) { console.error( - `Asset ${validAsset.id} has invalid user_metadata.filename (expected non-empty string, got ${typeof filename})` + `Asset ${validAsset.id} has no valid filename (all sources empty: user_metadata.filename, metadata.filename, asset.name)` ) return { success: false, error: { code: 'INVALID_ASSET', - message: `Invalid filename (expected non-empty string, got ${typeof filename})`, + message: 'Invalid filename (all filename sources are empty)', assetId: validAsset.id } } }
♻️ Duplicate comments (1)
src/renderer/extensions/vueNodes/widgets/composables/useComboWidget.ts (1)
98-142: Add error handling for dialog failures.The
throwon line 100 is reasonable for catching programming errors during development, butassetBrowserDialog.show()failures could bubble up as unhandled rejections. Consider wrapping the dialog call in a try/catch. As per coding guidelines, implement proper error handling.♻️ Suggested improvement
async function openModal(widget: IBaseWidget) { if (!isAssetWidget(widget)) { throw new Error(`Expected asset widget but received ${widget.type}`) } - await assetBrowserDialog.show({ + try { + await assetBrowserDialog.show({ nodeType: node.comfyClass || '', inputName: inputSpec.name, currentValue: widget.value, onAssetSelected: (asset) => { // ... existing callback logic } - }) + }) + } catch (error) { + console.error('Failed to open asset browser:', error) + } }
Backport of #8178 to `cloud/1.37` Automatically created by backport workflow. ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-8289-backport-cloud-1-37-Add-3d-control-buttons-to-linear-mode-2f26d73d3650817290ebf575f5f7a7a4) by [Unito](https://www.unito.io) Co-authored-by: AustinMroz <austin@comfy.org> Co-authored-by: Alexander Brown <drjkl@comfy.org>
## Summary Backport of #8245 to cloud/1.37. Add team workspace member management and invite system. - Add members panel with role management (owner/admin/member) and member removal - Add invite system with email invites, pending invite display, and revoke functionality - Add invite URL loading for accepting invites - Add subscription panel updates for member management - Add i18n translations for member and invite features ## Conflict Resolution - `src/components/dialog/GlobalDialog.vue`: Added missing `DialogPassThroughOptions` import - `src/locales/en/main.json`: Kept "nightly" section from main (was present before PR) - `src/platform/cloud/subscription/utils/subscriptionCheckoutUtil.ts`: Deleted (file doesn't exist in cloud/1.37, only contains unrelated method rename) (cherry picked from commit 4771565) ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-8301-backport-cloud-1-37-Workspaces-4-members-invites-2f36d73d36508119a388dac9d290efbd) by [Unito](https://www.unito.io)
dosubot
bot
added
size:S
There was a problem hiding this comment.
Actionable comments posted: 15
🤖 Fix all issues with AI agents
In `@src/components/dialog/content/setting/MembersPanelContent.vue`:
- Around line 324-329: Replace the raw HTML <button> in MembersPanelContent.vue
with the shared Button (or TextButton) component so the “create new workspace”
action uses the app's standard focus/hover styles; keep the
`@click`="handleCreateWorkspace" handler and the translated label {{
$t('workspacePanel.members.createNewWorkspace') }}, pass any existing
classes/variant props needed to match the original underline/bg-transparent
look, and add the import/registration for the shared Button/TextButton component
used across the codebase.
In `@src/components/dialog/content/setting/WorkspacePanelContent.vue`:
- Around line 49-61: The Invite Button (Button component rendering when
permissions.canInviteMembers) currently sets an explicit aria-label which
overrides its visible text; remove the
:aria-label="$t('workspacePanel.inviteMember')" prop from that Button so the
visible label becomes the accessible name, leaving other attributes (v-tooltip,
:disabled, `@click`="handleInviteMember") intact.
- Around line 199-201: The computed property menuItems currently initializes
items with `const items = []`, which infers `any[]`; change this to an
explicitly typed array (e.g., `const items: MenuItem[] = []` or define a local
interface and use that) and ensure the computed's return type matches (typing
the computed as `computed<MenuItem[]>(...)` if needed); update or import the
appropriate MenuItem type and apply the explicit type to both the `items`
variable and the `menuItems` computed to enforce type-safety in
WorkspacePanelContent.vue.
In `@src/components/dialog/content/workspace/EditWorkspaceDialogContent.vue`:
- Around line 31-46: The submit Button in EditWorkspaceDialogContent.vue uses an
invalid binding :loading and allows double-submits; change the attribute to
:loading="loading", make the Button disabled when loading (e.g.
:disabled="!isValidName || loading"), and update the Enter key handler to check
loading (replace `@keydown.enter`="isValidName && onSave()" with a guard that also
requires !loading). Inside the onSave method, add an early return if loading is
true, set loading = true before the async request and set loading = false in
finally (or on error) so the UI and handler properly prevent double-submit.
In `@src/components/dialog/content/workspace/InviteMemberDialogContent.vue`:
- Around line 16-22: Replace the plain <button> close control in
InviteMemberDialogContent.vue with the shared IconButton component: remove the
raw <button> element and use IconButton (preserving the aria-label from
$t('g.close') and the `@click`="onCancel" handler), map any needed styling/size to
IconButton props (e.g., size/variant/aria attributes) and render the existing <i
class="pi pi-times size-4" /> as the icon content or via the IconButton icon
prop so behavior and accessibility remain identical while using the shared
component.
- Around line 72-98: Replace the non-focusable clickable <div> used for the copy
action in InviteMemberDialogContent.vue with a proper focusable button
(preferably the shared IconButton component) so keyboard users can activate it;
keep the `@click`="onCopyLink" handler, set type="button" to avoid form
submission, and add an accessible aria-label like aria-label="Copy invite link"
(or use the component's label prop) to describe the action. Ensure the SVG stays
as the button's child and preserve any positioning classes (e.g., "absolute
right-4 top-2 cursor-pointer") while removing cursor-pointer if the shared
button already styles the cursor.
- Around line 149-153: The catch block in InviteMemberDialogContent.vue
currently uses the linkCopyFailed i18n key when create-link fails; update the
toast.add call in the catch handler to use a dedicated i18n key (e.g.,
t('workspacePanel.inviteMemberDialog.linkCreateFailed')) for the summary (and
provide the same error.detail logic), and add the new key to localization
resources so user-facing messages accurately reflect a create-link failure
instead of a copy failure.
In `@src/components/dialog/content/workspace/RemoveMemberDialogContent.vue`:
- Around line 12-18: The close icon uses a raw <button> in
RemoveMemberDialogContent.vue; replace it with the project's shared button
component (from src/components/button/) to keep styling/theming consistent and
preserve behavior: render the shared button component with the same
props/classes, bind the aria-label to $t('g.close'), and wire its click to the
existing onCancel handler (ensure the icon markup/class "pi pi-times size-4" is
placed inside the shared button). Update any import to bring in the shared
button component and remove the native button element.
In `@src/components/dialog/content/workspace/RevokeInviteDialogContent.vue`:
- Around line 12-18: Replace the raw <button> in RevokeInviteDialogContent.vue
with the shared IconButton component from src/components/button/, wiring the
existing onCancel handler to the IconButton's click event and preserving the
aria-label="$t('g.close')"; ensure the visual close icon (currently <i class="pi
pi-times size-4" />) is passed to IconButton using whichever prop/slot that
component expects (or use its "icon" prop), and carry over the current
styling/size via IconButton props instead of inline classes so the close control
uses the common button component and behavior.
- Around line 68-73: The catch block passes undefined to toast.add when error
isn't an Error or has an empty message; change it to compute a detail string
(e.g., const detail = error instanceof Error && error.message ? error.message :
t('g.unknownError')) and pass that to toast.add so the toast always shows a
localized fallback. Update the catch in RevokeInviteDialogContent.vue to use
this detail variable and keep the existing severity/summary/life fields.
In `@src/components/graph/GraphCanvas.vue`:
- Around line 468-482: The watcher uses immediate:true so the callback can run
before the const assignment completes, causing a TDZ when calling
stopWatching(); change the declaration to let stopWatching: (() => void) |
undefined (or appropriate type) and replace the direct call with optional
chaining stopWatching?.() inside the async watcher callback; locate the
watch(...) invocation that references flags.teamWorkspacesEnabled, stopWatching,
inviteUrlLoader, and inviteUrlLoader.loadInviteFromUrl() to apply this change.
In `@src/components/topbar/WorkspaceSwitcherPopover.vue`:
- Around line 164-168: The getRoleLabel function currently returns empty string
for the 'admin' role; update getRoleLabel(AvailableWorkspace['role']) to handle
role === 'admin' and return the appropriate i18n key (e.g.
t('workspaceSwitcher.roleAdmin')), and add the corresponding translation entry
for workspaceSwitcher.roleAdmin in the locale files so admins render the proper
label.
In `@src/platform/settings/components/SettingDialogContent.vue`:
- Around line 17-21: Replace the concatenation of $t('g.searchSettings') + '...'
with a single i18n key for the full placeholder string used in the SearchBox
inside SettingDialogContent.vue (e.g. $t('g.searchSettingsPlaceholder')); update
the SearchBox placeholder prop to use that key and add the corresponding
translation entry "Search Settings..." to src/locales/en/main.json (key name
must match the one used in the component).
In `@src/platform/workspace/composables/useInviteUrlLoader.ts`:
- Around line 91-96: The catch block in useInviteUrlLoader.ts currently shows
raw error.message in the toast (toast.add) which may leak technical details;
change the toast.detail to use a user-friendly i18n key (e.g.
t('workspace.inviteFailedDetail') or t('g.unknownError')) instead of
error.message, and send the raw error to a logger (console.error or
processLogger.error) for debugging; update the toast.add call and add a logger
call inside the catch to preserve the original error for diagnostics while
keeping the UI message safe.
In `@src/stores/workspaceAuthStore.ts`:
- Around line 168-265: The switchWorkspace function can overwrite a newer
workspace when called by an in-flight refreshToken; capture the current
refreshRequestId into a local variable (e.g., localRequestId) immediately after
you increment it (or set it if unchanged), then before committing state updates
(setting currentWorkspace.value, workspaceToken.value, calling persistToSession
and scheduleTokenRefresh) verify that refreshRequestId === localRequestId and
bail out if they differ; reference switchWorkspace, refreshRequestId,
refreshToken, currentWorkspace, workspaceToken, persistToSession, and
scheduleTokenRefresh when applying this guard.
♻️ Duplicate comments (18)
src/components/dialog/content/workspace/EditWorkspaceDialogContent.vue (2)
12-18: Use the shared Button component for the close control.This raw
<button>should be replaced with the shared Button component for consistency and focus styling.Based on learnings, prefer the common button components over raw
<button>elements.
24-31: Associate the label with the input.The label isn’t linked to the input, which reduces screen‑reader clarity.
src/components/dialog/content/workspace/CreateWorkspaceDialogContent.vue (3)
12-18: Use the shared Button component for the close control.This raw
<button>should be replaced with the shared button component.Based on learnings, prefer the common button components over raw
<button>elements.
27-38: Associate the label with the input.Link the label via
for/idto improve accessibility.
37-52: Fix invalid:loadingbinding and prevent double‑submit.The
:loadingbinding is invalid without an expression and the action lacks a loading guard.Also applies to: 90-92
src/platform/auth/workspace/useWorkspaceAuth.test.ts (1)
68-70: Restore global stubs inafterEachto avoid cross-test leakage.
fetchis stubbed in several tests, but global stubs aren’t cleaned up. Addvi.unstubAllGlobals()alongside the timer cleanup.Suggested fix
afterEach(() => { vi.useRealTimers() + vi.unstubAllGlobals() })src/platform/workspace/stores/teamWorkspaceStore.ts (2)
361-383: Remove unreachable return after reload (or change the return type).
window.location.reload()makes thereturn workspaceStateunreachable. Consider switching the return type toPromise<void>and dropping the return, or remove the reload if callers need the state.Suggested fix (if return value isn’t used)
- async function createWorkspace(name: string): Promise<WorkspaceState> { + async function createWorkspace(name: string): Promise<void> { @@ - // Code after this won't run (page reloads) - return workspaceState + // Code after this won't run (page reloads)
594-603: Handle clipboard write failures to avoid unhandled rejections.
navigator.clipboard.writeText()can reject (permissions/secure context). Wrap it and surface a controlled error.Suggested fix
async function copyInviteLink(inviteId: string): Promise<string> { @@ const inviteLink = buildInviteLink(invite.token) - await navigator.clipboard.writeText(inviteLink) + try { + await navigator.clipboard.writeText(inviteLink) + } catch { + throw new Error('Failed to copy to clipboard') + } return inviteLink }src/components/dialog/GlobalDialog.vue (2)
7-12: Replace :class array with cn() merging.Use
cn()to compose conditional classes instead of array syntax. As per coding guidelines.
91-101: Move workspace sizing rules into Tailwind utilities.New
<style>rules should be moved into Tailwind class usage on the dialog elements. As per coding guidelines.src/components/topbar/CurrentUserPopoverWorkspace.vue (2)
26-185: Use Button components for clickable rows to restore keyboard semantics.Interactive divs (workspace selector and menu items) should be real buttons for keyboard/ARIA support. Based on learnings, use the shared button components instead of clickable divs.
195-198: Avoid adding new PrimeVue primitives in this component.Prefer internal primitives/wrappers over new PrimeVue Divider/Popover/Skeleton usage. As per coding guidelines.
src/platform/cloud/subscription/components/SubscriptionPanelContentWorkspace.vue (3)
46-68: Localize currency, zero‑state, and date formatting.Replace
$/0/0 / 0literals and manualMM/DD/YYformatting with vue‑i18nn()/d()(and translation keys where needed). As per coding guidelines.Also applies to: 137-168, 339-345
104-105: Prefer internal UI primitives over new PrimeVue Menu/Skeleton.Avoid introducing new PrimeVue components; use established internal patterns instead. As per coding guidelines.
Also applies to: 135-165, 249-250
446-449: Remove scoped style override; use Tailwind utilities.The new scoped style block should be replaced by Tailwind classes on the relevant elements. As per coding guidelines.
src/components/dialog/content/setting/WorkspacePanelContent.vue (2)
4-6: Remove Tailwind!important modifier on WorkspaceProfilePic.Avoid
!text-3xland rely on standard class merging/props. As per coding guidelines.
85-89: Switch :class array to cn() for menu item rows.Use
cn()for class merging instead of array syntax. As per coding guidelines.src/components/topbar/WorkspaceSwitcherPopover.vue (1)
71-104: Create workspace action still uses a non-focusable div.This should be a real button (or shared IconTextButton/TextButton) so keyboard users can activate it and disabled state is conveyed. Based on learnings, prefer the shared button components.
| <button | ||
| class="underline bg-transparent border-none cursor-pointer" | ||
| @click="handleCreateWorkspace" | ||
| > | ||
| {{ $t('workspacePanel.members.createNewWorkspace') }} | ||
| </button> |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Use the shared Button component for “create new workspace”.
This raw <button> bypasses shared focus/hover styles. Use the shared Button (or TextButton) instead.
♻️ Proposed change
- <button
- class="underline bg-transparent border-none cursor-pointer"
- `@click`="handleCreateWorkspace"
- >
- {{ $t('workspacePanel.members.createNewWorkspace') }}
- </button>
+ <Button
+ variant="muted-textonly"
+ class="underline"
+ `@click`="handleCreateWorkspace"
+ >
+ {{ $t('workspacePanel.members.createNewWorkspace') }}
+ </Button>Based on learnings, prefer shared button components for actions.
🤖 Prompt for AI Agents
In `@src/components/dialog/content/setting/MembersPanelContent.vue` around lines
324 - 329, Replace the raw HTML <button> in MembersPanelContent.vue with the
shared Button (or TextButton) component so the “create new workspace” action
uses the app's standard focus/hover styles; keep the
`@click`="handleCreateWorkspace" handler and the translated label {{
$t('workspacePanel.members.createNewWorkspace') }}, pass any existing
classes/variant props needed to match the original underline/bg-transparent
look, and add the import/registration for the shared Button/TextButton component
used across the codebase.
| <Button | ||
| v-if="permissions.canInviteMembers" | ||
| v-tooltip=" | ||
| inviteTooltip | ||
| ? { value: inviteTooltip, showDelay: 0 } | ||
| : { value: $t('workspacePanel.inviteMember'), showDelay: 300 } | ||
| " | ||
| variant="secondary" | ||
| size="lg" | ||
| :disabled="isInviteLimitReached" | ||
| :class="isInviteLimitReached && 'opacity-50 cursor-not-allowed'" | ||
| :aria-label="$t('workspacePanel.inviteMember')" | ||
| @click="handleInviteMember" |
There was a problem hiding this comment.
Drop aria-label from the Invite button’s visible text label.
The visible label already provides the accessible name; aria-label overrides it and should be removed. Based on learnings.
♻️ Proposed fix
- :aria-label="$t('workspacePanel.inviteMember')"📝 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.
| <Button | |
| v-if="permissions.canInviteMembers" | |
| v-tooltip=" | |
| inviteTooltip | |
| ? { value: inviteTooltip, showDelay: 0 } | |
| : { value: $t('workspacePanel.inviteMember'), showDelay: 300 } | |
| " | |
| variant="secondary" | |
| size="lg" | |
| :disabled="isInviteLimitReached" | |
| :class="isInviteLimitReached && 'opacity-50 cursor-not-allowed'" | |
| :aria-label="$t('workspacePanel.inviteMember')" | |
| @click="handleInviteMember" | |
| <Button | |
| v-if="permissions.canInviteMembers" | |
| v-tooltip=" | |
| inviteTooltip | |
| ? { value: inviteTooltip, showDelay: 0 } | |
| : { value: $t('workspacePanel.inviteMember'), showDelay: 300 } | |
| " | |
| variant="secondary" | |
| size="lg" | |
| :disabled="isInviteLimitReached" | |
| :class="isInviteLimitReached && 'opacity-50 cursor-not-allowed'" | |
| `@click`="handleInviteMember" |
🤖 Prompt for AI Agents
In `@src/components/dialog/content/setting/WorkspacePanelContent.vue` around lines
49 - 61, The Invite Button (Button component rendering when
permissions.canInviteMembers) currently sets an explicit aria-label which
overrides its visible text; remove the
:aria-label="$t('workspacePanel.inviteMember')" prop from that Button so the
visible label becomes the accessible name, leaving other attributes (v-tooltip,
:disabled, `@click`="handleInviteMember") intact.
| const menuItems = computed(() => { | ||
| const items = [] | ||
|
|
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Avoid implicit any[] for menuItems.
const items = [] infers any[]. Type it explicitly (e.g., MenuItem[] or a local interface) to keep type-safety. As per coding guidelines.
♻️ Suggested typing
-import Menu from 'primevue/menu'
+import Menu from 'primevue/menu'
+import type { MenuItem } from 'primevue/menuitem'
@@
-const menuItems = computed(() => {
- const items = []
+const menuItems = computed(() => {
+ const items: MenuItem[] = []🤖 Prompt for AI Agents
In `@src/components/dialog/content/setting/WorkspacePanelContent.vue` around lines
199 - 201, The computed property menuItems currently initializes items with
`const items = []`, which infers `any[]`; change this to an explicitly typed
array (e.g., `const items: MenuItem[] = []` or define a local interface and use
that) and ensure the computed's return type matches (typing the computed as
`computed<MenuItem[]>(...)` if needed); update or import the appropriate
MenuItem type and apply the explicit type to both the `items` variable and the
`menuItems` computed to enforce type-safety in WorkspacePanelContent.vue.
| @keydown.enter="isValidName && onSave()" | ||
| /> | ||
| </div> | ||
| </div> | ||
|
|
||
| <!-- Footer --> | ||
| <div class="flex items-center justify-end gap-4 px-4 py-4"> | ||
| <Button variant="muted-textonly" @click="onCancel"> | ||
| {{ $t('g.cancel') }} | ||
| </Button> | ||
| <Button | ||
| variant="primary" | ||
| size="lg" | ||
| :loading | ||
| :disabled="!isValidName" | ||
| @click="onSave" |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n src/components/dialog/content/workspace/EditWorkspaceDialogContent.vue | head -100Repository: Comfy-Org/ComfyUI_frontend
Length of output: 4194
🏁 Script executed:
cat -n src/components/dialog/content/workspace/EditWorkspaceDialogContent.vue | tail -20Repository: Comfy-Org/ComfyUI_frontend
Length of output: 823
🏁 Script executed:
cat -n src/components/ui/button/Button.vue | head -80Repository: Comfy-Org/ComfyUI_frontend
Length of output: 1205
Fix invalid :loading binding and guard against double‑submit.
:loading without an expression is invalid Vue syntax and should be :loading="loading". The handler and button also allow multiple saves while a request is in flight since loading state is not checked.
Proposed fix
- `@keydown.enter`="isValidName && onSave()"
+ `@keydown.enter`="isValidName && !loading && onSave()" <Button
variant="primary"
size="lg"
- :loading
- :disabled="!isValidName"
+ :loading="loading"
+ :disabled="!isValidName || loading"
`@click`="onSave" async function onSave() {
- if (!isValidName.value) return
+ if (loading.value || !isValidName.value) return
loading.value = true🤖 Prompt for AI Agents
In `@src/components/dialog/content/workspace/EditWorkspaceDialogContent.vue`
around lines 31 - 46, The submit Button in EditWorkspaceDialogContent.vue uses
an invalid binding :loading and allows double-submits; change the attribute to
:loading="loading", make the Button disabled when loading (e.g.
:disabled="!isValidName || loading"), and update the Enter key handler to check
loading (replace `@keydown.enter`="isValidName && onSave()" with a guard that also
requires !loading). Inside the onSave method, add an early return if loading is
true, set loading = true before the async request and set loading = false in
finally (or on error) so the UI and handler properly prevent double-submit.
| <button | ||
| class="cursor-pointer rounded border-none bg-transparent p-0 text-muted-foreground transition-colors hover:text-base-foreground focus-visible:outline-none focus-visible:ring-1 focus-visible:ring-secondary-foreground" | ||
| :aria-label="$t('g.close')" | ||
| @click="onCancel" | ||
| > | ||
| <i class="pi pi-times size-4" /> | ||
| </button> |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Prefer the shared icon button component for the close control.
This is a plain <button>; the repo prefers shared button components for consistency/theming. Consider swapping to IconButton. Based on learnings, prefer the shared button components.
🤖 Prompt for AI Agents
In `@src/components/dialog/content/workspace/InviteMemberDialogContent.vue` around
lines 16 - 22, Replace the plain <button> close control in
InviteMemberDialogContent.vue with the shared IconButton component: remove the
raw <button> element and use IconButton (preserving the aria-label from
$t('g.close') and the `@click`="onCancel" handler), map any needed styling/size to
IconButton props (e.g., size/variant/aria attributes) and render the existing <i
class="pi pi-times size-4" /> as the icon content or via the IconButton icon
prop so behavior and accessibility remain identical while using the shared
component.
| // Accept workspace invite from URL if present (e.g., ?invite=TOKEN) | ||
| // Uses watch because feature flags load asynchronously - flag may be false initially | ||
| // then become true once remoteConfig or websocket features are loaded | ||
| if (inviteUrlLoader) { | ||
| const stopWatching = watch( | ||
| () => flags.teamWorkspacesEnabled, | ||
| async (enabled) => { | ||
| if (enabled) { | ||
| stopWatching() | ||
| await inviteUrlLoader.loadInviteFromUrl() | ||
| } | ||
| }, | ||
| { immediate: true } | ||
| ) | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cd src/components/graph && head -n 500 GraphCanvas.vue | tail -n 100Repository: Comfy-Org/ComfyUI_frontend
Length of output: 3057
🌐 Web query:
Vue 3 watch immediate: true callback synchronous execution timing
💡 Result:
In Vue 3, immediate: true makes the watcher’s callback run right when the watcher is created (i.e., “eagerly”), not waiting for the first change. In Options API, that first run happens just before the created hook. In Composition API, it runs immediately after watch() is set up. [1]
However, “immediate” does not mean “synchronous for all future changes.” After that initial eager call, watcher callbacks follow the normal flush timing / scheduler behavior:
- Default (
flush: 'pre'): callbacks are batched and run after parent updates (if any) and before the owner component’s DOM updates, so they’re not guaranteed to run inline with each mutation. [1] flush: 'post': runs after the owner component’s DOM updates (still batched). [1]flush: 'sync': runs synchronously on each reactive change (no batching). Use sparingly. [1]
Also note: on the first (immediate) invocation, the watcher’s oldValue is undefined. [2]
Sources: Vue Guide – Watchers (Eager Watchers + Callback Flush Timing) [1], Vue API – Reactivity Core (watch() options) [2].
🏁 Script executed:
# Check Vue 3 watch implementation to understand execution timing
rg "watch.*immediate" --type vue -A 5 -B 2 src/ | head -n 100Repository: Comfy-Org/ComfyUI_frontend
Length of output: 97
🏁 Script executed:
# Search for Vue watch implementations or similar patterns in the codebase
rg "watch.*immediate.*true" --type-list | head -5Repository: Comfy-Org/ComfyUI_frontend
Length of output: 459
🏁 Script executed:
# Look for other patterns with watch/immediate to see if async is standard
rg "watch\(" src/ --type ts --type vue -A 3 | grep -A 3 "immediate.*true" | head -30Repository: Comfy-Org/ComfyUI_frontend
Length of output: 97
🌐 Web query:
Vue 3 watch immediate: true with async callback TDZ temporal dead zone
💡 Result:
In Vue 3, watch(..., { immediate: true }) runs the callback immediately during watcher creation (i.e., synchronously while your <script setup>/setup() is still executing) [1]. If that callback references a let/const binding that hasn’t been initialized yet, JavaScript throws a TDZ (temporal dead zone) error like ReferenceError: Cannot access 'X' before initialization [2].
This commonly shows up in two patterns:
- Calling a function declared later
watch(source, async () => { test() }, { immediate: true })
const test = () => {}On the immediate run, test is still in TDZ → error (often surfaced as Uncaught (in promise) because the callback is async) [3].
- Stopping the watcher inside itself
const unwatch = watch(source, () => {
unwatch() // TDZ on the immediate run
}, { immediate: true })Because the immediate callback can run before watch() returns and assigns unwatch, this hits TDZ [4].
Fixes
- Move declarations above the watcher or use a function declaration (hoisted) instead of
const fn = () => {}[3]. - If your goal is “run once then stop”, use Vue’s built-in
once: true(Vue 3.4+) instead of manually callingunwatch()[1].
Sources: Vue watch options (immediate, once) [1]; JS TDZ explanation [2]; Vue <script setup> example causing Cannot access 'test' before initialization [3]; unwatch TDZ with immediate: true [4].
🌐 Web query:
JavaScript async function callback immediate execution synchronous
💡 Result:
- Callbacks aren’t automatically async. Whether a callback runs immediately (synchronously) or later (asynchronously) depends on the API you pass it to (e.g.,
Array.prototype.forEachruns its callback immediately;setTimeoutschedules it). asyncdoesn’t make the whole function “run later.” When you call anasync function, it starts executing synchronously right away and keeps running until the firstawait(or until it returns/throws). The call itself returns aPromise, but the “top part” of the function body can still run before the caller continues. [1]- After an
await, continuation is deferred. Even if youawaitan already-resolved value, execution resumes asynchronously (scheduled as a microtask), after the current call stack finishes. [2] - Promise continuations run before timers.
.then(...)/awaitcontinuations (microtasks) are processed beforesetTimeout(..., 0)callbacks (macrotasks). [3]
Example showing “immediate” execution up to the first await:
async function f() {
console.log("A"); // runs now (sync)
await 0; // yields
console.log("B"); // runs later (microtask)
}
console.log("C");
f();
console.log("D");
// Output: C, A, D, BSources: MDN async function [1], MDN await [2], microtask vs macrotask ordering [3].
[1] (developer.mozilla.org)
[2] (developer.mozilla.org)
[3] (stackoverflow.com)
Citations:
- 1: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Statements/async_function?utm_source=openai
- 2: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/await?utm_source=openai
- 3: https://stackoverflow.com/questions/60977681/why-promise-is-executed-before-settimeout?utm_source=openai
🏁 Script executed:
# Check if there are other uses of watch with async callbacks in the codebase
rg "watch\(" src/ -A 2 | grep -B 2 "async" | head -40Repository: Comfy-Org/ComfyUI_frontend
Length of output: 2488
Use let with optional chaining to avoid TDZ when stopping an immediate watcher.
With immediate: true, the async callback executes synchronously (up to the first await) when the watcher is created. If flags.teamWorkspacesEnabled is already true, stopWatching() will be called before the const stopWatching = watch(...) assignment completes, throwing a TDZ error. Initialize with let stopWatching: (() => void) | undefined and use optional chaining stopWatching?.() to avoid this.
Proposed fix
if (inviteUrlLoader) {
+ let stopWatching: (() => void) | undefined
- const stopWatching = watch(
+ stopWatching = watch(
() => flags.teamWorkspacesEnabled,
async (enabled) => {
- if (enabled) {
- stopWatching()
+ if (!enabled) return
+ stopWatching?.()
await inviteUrlLoader.loadInviteFromUrl()
- }
},
{ immediate: true }
)
}🤖 Prompt for AI Agents
In `@src/components/graph/GraphCanvas.vue` around lines 468 - 482, The watcher
uses immediate:true so the callback can run before the const assignment
completes, causing a TDZ when calling stopWatching(); change the declaration to
let stopWatching: (() => void) | undefined (or appropriate type) and replace the
direct call with optional chaining stopWatching?.() inside the async watcher
callback; locate the watch(...) invocation that references
flags.teamWorkspacesEnabled, stopWatching, inviteUrlLoader, and
inviteUrlLoader.loadInviteFromUrl() to apply this change.
| function getRoleLabel(role: AvailableWorkspace['role']): string { | ||
| if (role === 'owner') return t('workspaceSwitcher.roleOwner') | ||
| if (role === 'member') return t('workspaceSwitcher.roleMember') | ||
| return '' | ||
| } |
There was a problem hiding this comment.
Admin role label is missing.
Roles include owner/admin/member; admin currently renders as empty string. Add an admin label and translation entry.
✅ Suggested fix
function getRoleLabel(role: AvailableWorkspace['role']): string {
if (role === 'owner') return t('workspaceSwitcher.roleOwner')
+ if (role === 'admin') return t('workspaceSwitcher.roleAdmin')
if (role === 'member') return t('workspaceSwitcher.roleMember')
return ''
}🤖 Prompt for AI Agents
In `@src/components/topbar/WorkspaceSwitcherPopover.vue` around lines 164 - 168,
The getRoleLabel function currently returns empty string for the 'admin' role;
update getRoleLabel(AvailableWorkspace['role']) to handle role === 'admin' and
return the appropriate i18n key (e.g. t('workspaceSwitcher.roleAdmin')), and add
the corresponding translation entry for workspaceSwitcher.roleAdmin in the
locale files so admins render the proper label.
| <SearchBox | ||
| v-model:model-value="searchQuery" | ||
| class="settings-search-box mb-2 w-full" | ||
| :placeholder="$t('g.searchSettings') + '...'" | ||
| :debounce-time="128" |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, find and check the main translation file
find src/locales -name "*.json" -type f | head -20Repository: Comfy-Org/ComfyUI_frontend
Length of output: 629
🏁 Script executed:
# Read the translation file to check for g.searchSettings and g.searchSettingsPlaceholder
cat src/locales/en/main.json | jq '.g | {searchSettings, searchSettingsPlaceholder}' 2>/dev/null || grep -A2 -B2 "searchSettings" src/locales/en/main.json | head -20Repository: Comfy-Org/ComfyUI_frontend
Length of output: 148
🏁 Script executed:
# Inspect the actual Vue component file
sed -n '1,50p' src/platform/settings/components/SettingDialogContent.vue | cat -nRepository: Comfy-Org/ComfyUI_frontend
Length of output: 2050
🏁 Script executed:
# Check if there are other similar patterns with concatenated ellipsis in Vue files
rg -t vue '\$t\([^)]+\)\s*\+\s*["\']\.\.\.["\'']' src/Repository: Comfy-Org/ComfyUI_frontend
Length of output: 138
🏁 Script executed:
# Check other uses of g.searchSettings and g.searchSettingsPlaceholder
rg 'searchSettings' src/locales/en/main.json -A 2 -B 2Repository: Comfy-Org/ComfyUI_frontend
Length of output: 250
🏁 Script executed:
# Look for how other placeholders are handled in the codebase
rg -t vue 'placeholder.*\$t' src/ | head -20Repository: Comfy-Org/ComfyUI_frontend
Length of output: 97
🏁 Script executed:
# Search for similar patterns with ellipsis in Vue files (escaped properly)
rg 'placeholder.*\+.*\.\.\.' src/ --type vueRepository: Comfy-Org/ComfyUI_frontend
Length of output: 97
🏁 Script executed:
# Check other placeholder usages in Vue
rg 'placeholder=.*\$t' src/ --type vue | head -15Repository: Comfy-Org/ComfyUI_frontend
Length of output: 97
🏁 Script executed:
# Verify the translation file structure for placeholder-related keys
jq '.g | to_entries[] | select(.key | contains("search"))' src/locales/en/main.jsonRepository: Comfy-Org/ComfyUI_frontend
Length of output: 665
🏁 Script executed:
# Search for usage of searchPlaceholder in the codebase
rg 'searchPlaceholder' src/Repository: Comfy-Org/ComfyUI_frontend
Length of output: 5553
🏁 Script executed:
# Check if any components use g.searchPlaceholder
rg 'g\.searchPlaceholder' src/Repository: Comfy-Org/ComfyUI_frontend
Length of output: 277
🏁 Script executed:
# Get context around the SearchBox component to understand what placeholder should say
grep -r "SearchBox" src/ -A 3 -B 1 | grep -A 3 "placeholder"Repository: Comfy-Org/ComfyUI_frontend
Length of output: 1942
Localize the search placeholder ellipsis.
Hard‑coded '...' prevents full localization; extract the entire placeholder to a translation key as shown below.
Suggested change
- :placeholder="$t('g.searchSettings') + '...'"
+ :placeholder="$t('g.searchSettingsPlaceholder')"Per the coding guidelines, all user‑facing strings must use i18n for full localization support. Add the corresponding translation entry to src/locales/en/main.json with value "Search Settings...".
🤖 Prompt for AI Agents
In `@src/platform/settings/components/SettingDialogContent.vue` around lines 17 -
21, Replace the concatenation of $t('g.searchSettings') + '...' with a single
i18n key for the full placeholder string used in the SearchBox inside
SettingDialogContent.vue (e.g. $t('g.searchSettingsPlaceholder')); update the
SearchBox placeholder prop to use that key and add the corresponding translation
entry "Search Settings..." to src/locales/en/main.json (key name must match the
one used in the component).
| } catch (error) { | ||
| toast.add({ | ||
| severity: 'error', | ||
| summary: t('workspace.inviteFailed'), | ||
| detail: error instanceof Error ? error.message : t('g.unknownError'), | ||
| life: 5000 |
There was a problem hiding this comment.
Avoid showing raw error messages to users.
error.message can leak technical details; prefer an i18n error detail that’s user-friendly and actionable. As per coding guidelines, ...
🐛 Suggested change
- detail: error instanceof Error ? error.message : t('g.unknownError'),
+ detail: t('workspace.inviteFailedDetail')🤖 Prompt for AI Agents
In `@src/platform/workspace/composables/useInviteUrlLoader.ts` around lines 91 -
96, The catch block in useInviteUrlLoader.ts currently shows raw error.message
in the toast (toast.add) which may leak technical details; change the
toast.detail to use a user-friendly i18n key (e.g.
t('workspace.inviteFailedDetail') or t('g.unknownError')) instead of
error.message, and send the raw error to a logger (console.error or
processLogger.error) for debugging; update the toast.add call and add a logger
call inside the catch to preserve the original error for diagnostics while
keeping the UI message safe.
| async function switchWorkspace(workspaceId: string): Promise<void> { | ||
| if (!flags.teamWorkspacesEnabled) { | ||
| return | ||
| } | ||
|
|
||
| // Only increment request ID when switching to a different workspace | ||
| // This invalidates stale refresh operations for the old workspace | ||
| // but allows refresh operations for the same workspace to complete | ||
| if (currentWorkspace.value?.id !== workspaceId) { | ||
| refreshRequestId++ | ||
| } | ||
|
|
||
| isLoading.value = true | ||
| error.value = null | ||
|
|
||
| try { | ||
| const firebaseAuthStore = useFirebaseAuthStore() | ||
| const firebaseToken = await firebaseAuthStore.getIdToken() | ||
| if (!firebaseToken) { | ||
| throw new WorkspaceAuthError( | ||
| t('workspaceAuth.errors.notAuthenticated'), | ||
| 'NOT_AUTHENTICATED' | ||
| ) | ||
| } | ||
|
|
||
| const response = await fetch(api.apiURL('/auth/token'), { | ||
| method: 'POST', | ||
| headers: { | ||
| Authorization: `Bearer ${firebaseToken}`, | ||
| 'Content-Type': 'application/json' | ||
| }, | ||
| body: JSON.stringify({ workspace_id: workspaceId }) | ||
| }) | ||
|
|
||
| if (!response.ok) { | ||
| const errorData = await response.json().catch(() => ({})) | ||
| const message = errorData.message || response.statusText | ||
|
|
||
| if (response.status === 401) { | ||
| throw new WorkspaceAuthError( | ||
| t('workspaceAuth.errors.invalidFirebaseToken'), | ||
| 'INVALID_FIREBASE_TOKEN' | ||
| ) | ||
| } | ||
| if (response.status === 403) { | ||
| throw new WorkspaceAuthError( | ||
| t('workspaceAuth.errors.accessDenied'), | ||
| 'ACCESS_DENIED' | ||
| ) | ||
| } | ||
| if (response.status === 404) { | ||
| throw new WorkspaceAuthError( | ||
| t('workspaceAuth.errors.workspaceNotFound'), | ||
| 'WORKSPACE_NOT_FOUND' | ||
| ) | ||
| } | ||
|
|
||
| throw new WorkspaceAuthError( | ||
| t('workspaceAuth.errors.tokenExchangeFailed', { error: message }), | ||
| 'TOKEN_EXCHANGE_FAILED' | ||
| ) | ||
| } | ||
|
|
||
| const rawData = await response.json() | ||
| const parseResult = WorkspaceTokenResponseSchema.safeParse(rawData) | ||
|
|
||
| if (!parseResult.success) { | ||
| throw new WorkspaceAuthError( | ||
| t('workspaceAuth.errors.tokenExchangeFailed', { | ||
| error: fromZodError(parseResult.error).message | ||
| }), | ||
| 'TOKEN_EXCHANGE_FAILED' | ||
| ) | ||
| } | ||
|
|
||
| const data = parseResult.data | ||
| const expiresAt = new Date(data.expires_at).getTime() | ||
|
|
||
| if (isNaN(expiresAt)) { | ||
| throw new WorkspaceAuthError( | ||
| t('workspaceAuth.errors.tokenExchangeFailed', { | ||
| error: 'Invalid expiry timestamp' | ||
| }), | ||
| 'TOKEN_EXCHANGE_FAILED' | ||
| ) | ||
| } | ||
|
|
||
| const workspaceWithRole: WorkspaceWithRole = { | ||
| ...data.workspace, | ||
| role: data.role | ||
| } | ||
|
|
||
| currentWorkspace.value = workspaceWithRole | ||
| workspaceToken.value = data.token | ||
|
|
||
| persistToSession(workspaceWithRole, data.token, expiresAt) | ||
| scheduleTokenRefresh(expiresAt) | ||
| } catch (err) { |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Get the full workspaceAuthStore.ts file to understand the context
find . -type f -name "workspaceAuthStore.ts" | head -20Repository: Comfy-Org/ComfyUI_frontend
Length of output: 104
🏁 Script executed:
cat -n ./src/stores/workspaceAuthStore.tsRepository: Comfy-Org/ComfyUI_frontend
Length of output: 13279
🏁 Script executed:
# Verify the exact flow and confirm no other guards exist
rg -n "refreshRequestId" ./src/stores/workspaceAuthStore.tsRepository: Comfy-Org/ComfyUI_frontend
Length of output: 380
Guard against stale refresh overwriting a newer workspace switch.
When refreshToken() calls switchWorkspace(), a race condition exists: if the user manually switches workspaces while that refresh call is in-flight, the refresh's switchWorkspace() can still commit the old workspace data at lines 260–264, overwriting the newer state. The check at line 286 in refreshToken() happens before the await, so it cannot protect against state commits that occur during the async operation.
Capture refreshRequestId at the start of switchWorkspace() (after incrementing) and check it before applying state updates. If the ID changed during the async work, bail out to prevent overwriting a newer workspace context.
🛠️ Suggested guard
async function switchWorkspace(workspaceId: string): Promise<void> {
if (!flags.teamWorkspacesEnabled) {
return
}
// Only increment request ID when switching to a different workspace
if (currentWorkspace.value?.id !== workspaceId) {
refreshRequestId++
}
+ const switchRequestId = refreshRequestId
isLoading.value = true
error.value = null
@@
- currentWorkspace.value = workspaceWithRole
- workspaceToken.value = data.token
+ if (switchRequestId !== refreshRequestId) {
+ return
+ }
+ currentWorkspace.value = workspaceWithRole
+ workspaceToken.value = data.token
persistToSession(workspaceWithRole, data.token, expiresAt)
scheduleTokenRefresh(expiresAt)📝 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.
| async function switchWorkspace(workspaceId: string): Promise<void> { | |
| if (!flags.teamWorkspacesEnabled) { | |
| return | |
| } | |
| // Only increment request ID when switching to a different workspace | |
| // This invalidates stale refresh operations for the old workspace | |
| // but allows refresh operations for the same workspace to complete | |
| if (currentWorkspace.value?.id !== workspaceId) { | |
| refreshRequestId++ | |
| } | |
| isLoading.value = true | |
| error.value = null | |
| try { | |
| const firebaseAuthStore = useFirebaseAuthStore() | |
| const firebaseToken = await firebaseAuthStore.getIdToken() | |
| if (!firebaseToken) { | |
| throw new WorkspaceAuthError( | |
| t('workspaceAuth.errors.notAuthenticated'), | |
| 'NOT_AUTHENTICATED' | |
| ) | |
| } | |
| const response = await fetch(api.apiURL('/auth/token'), { | |
| method: 'POST', | |
| headers: { | |
| Authorization: `Bearer ${firebaseToken}`, | |
| 'Content-Type': 'application/json' | |
| }, | |
| body: JSON.stringify({ workspace_id: workspaceId }) | |
| }) | |
| if (!response.ok) { | |
| const errorData = await response.json().catch(() => ({})) | |
| const message = errorData.message || response.statusText | |
| if (response.status === 401) { | |
| throw new WorkspaceAuthError( | |
| t('workspaceAuth.errors.invalidFirebaseToken'), | |
| 'INVALID_FIREBASE_TOKEN' | |
| ) | |
| } | |
| if (response.status === 403) { | |
| throw new WorkspaceAuthError( | |
| t('workspaceAuth.errors.accessDenied'), | |
| 'ACCESS_DENIED' | |
| ) | |
| } | |
| if (response.status === 404) { | |
| throw new WorkspaceAuthError( | |
| t('workspaceAuth.errors.workspaceNotFound'), | |
| 'WORKSPACE_NOT_FOUND' | |
| ) | |
| } | |
| throw new WorkspaceAuthError( | |
| t('workspaceAuth.errors.tokenExchangeFailed', { error: message }), | |
| 'TOKEN_EXCHANGE_FAILED' | |
| ) | |
| } | |
| const rawData = await response.json() | |
| const parseResult = WorkspaceTokenResponseSchema.safeParse(rawData) | |
| if (!parseResult.success) { | |
| throw new WorkspaceAuthError( | |
| t('workspaceAuth.errors.tokenExchangeFailed', { | |
| error: fromZodError(parseResult.error).message | |
| }), | |
| 'TOKEN_EXCHANGE_FAILED' | |
| ) | |
| } | |
| const data = parseResult.data | |
| const expiresAt = new Date(data.expires_at).getTime() | |
| if (isNaN(expiresAt)) { | |
| throw new WorkspaceAuthError( | |
| t('workspaceAuth.errors.tokenExchangeFailed', { | |
| error: 'Invalid expiry timestamp' | |
| }), | |
| 'TOKEN_EXCHANGE_FAILED' | |
| ) | |
| } | |
| const workspaceWithRole: WorkspaceWithRole = { | |
| ...data.workspace, | |
| role: data.role | |
| } | |
| currentWorkspace.value = workspaceWithRole | |
| workspaceToken.value = data.token | |
| persistToSession(workspaceWithRole, data.token, expiresAt) | |
| scheduleTokenRefresh(expiresAt) | |
| } catch (err) { | |
| async function switchWorkspace(workspaceId: string): Promise<void> { | |
| if (!flags.teamWorkspacesEnabled) { | |
| return | |
| } | |
| // Only increment request ID when switching to a different workspace | |
| // This invalidates stale refresh operations for the old workspace | |
| // but allows refresh operations for the same workspace to complete | |
| if (currentWorkspace.value?.id !== workspaceId) { | |
| refreshRequestId++ | |
| } | |
| const switchRequestId = refreshRequestId | |
| isLoading.value = true | |
| error.value = null | |
| try { | |
| const firebaseAuthStore = useFirebaseAuthStore() | |
| const firebaseToken = await firebaseAuthStore.getIdToken() | |
| if (!firebaseToken) { | |
| throw new WorkspaceAuthError( | |
| t('workspaceAuth.errors.notAuthenticated'), | |
| 'NOT_AUTHENTICATED' | |
| ) | |
| } | |
| const response = await fetch(api.apiURL('/auth/token'), { | |
| method: 'POST', | |
| headers: { | |
| Authorization: `Bearer ${firebaseToken}`, | |
| 'Content-Type': 'application/json' | |
| }, | |
| body: JSON.stringify({ workspace_id: workspaceId }) | |
| }) | |
| if (!response.ok) { | |
| const errorData = await response.json().catch(() => ({})) | |
| const message = errorData.message || response.statusText | |
| if (response.status === 401) { | |
| throw new WorkspaceAuthError( | |
| t('workspaceAuth.errors.invalidFirebaseToken'), | |
| 'INVALID_FIREBASE_TOKEN' | |
| ) | |
| } | |
| if (response.status === 403) { | |
| throw new WorkspaceAuthError( | |
| t('workspaceAuth.errors.accessDenied'), | |
| 'ACCESS_DENIED' | |
| ) | |
| } | |
| if (response.status === 404) { | |
| throw new WorkspaceAuthError( | |
| t('workspaceAuth.errors.workspaceNotFound'), | |
| 'WORKSPACE_NOT_FOUND' | |
| ) | |
| } | |
| throw new WorkspaceAuthError( | |
| t('workspaceAuth.errors.tokenExchangeFailed', { error: message }), | |
| 'TOKEN_EXCHANGE_FAILED' | |
| ) | |
| } | |
| const rawData = await response.json() | |
| const parseResult = WorkspaceTokenResponseSchema.safeParse(rawData) | |
| if (!parseResult.success) { | |
| throw new WorkspaceAuthError( | |
| t('workspaceAuth.errors.tokenExchangeFailed', { | |
| error: fromZodError(parseResult.error).message | |
| }), | |
| 'TOKEN_EXCHANGE_FAILED' | |
| ) | |
| } | |
| const data = parseResult.data | |
| const expiresAt = new Date(data.expires_at).getTime() | |
| if (isNaN(expiresAt)) { | |
| throw new WorkspaceAuthError( | |
| t('workspaceAuth.errors.tokenExchangeFailed', { | |
| error: 'Invalid expiry timestamp' | |
| }), | |
| 'TOKEN_EXCHANGE_FAILED' | |
| ) | |
| } | |
| const workspaceWithRole: WorkspaceWithRole = { | |
| ...data.workspace, | |
| role: data.role | |
| } | |
| if (switchRequestId !== refreshRequestId) { | |
| return | |
| } | |
| currentWorkspace.value = workspaceWithRole | |
| workspaceToken.value = data.token | |
| persistToSession(workspaceWithRole, data.token, expiresAt) | |
| scheduleTokenRefresh(expiresAt) | |
| } catch (err) { |
🤖 Prompt for AI Agents
In `@src/stores/workspaceAuthStore.ts` around lines 168 - 265, The switchWorkspace
function can overwrite a newer workspace when called by an in-flight
refreshToken; capture the current refreshRequestId into a local variable (e.g.,
localRequestId) immediately after you increment it (or set it if unchanged),
then before committing state updates (setting currentWorkspace.value,
workspaceToken.value, calling persistToSession and scheduleTokenRefresh) verify
that refreshRequestId === localRequestId and bail out if they differ; reference
switchWorkspace, refreshRequestId, refreshToken, currentWorkspace,
workspaceToken, persistToSession, and scheduleTokenRefresh when applying this
guard.
… while workspace is in… (#8324) Backport of #8323 to `cloud/1.37` Automatically created by backport workflow. ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-8324-backport-cloud-1-37-fix-workspace-icon-flash-and-credits-showing-0-while-workspace-is--2f56d73d365081e18764dc79feadbf3a) by [Unito](https://www.unito.io) Co-authored-by: Simula_r <18093452+simula-r@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
src/components/topbar/CurrentUserButton.test.ts (3)
102-102: Consider using the real Button component instead of stubbing.Based on learnings, primitive UI components like Button should not be stubbed in tests. Import and register the real component to ensure tests accurately reflect production behavior.
♻️ Suggested fix
global: { plugins: [i18n], stubs: { // Use shallow mount for popover to make testing easier Popover: { template: '<div><slot></slot></div>', methods: { toggle: vi.fn(), hide: vi.fn() } }, - Button: true } }
117-122: Avoid@ts-expect-error- fix the underlying type issue instead.Accessing
wrapper.vm.popoverbreaks component encapsulation and requires type suppression. Consider usingdefineExposein the component if imperative access is needed, or test the behavior through user interactions instead.As per coding guidelines: "Avoid using
@ts-expect-error- fix the underlying type issue instead."
128-138: Refactor to avoid multiple@ts-expect-errorcomments.The test directly manipulates internal component state (
wrapper.vm.popover,wrapper.vm.closePopover), requiring type suppressions. Consider either:
- Exposing these via
defineExposein the component for legitimate imperative access- Testing behavior through DOM interactions rather than internal method calls
♻️ Alternative approach - test via DOM interaction
it('hides popover when close event is emitted', async () => { const wrapper = mountComponent() // Find the popover content and emit close event // This tests the actual integration rather than internal methods })
🤖 Fix all issues with AI agents
In `@src/components/topbar/CurrentUserButton.vue`:
- Around line 49-58: The current conditional renders CurrentUserPopoverWorkspace
only when teamWorkspacesEnabled && initState === 'ready' and the legacy
CurrentUserPopover only when !teamWorkspacesEnabled, leaving a gap when
teamWorkspacesEnabled is true but initState !== 'ready'; update the logic in
CurrentUserButton.vue to handle the loading case—either render a loading variant
or fallback to CurrentUserPopover while initState is not 'ready'. Concretely,
change the template conditions around CurrentUserPopoverWorkspace and
CurrentUserPopover (references: CurrentUserPopoverWorkspace, CurrentUserPopover,
teamWorkspacesEnabled, initState, closePopover) so that when
teamWorkspacesEnabled is true and initState !== 'ready' you display a visible
fallback (e.g., the legacy CurrentUserPopover or a loading component) instead of
rendering nothing.
- Around line 109-112: The computed property workspaceName is redundant because
it only returns teamWorkspaceName.value when showWorkspaceIcon is true and the
template already conditionally renders that branch via v-else-if; remove the
workspaceName computed entirely and update the template to reference
teamWorkspaceName directly in the branch guarded by showWorkspaceIcon (keep
showWorkspaceIcon and teamWorkspaceName as-is and remove all references to
workspaceName).
- Line 66: The PrimeVue Skeleton import and usage in CurrentUserButton.vue
should be removed and replaced with either a small custom circular placeholder
component (e.g., create CircularSkeleton that renders a 32x32px circular div
with neutral background and optional shimmer) or use the existing DotSpinner
from src/components/common/DotSpinner.vue as the loading indicator; update the
import (remove "primevue/skeleton") and swap any <Skeleton> usage in the
CurrentUserButton component to either <CircularSkeleton /> (if you add it) or
<DotSpinner /> and ensure the element is constrained to 32x32px and styled as
circular to match the expected avatar placeholder.
In `@src/components/topbar/CurrentUserPopoverWorkspace.vue`:
- Around line 302-308: The window.open call in handleOpenPartnerNodesInfo is
missing security attributes and can allow tabnabbing; update the function so the
opened window is created with no opener (e.g., pass the third argument with
"noopener,noreferrer" to window.open or immediately set newWin.opener = null
after opening) when calling buildDocsUrl(docsPaths.partnerNodesPricing...), and
then still call emit('close'). Ensure the change targets
handleOpenPartnerNodesInfo so the new window cannot access window.opener.
- Around line 310-313: The handleLogout function should guard against errors
from handleSignOut so emit('close') always runs and the user gets feedback; wrap
the await handleSignOut() in a try/catch/finally: in try await handleSignOut(),
in catch call the user-facing error notifier (e.g., a toast or process error
handler) with the caught error, and in finally call emit('close') so the popover
is closed regardless of success or failure; update the handleLogout
implementation to reference handleSignOut and emit('close') accordingly.
- Around line 295-300: Move the useTelemetry() call out of the click handler and
instantiate it once at the component setup/top-level (e.g., const telemetry =
useTelemetry();) then update handleTopUp to call
telemetry.trackAddApiCreditButtonClicked() instead of calling useTelemetry()
inside the function; ensure the import for useTelemetry is present and the
handler (handleTopUp) uses the top-level telemetry instance so the composable is
created only once per component lifecycle.
| <!-- Workspace mode: workspace-aware popover (only when ready) --> | ||
| <CurrentUserPopoverWorkspace | ||
| v-if="teamWorkspacesEnabled && initState === 'ready'" | ||
| @close="closePopover" | ||
| /> | ||
| <!-- Legacy mode: original popover --> | ||
| <CurrentUserPopover | ||
| v-else-if="!teamWorkspacesEnabled" | ||
| @close="closePopover" | ||
| /> |
There was a problem hiding this comment.
Potential UI gap when workspace is loading.
When teamWorkspacesEnabled is true but initState is not 'ready' (e.g., during loading), neither popover component renders. Users clicking the button during workspace initialization will see an empty popover.
Consider showing a loading state or falling back to the legacy popover during initialization:
♻️ Suggested fix
<!-- Workspace mode: workspace-aware popover (only when ready) -->
<CurrentUserPopoverWorkspace
v-if="teamWorkspacesEnabled && initState === 'ready'"
`@close`="closePopover"
/>
- <!-- Legacy mode: original popover -->
+ <!-- Legacy mode or workspace loading: original popover -->
<CurrentUserPopover
- v-else-if="!teamWorkspacesEnabled"
+ v-else
`@close`="closePopover"
/>📝 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.
| <!-- Workspace mode: workspace-aware popover (only when ready) --> | |
| <CurrentUserPopoverWorkspace | |
| v-if="teamWorkspacesEnabled && initState === 'ready'" | |
| @close="closePopover" | |
| /> | |
| <!-- Legacy mode: original popover --> | |
| <CurrentUserPopover | |
| v-else-if="!teamWorkspacesEnabled" | |
| @close="closePopover" | |
| /> | |
| <!-- Workspace mode: workspace-aware popover (only when ready) --> | |
| <CurrentUserPopoverWorkspace | |
| v-if="teamWorkspacesEnabled && initState === 'ready'" | |
| `@close`="closePopover" | |
| /> | |
| <!-- Legacy mode or workspace loading: original popover --> | |
| <CurrentUserPopover | |
| v-else | |
| `@close`="closePopover" | |
| /> |
🤖 Prompt for AI Agents
In `@src/components/topbar/CurrentUserButton.vue` around lines 49 - 58, The
current conditional renders CurrentUserPopoverWorkspace only when
teamWorkspacesEnabled && initState === 'ready' and the legacy CurrentUserPopover
only when !teamWorkspacesEnabled, leaving a gap when teamWorkspacesEnabled is
true but initState !== 'ready'; update the logic in CurrentUserButton.vue to
handle the loading case—either render a loading variant or fallback to
CurrentUserPopover while initState is not 'ready'. Concretely, change the
template conditions around CurrentUserPopoverWorkspace and CurrentUserPopover
(references: CurrentUserPopoverWorkspace, CurrentUserPopover,
teamWorkspacesEnabled, initState, closePopover) so that when
teamWorkspacesEnabled is true and initState !== 'ready' you display a visible
fallback (e.g., the legacy CurrentUserPopover or a loading component) instead of
rendering nothing.
| import { storeToRefs } from 'pinia' | ||
| import Popover from 'primevue/popover' | ||
| import { computed, ref } from 'vue' | ||
| import Skeleton from 'primevue/skeleton' |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for existing internal Skeleton component
echo "=== Searching for Skeleton in src/components/ui ==="
fd -t f "Skeleton" src/components/ui 2>/dev/null || echo "No direct Skeleton file found"
echo "=== Searching for Skeleton imports/exports in src/components/ui ==="
rg "Skeleton" src/components/ui --type vue -l 2>/dev/null || echo "No Skeleton references found"
echo "=== Checking the CurrentUserButton.vue file for Skeleton usage ==="
cat -n src/components/topbar/CurrentUserButton.vue | head -80Repository: Comfy-Org/ComfyUI_frontend
Length of output: 3245
🏁 Script executed:
#!/bin/bash
# Search for other Skeleton usage
echo "=== All Skeleton imports in codebase ==="
rg "import.*Skeleton.*from" src/ --type vue
# Search for loading/skeleton patterns
echo -e "\n=== Looking for alternative loading state patterns ==="
rg "loading|skeleton" src/components --type vue -i | head -20
# Check if there are any custom loading components
echo -e "\n=== Custom loading/spinner components ==="
fd -t f "Loading|Spinner|Skeleton" src/components --type fRepository: Comfy-Org/ComfyUI_frontend
Length of output: 360
🏁 Script executed:
#!/bin/bash
cat -n src/components/common/DotSpinner.vue
echo "---"
cat -n src/components/load3d/LoadingOverlay.vueRepository: Comfy-Org/ComfyUI_frontend
Length of output: 6583
Replace PrimeVue Skeleton with a custom skeleton component or alternative loading indicator.
Skeleton from primevue/skeleton violates the guideline to avoid new PrimeVue component usage. No internal skeleton component currently exists in the codebase. Either create a custom skeleton placeholder component matching the 32x32px circular shape, or use DotSpinner from src/components/common/DotSpinner.vue as an alternative loading indicator (note: this changes the visual appearance from a static placeholder to an animated spinner).
🧰 Tools
🪛 ESLint
[error] 66-66: Unable to resolve path to module 'primevue/skeleton'.
(import-x/no-unresolved)
🤖 Prompt for AI Agents
In `@src/components/topbar/CurrentUserButton.vue` at line 66, The PrimeVue
Skeleton import and usage in CurrentUserButton.vue should be removed and
replaced with either a small custom circular placeholder component (e.g., create
CircularSkeleton that renders a 32x32px circular div with neutral background and
optional shimmer) or use the existing DotSpinner from
src/components/common/DotSpinner.vue as the loading indicator; update the import
(remove "primevue/skeleton") and swap any <Skeleton> usage in the
CurrentUserButton component to either <CircularSkeleton /> (if you add it) or
<DotSpinner /> and ensure the element is constrained to 32x32px and styled as
circular to match the expected avatar placeholder.
| const workspaceName = computed(() => { | ||
| if (!showWorkspaceIcon.value) return '' | ||
| return teamWorkspaceName.value | ||
| }) |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Simplify redundant computed property.
The workspaceName computed just returns teamWorkspaceName.value when showWorkspaceIcon is true. Since it's only used in the template when showWorkspaceIcon is already true (via v-else-if), this computed adds unnecessary indirection.
♻️ Suggested simplification
In template, use teamWorkspaceName directly:
<WorkspaceProfilePic
v-else-if="showWorkspaceIcon"
- :workspace-name="workspaceName"
+ :workspace-name="teamWorkspaceName"
:class="compact && 'size-full'"
/>And remove the computed:
-const workspaceName = computed(() => {
- if (!showWorkspaceIcon.value) return ''
- return teamWorkspaceName.value
-})🤖 Prompt for AI Agents
In `@src/components/topbar/CurrentUserButton.vue` around lines 109 - 112, The
computed property workspaceName is redundant because it only returns
teamWorkspaceName.value when showWorkspaceIcon is true and the template already
conditionally renders that branch via v-else-if; remove the workspaceName
computed entirely and update the template to reference teamWorkspaceName
directly in the branch guarded by showWorkspaceIcon (keep showWorkspaceIcon and
teamWorkspaceName as-is and remove all references to workspaceName).
| const handleTopUp = () => { | ||
| // Track purchase credits entry from avatar popover | ||
| useTelemetry()?.trackAddApiCreditButtonClicked() | ||
| dialogService.showTopUpCreditsDialog() | ||
| emit('close') | ||
| } |
There was a problem hiding this comment.
Instantiate telemetry composable at component setup level.
Calling useTelemetry() inside the event handler creates a new instance on each click. Vue composables should be called at the top level of <script setup> to ensure consistent behavior and proper lifecycle management.
♻️ Suggested fix
const subscriptionDialog = useSubscriptionDialog()
+const telemetry = useTelemetry()
const { totalCredits, isLoadingBalance } = useSubscriptionCredits()
// ... later in handleTopUp:
const handleTopUp = () => {
// Track purchase credits entry from avatar popover
- useTelemetry()?.trackAddApiCreditButtonClicked()
+ telemetry?.trackAddApiCreditButtonClicked()
dialogService.showTopUpCreditsDialog()
emit('close')
}🤖 Prompt for AI Agents
In `@src/components/topbar/CurrentUserPopoverWorkspace.vue` around lines 295 -
300, Move the useTelemetry() call out of the click handler and instantiate it
once at the component setup/top-level (e.g., const telemetry = useTelemetry();)
then update handleTopUp to call telemetry.trackAddApiCreditButtonClicked()
instead of calling useTelemetry() inside the function; ensure the import for
useTelemetry is present and the handler (handleTopUp) uses the top-level
telemetry instance so the composable is created only once per component
lifecycle.
| const handleOpenPartnerNodesInfo = () => { | ||
| window.open( | ||
| buildDocsUrl(docsPaths.partnerNodesPricing, { includeLocale: true }), | ||
| '_blank' | ||
| ) | ||
| emit('close') | ||
| } |
There was a problem hiding this comment.
Add security attributes to window.open call.
Opening links with window.open and '_blank' without noopener can expose the opener to potential tabnabbing attacks. The opened page can access window.opener and redirect the original page.
♻️ Suggested fix
const handleOpenPartnerNodesInfo = () => {
- window.open(
- buildDocsUrl(docsPaths.partnerNodesPricing, { includeLocale: true }),
- '_blank'
- )
+ window.open(
+ buildDocsUrl(docsPaths.partnerNodesPricing, { includeLocale: true }),
+ '_blank',
+ 'noopener,noreferrer'
+ )
emit('close')
}🤖 Prompt for AI Agents
In `@src/components/topbar/CurrentUserPopoverWorkspace.vue` around lines 302 -
308, The window.open call in handleOpenPartnerNodesInfo is missing security
attributes and can allow tabnabbing; update the function so the opened window is
created with no opener (e.g., pass the third argument with "noopener,noreferrer"
to window.open or immediately set newWin.opener = null after opening) when
calling buildDocsUrl(docsPaths.partnerNodesPricing...), and then still call
emit('close'). Ensure the change targets handleOpenPartnerNodesInfo so the new
window cannot access window.opener.
| const handleLogout = async () => { | ||
| await handleSignOut() | ||
| emit('close') | ||
| } |
There was a problem hiding this comment.
Add error handling for logout.
If handleSignOut() throws an error, the emit('close') will never execute, potentially leaving the popover open in an inconsistent state. The user won't receive feedback about the failed logout attempt.
♻️ Suggested fix
const handleLogout = async () => {
- await handleSignOut()
- emit('close')
+ try {
+ await handleSignOut()
+ } finally {
+ emit('close')
+ }
}📝 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.
| const handleLogout = async () => { | |
| await handleSignOut() | |
| emit('close') | |
| } | |
| const handleLogout = async () => { | |
| try { | |
| await handleSignOut() | |
| } finally { | |
| emit('close') | |
| } | |
| } |
🤖 Prompt for AI Agents
In `@src/components/topbar/CurrentUserPopoverWorkspace.vue` around lines 310 -
313, The handleLogout function should guard against errors from handleSignOut so
emit('close') always runs and the user gets feedback; wrap the await
handleSignOut() in a try/catch/finally: in try await handleSignOut(), in catch
call the user-facing error notifier (e.g., a toast or process error handler)
with the caught error, and in finally call emit('close') so the popover is
closed regardless of success or failure; update the handleLogout implementation
to reference handleSignOut and emit('close') accordingly.
Backport of #8325 to `cloud/1.37` Automatically created by backport workflow. ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-8327-backport-cloud-1-37-perf-remove-autoplay-from-assets-cards-2f56d73d365081cd8097e7a8619c427f) by [Unito](https://www.unito.io) Co-authored-by: Christian Byrne <cbyrne@comfy.org>
) Backport of #8330 to `cloud/1.37` Automatically created by backport workflow. ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-8331-backport-cloud-1-37-feat-add-Hugging-Face-model-source-support-2f56d73d365081708413c99c384c0806) by [Unito](https://www.unito.io) Co-authored-by: Alexander Brown <drjkl@comfy.org> Co-authored-by: Amp <amp@ampcode.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/platform/assets/utils/assetMetadataUtils.ts`:
- Around line 139-143: getAssetModelType currently returns an empty string when
a matching tag ends with a trailing slash (e.g., "checkpoints/"); update the
function so after computing typeTag.split('/').pop() you treat an empty string
as null and return null instead. Specifically, in getAssetModelType locate the
logic that computes the segment from typeTag, assign the pop result to a
variable, and return that variable if non-empty otherwise return null
(preserving the existing branch that returns typeTag when no '/' is present).
| export function getAssetModelType(asset: AssetItem): string | null { | ||
| const typeTag = asset.tags?.find((tag) => tag && tag !== 'models') | ||
| if (!typeTag) return null | ||
| return typeTag.includes('/') ? (typeTag.split('/').pop() ?? null) : typeTag | ||
| } |
There was a problem hiding this comment.
Minor edge case: trailing slash returns empty string instead of null.
If a tag ends with / (e.g., 'checkpoints/'), split('/').pop() returns '' (empty string), which passes through ?? null. The function would return '' instead of null.
🔧 Suggested fix to handle empty segments
export function getAssetModelType(asset: AssetItem): string | null {
const typeTag = asset.tags?.find((tag) => tag && tag !== 'models')
if (!typeTag) return null
- return typeTag.includes('/') ? (typeTag.split('/').pop() ?? null) : typeTag
+ if (typeTag.includes('/')) {
+ const segment = typeTag.split('/').pop()
+ return segment || null
+ }
+ return typeTag
}🤖 Prompt for AI Agents
In `@src/platform/assets/utils/assetMetadataUtils.ts` around lines 139 - 143,
getAssetModelType currently returns an empty string when a matching tag ends
with a trailing slash (e.g., "checkpoints/"); update the function so after
computing typeTag.split('/').pop() you treat an empty string as null and return
null instead. Specifically, in getAssetModelType locate the logic that computes
the segment from typeTag, assign the pop result to a variable, and return that
variable if non-empty otherwise return null (preserving the existing branch that
returns typeTag when no '/' is present).
…#8346) Backport of #8118 to `cloud/1.37` Automatically created by backport workflow. ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-8346-backport-cloud-1-37-Fix-dragging-Vue-nodes-into-canvas-from-library-2f66d73d3650816b804ae117a4674d53) by [Unito](https://www.unito.io) Co-authored-by: pythongosssss <125205205+pythongosssss@users.noreply.github.com>
## Summary Backport of #8350 to cloud/1.37 - Fix auth related race conditions with a new WorkspaceAuthGate in App.vue - De dup initialization calls - Add state machine to track state of refreshRemoteConfig - Fix websocket not using new workspace jwt - Misc improvements ## Changes Cherry-picked from commit 34fc28a Resolved conflict in `src/views/GraphView.vue`: - Kept workspace store initialization from cloud/1.37 branch - Applied PR's refactored event listener pattern using `useEventListener` from VueUse - Applied PR's `useIntervalFn` for tab count tracking ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-8357-backport-cloud-1-37-Feat-workspaces-5-auth-gate-check-2f66d73d365081bd8fe4f0cea481de11) by [Unito](https://www.unito.io) --------- Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/platform/remoteConfig/remoteConfig.ts (1)
45-52: Use nullish coalescing to preserve falsy config values.Line 51 uses
||which will returndefaultValuefor legitimate falsy values likefalse,0, or''. Use??instead to only fall back onnull/undefined.🐛 Proposed fix
export function configValueOrDefault<K extends keyof RemoteConfig>( remoteConfig: RemoteConfig, key: K, defaultValue: NonNullable<RemoteConfig[K]> ): NonNullable<RemoteConfig[K]> { const configValue = remoteConfig[key] - return configValue || defaultValue + return configValue ?? defaultValue }
🤖 Fix all issues with AI agents
In `@src/components/auth/WorkspaceAuthGate.test.ts`:
- Around line 69-74: Change the test helper from a function expression to a
function declaration: replace the const mountComponent = () => ... with a
declared function named mountComponent() that returns the mounted
WorkspaceAuthGate with the same slots; update any local references to
mountComponent unchanged. This keeps behavior identical but follows the codebase
convention for pure test helpers (refer to the mountComponent helper and the
WorkspaceAuthGate test).
In `@src/components/auth/WorkspaceAuthGate.vue`:
- Around line 1-9: The template uses the PrimeVue ProgressSpinner component
(ProgressSpinner) which the guidelines discourage; replace it with the
project-specific spinner component (e.g., LoadingSpinner or AppSpinner) used
elsewhere in the codebase: update the template to render that component in place
of ProgressSpinner, add the appropriate import/registration inside
WorkspaceAuthGate.vue (or rely on global registration), and ensure props/classes
match the original styling so the fixed-position loading overlay behavior
remains unchanged.
In `@src/platform/remoteConfig/refreshRemoteConfig.ts`:
- Around line 40-45: In refreshRemoteConfig where the code currently logs non-OK
responses (the block with console.warn('Failed to load remote config:',
response.statusText)), ensure all non-OK responses set failure state instead of
only 401/403: clear window.__CONFIG__ and remoteConfig.value and set
remoteConfigState.value = 'error' for any response that is not response.ok (not
just status 401/403) so transient/non-auth errors don't leave the state as
'unloaded'.
| const mountComponent = () => | ||
| mount(WorkspaceAuthGate, { | ||
| slots: { | ||
| default: '<div data-testid="slot-content">App Content</div>' | ||
| } | ||
| }) |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider using function declaration for test helper.
Per codebase conventions, prefer function declarations over function expressions for pure functions. This is a minor style nit.
♻️ Suggested change
- const mountComponent = () =>
- mount(WorkspaceAuthGate, {
+ function mountComponent() {
+ return mount(WorkspaceAuthGate, {
slots: {
default: '<div data-testid="slot-content">App Content</div>'
}
})
+ }📝 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.
| const mountComponent = () => | |
| mount(WorkspaceAuthGate, { | |
| slots: { | |
| default: '<div data-testid="slot-content">App Content</div>' | |
| } | |
| }) | |
| function mountComponent() { | |
| return mount(WorkspaceAuthGate, { | |
| slots: { | |
| default: '<div data-testid="slot-content">App Content</div>' | |
| } | |
| }) | |
| } |
🤖 Prompt for AI Agents
In `@src/components/auth/WorkspaceAuthGate.test.ts` around lines 69 - 74, Change
the test helper from a function expression to a function declaration: replace
the const mountComponent = () => ... with a declared function named
mountComponent() that returns the mounted WorkspaceAuthGate with the same slots;
update any local references to mountComponent unchanged. This keeps behavior
identical but follows the codebase convention for pure test helpers (refer to
the mountComponent helper and the WorkspaceAuthGate test).
| <template> | ||
| <slot v-if="isReady" /> | ||
| <div | ||
| v-else | ||
| class="fixed inset-0 z-[1100] flex items-center justify-center bg-[var(--p-mask-background)]" | ||
| > | ||
| <ProgressSpinner /> | ||
| </div> | ||
| </template> |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Template structure looks good.
The conditional rendering with v-if/v-else for the loading state is clean. One note: the coding guidelines indicate "Avoid new usage of PrimeVue components." Consider using a project-specific spinner component if one exists, or this can be addressed in a follow-up.
🧰 Tools
🪛 ESLint
[error] 1-1: Resolve error: EACCES: permission denied, open '/iESxnOkMEy'
at Object.writeFileSync (node:fs:2409:20)
at l (file:///home/jailuser/git/node_modules/.pnpm/get-tsconfig@4.10.1/node_modules/get-tsconfig/dist/index.mjs:7:13670)
at createFilesMatcher (file:///home/jailuser/git/node_modules/.pnpm/get-tsconfig@4.10.1/node_modules/get-tsconfig/dist/index.mjs:7:14422)
at resolve (file:///home/jailuser/git/node_modules/.pnpm/eslint-import-resolver-typescript@4.4.4_eslint-plugin-import-x@4.16.1_@typescript-eslin_44eddb5b99ae4bce470e6fb9a90221ee/node_modules/eslint-import-resolver-typescript/lib/index.js:70:65)
at Object.resolve (file:///home/jailuser/git/node_modules/.pnpm/eslint-import-resolver-typescript@4.4.4_eslint-plugin-import-x@4.16.1_@typescript-eslin_44eddb5b99ae4bce470e6fb9a90221ee/node_modules/eslint-import-resolver-typescript/lib/index.js:147:20)
at file:///home/jailuser/git/node_modules/.pnpm/eslint-plugin-import-x@4.16.1_@typescript-eslint+utils@8.50.0_eslint@9.39.1_jiti@2.6.1__eacac87f98d760f1781d40e8519857dc/node_modules/eslint-plugin-import-x/lib/utils/resolve.js:170:69
at setRuleContext (/home/jailuser/git/node_modules/.pnpm/eslint-import-context@0.1.9_unrs-resolver@1.11.1/node_modules/eslint-import-context/lib/index.js:23:20)
at fullResolve (file:///home/jailuser/git/node_modules/.pnpm/eslint-plugin-import-x@4.16.1_@typescript-eslint+utils@8.50.0_eslint@9.39.1_jiti@2.6.1__eacac87f98d760f1781d40e8519857dc/node_modules/eslint-plugin-import-x/lib/utils/resolve.js:170:30)
at relative (file:///home/jailuser/git/node_modules/.pnpm/eslint-plugin-import-x@4.16.1_@typescript-eslint+utils@8.50.0_eslint@9.39.1_jiti@2.6.1__eacac87f98d760f1781d40e8519857dc/node_modules/eslint-plugin-import-x/lib/utils/resolve.js:215:12)
at resolve (file:///home/jailuser/git/node_modules/.pnpm/eslint-plugin-import-x@4.16.1_@typescript-eslint+utils@8.50.0_eslint@9.39.1_jiti@2.6.1__eacac87f98d760f1781d40e8519857dc/node_modules/eslint-plugin-import-x/lib/utils/resolve.js:220:16)
at checkSourceValue (file:///home/jailuser/git/node_modules/.pnpm/eslint-plugin-import-x@4.16.1_@typescript-eslint+utils@8.50.0_eslint@9.39.1_jiti@2.6.1__eacac87f98d760f1781d40e8519857dc/node_modules/eslint-plugin-import-x/lib/rules/no-unresolved.js:31:34)
at checkSourceValue (file:///home/jailuser/git/node_modules/.pnpm/eslint-plugin-import-x@4.16.1_@typescript-eslint+utils@8.50.0_eslint@9.39.1_jiti@2.6.1__eacac87f98d760f1781d40e8519857dc/node_modules/eslint-plugin-import-x/lib/utils/module-visitor.js:14:9)
at checkSource (file:///home/jailuser/git/node_modules/.pnpm/eslint-plugin-import-x@4.16.1_@typescript-eslint+utils@8.50.0_eslint@9.39.1_jiti@2.6.1__eacac87f98d760f1781d40e8519857dc/node_modules/eslint-plugin-import-x/lib/utils/module-visitor.js:17:9)
at ruleErrorHandler (/home/jailuser/git/node_modules/.pnpm/eslint@9.39.1_jiti@2.6.1/node_modules/eslint/lib/linter/linter.js:1173:33)
at /home/jailuser/git/node_modules/.pnpm/eslint@9.39.1_jiti@2.6.1/node_modules/eslint/lib/linter/source-code-visitor.js:76:46
at Array.forEach ()
at SourceCodeVisitor.callSync (/home/jailuser/git/node_modules/.pnpm/eslint@9.39.1_jiti@2.6.1/node_modules/eslint/lib/linter/source-code-visitor.js:76:30)
at /home/jailuser/git/node_modules/.pnpm/eslint@9.39.1_jiti@2.6.1/node_modules/eslint/lib/linter/source-code-traverser.js:291:18
at Array.forEach ()
at SourceCodeTraverser.traverseSync (/home/jailuser/git/node_modules/.pnpm/eslint@9.39.1_jiti@2.6.1/node_modules/eslint/lib/linter/source-code-traverser.js:290:10)
at runRules (/home/jailuser/git/node_modules/.pnpm/eslint@9.39.1_jiti@2.6.1/node_modules/eslint/lib/linter/linter.js:1214:12)
at #flatVerifyWithoutProcessors (/home/jailuser/git/node_modules/.pnpm/eslint@9.39.1_jiti@2.6.1/node_modules/eslint/lib/linter/linter.js:2101:4)
at Linter._verifyWithFlatConfigArrayAndWithoutProcessors (/home/jailuser/git/node_modules/.pnpm/eslint@9.39.1_jiti@2.6.1/node_modules/eslint/lib/linter/linter.js:2189:43)
at /home/jailuser/git/node_modules/.pnpm/eslint@9.39.1_jiti@2.6.1/node_modules/eslint/lib/linter/linter.js:1773:17
at Array.map ()
at Linter._verifyWithFlatConfigArrayAndProcessor (/home/jailuser/git/node_modules/.pnpm/eslint@9.39.1_jiti@2.6.1/node_modules/eslint/lib/linter/linter.js:1768:30)
at Linter._verifyWithFlatConfigArray (/home/jailuser/git/node_modules/.pnpm/eslint@9.39.1_jiti@2.6.1/node_modules/eslint/lib/linter/linter.js:2275:16)
at Linter.verify (/home/jailuser/git/node_modules/.pnpm/eslint@9.39.1_jiti@2.6.1/node_modules/eslint/lib/linter/linter.js:1677:10)
at Linter.verifyAndFix (/home/jailuser/git/node_modules/.pnpm/eslint@9.39.1_jiti@2.6.1/node_modules/eslint/lib/linter/linter.js:2557:20)
at verifyText (/home/jailuser/git/node_modules/.pnpm/eslint@9.39.1_jiti@2.6.1/node_modules/eslint/lib/eslint/eslint-helpers.js:1180:45)
at readAndVerifyFile (/home/jailuser/git/node_modules/.pnpm/eslint@9.39.1_jiti@2.6.1/node_modules/eslint/lib/eslint/eslint-helpers.js:1321:10)
(import-x/no-unresolved)
[error] 1-1: Resolve error: EACCES: permission denied, open '/aceOEpVBfv'
at Object.writeFileSync (node:fs:2409:20)
at l (file:///home/jailuser/git/node_modules/.pnpm/get-tsconfig@4.10.1/node_modules/get-tsconfig/dist/index.mjs:7:13670)
at createFilesMatcher (file:///home/jailuser/git/node_modules/.pnpm/get-tsconfig@4.10.1/node_modules/get-tsconfig/dist/index.mjs:7:14422)
at resolve (file:///home/jailuser/git/node_modules/.pnpm/eslint-import-resolver-typescript@4.4.4_eslint-plugin-import-x@4.16.1_@typescript-eslin_44eddb5b99ae4bce470e6fb9a90221ee/node_modules/eslint-import-resolver-typescript/lib/index.js:70:65)
at Object.resolve (file:///home/jailuser/git/node_modules/.pnpm/eslint-import-resolver-typescript@4.4.4_eslint-plugin-import-x@4.16.1_@typescript-eslin_44eddb5b99ae4bce470e6fb9a90221ee/node_modules/eslint-import-resolver-typescript/lib/index.js:147:20)
at file:///home/jailuser/git/node_modules/.pnpm/eslint-plugin-import-x@4.16.1_@typescript-eslint+utils@8.50.0_eslint@9.39.1_jiti@2.6.1__eacac87f98d760f1781d40e8519857dc/node_modules/eslint-plugin-import-x/lib/utils/resolve.js:170:69
at setRuleContext (/home/jailuser/git/node_modules/.pnpm/eslint-import-context@0.1.9_unrs-resolver@1.11.1/node_modules/eslint-import-context/lib/index.js:23:20)
at fullResolve (file:///home/jailuser/git/node_modules/.pnpm/eslint-plugin-import-x@4.16.1_@typescript-eslint+utils@8.50.0_eslint@9.39.1_jiti@2.6.1__eacac87f98d760f1781d40e8519857dc/node_modules/eslint-plugin-import-x/lib/utils/resolve.js:170:30)
at relative (file:///home/jailuser/git/node_modules/.pnpm/eslint-plugin-import-x@4.16.1_@typescript-eslint+utils@8.50.0_eslint@9.39.1_jiti@2.6.1__eacac87f98d760f1781d40e8519857dc/node_modules/eslint-plugin-import-x/lib/utils/resolve.js:215:12)
at resolve (file:///home/jailuser/git/node_modules/.pnpm/eslint-plugin-import-x@4.16.1_@typescript-eslint+utils@8.50.0_eslint@9.39.1_jiti@2.6.1__eacac87f98d760f1781d40e8519857dc/node_modules/eslint-plugin-import-x/lib/utils/resolve.js:220:16)
at importType (file:///home/jailuser/git/node_modules/.pnpm/eslint-plugin-import-x@4.16.1_@typescript-eslint+utils@8.50.0_eslint@9.39.1_jiti@2.6.1__eacac87f98d760f1781d40e8519857dc/node_modules/eslint-plugin-import-x/lib/utils/import-type.js:126:63)
at checkImportForRelativePackage (file:///home/jailuser/git/node_modules/.pnpm/eslint-plugin-import-x@4.16.1_@typescript-eslint+utils@8.50.0_eslint@9.39.1_jiti@2.6.1__eacac87f98d760f1781d40e8519857dc/node_modules/eslint-plugin-import-x/lib/rules/no-relative-packages.js:15:38)
at file:///home/jailuser/git/node_modules/.pnpm/eslint-plugin-import-x@4.16.1_@typescript-eslint+utils@8.50.0_eslint@9.39.1_jiti@2.6.1__eacac87f98d760f1781d40e8519857dc/node_modules/eslint-plugin-import-x/lib/rules/no-relative-packages.js:59:40
at checkSourceValue (file:///home/jailuser/git/node_modules/.pnpm/eslint-plugin-import-x@4.16.1_@typescript-eslint+utils@8.50.0_eslint@9.39.1_jiti@2.6.1__eacac87f98d760f1781d40e8519857dc/node_modules/eslint-plugin-import-x/lib/utils/module-visitor.js:14:9)
at checkSource (file:///home/jailuser/git/node_modules/.pnpm/eslint-plugin-import-x@4.16.1_@typescript-eslint+utils@8.50.0_eslint@9.39.1_jiti@2.6.1__eacac87f98d760f1781d40e8519857dc/node_modules/eslint-plugin-import-x/lib/utils/module-visitor.js:17:9)
at ruleErrorHandler (/home/jailuser/git/node_modules/.pnpm/eslint@9.39.1_jiti@2.6.1/node_modules/eslint/lib/linter/linter.js:1173:33)
at /home/jailuser/git/node_modules/.pnpm/eslint@9.39.1_jiti@2.6.1/node_modules/eslint/lib/linter/source-code-visitor.js:76:46
at Array.forEach ()
at SourceCodeVisitor.callSync (/home/jailuser/git/node_modules/.pnpm/eslint@9.39.1_jiti@2.6.1/node_modules/eslint/lib/linter/source-code-visitor.js:76:30)
at /home/jailuser/git/node_modules/.pnpm/eslint@9.39.1_jiti@2.6.1/node_modules/eslint/lib/linter/source-code-traverser.js:291:18
at Array.forEach ()
at SourceCodeTraverser.traverseSync (/home/jailuser/git/node_modules/.pnpm/eslint@9.39.1_jiti@2.6.1/node_modules/eslint/lib/linter/source-code-traverser.js:290:10)
at runRules (/home/jailuser/git/node_modules/.pnpm/eslint@9.39.1_jiti@2.6.1/node_modules/eslint/lib/linter/linter.js:1214:12)
at #flatVerifyWithoutProcessors (/home/jailuser/git/node_modules/.pnpm/eslint@9.39.1_jiti@2.6.1/node_modules/eslint/lib/linter/linter.js:2101:4)
at Linter._verifyWithFlatConfigArrayAndWithoutProcessors (/home/jailuser/git/node_modules/.pnpm/eslint@9.39.1_jiti@2.6.1/node_modules/eslint/lib/linter/linter.js:2189:43)
at /home/jailuser/git/node_modules/.pnpm/eslint@9.39.1_jiti@2.6.1/node_modules/eslint/lib/linter/linter.js:1773:17
at Array.map ()
at Linter._verifyWithFlatConfigArrayAndProcessor (/home/jailuser/git/node_modules/.pnpm/eslint@9.39.1_jiti@2.6.1/node_modules/eslint/lib/linter/linter.js:1768:30)
at Linter._verifyWithFlatConfigArray (/home/jailuser/git/node_modules/.pnpm/eslint@9.39.1_jiti@2.6.1/node_modules/eslint/lib/linter/linter.js:2275:16)
at Linter.verify (/home/jailuser/git/node_modules/.pnpm/eslint@9.39.1_jiti@2.6.1/node_modules/eslint/lib/linter/linter.js:1677:10)
at Linter.verifyAndFix (/home/jailuser/git/node_modules/.pnpm/eslint@9.39.1_jiti@2.6.1/node_modules/eslint/lib/linter/linter.js:2557:20)
at verifyText (/home/jailuser/git/node_modules/.pnpm/eslint@9.39.1_jiti@2.6.1/node_modules/eslint/lib/eslint/eslint-helpers.js:1180:45)
at readAndVerifyFile (/home/jailuser/git/node_modules/.pnpm/eslint@9.39.1_jiti@2.6.1/node_modules/eslint/lib/eslint/eslint-helpers.js:1321:10)
(import-x/no-relative-packages)
🤖 Prompt for AI Agents
In `@src/components/auth/WorkspaceAuthGate.vue` around lines 1 - 9, The template
uses the PrimeVue ProgressSpinner component (ProgressSpinner) which the
guidelines discourage; replace it with the project-specific spinner component
(e.g., LoadingSpinner or AppSpinner) used elsewhere in the codebase: update the
template to render that component in place of ProgressSpinner, add the
appropriate import/registration inside WorkspaceAuthGate.vue (or rely on global
registration), and ensure props/classes match the original styling so the
fixed-position loading overlay behavior remains unchanged.
| console.warn('Failed to load remote config:', response.statusText) | ||
| if (response.status === 401 || response.status === 403) { | ||
| window.__CONFIG__ = {} | ||
| remoteConfig.value = {} | ||
| remoteConfigState.value = 'error' | ||
| } |
There was a problem hiding this comment.
Non-401/403 error responses don't update state.
When the response is not OK but also not 401/403 (e.g., 500 Server Error), the function logs a warning but leaves remoteConfigState unchanged. This could leave state as 'unloaded' indefinitely. Consider setting remoteConfigState.value = 'error' for all non-OK responses to provide consistent failure visibility.
🔧 Proposed fix
console.warn('Failed to load remote config:', response.statusText)
- if (response.status === 401 || response.status === 403) {
- window.__CONFIG__ = {}
- remoteConfig.value = {}
- remoteConfigState.value = 'error'
- }
+ window.__CONFIG__ = {}
+ remoteConfig.value = {}
+ remoteConfigState.value = 'error'Alternatively, if preserving partial state is intentional for transient errors, document that behavior.
🤖 Prompt for AI Agents
In `@src/platform/remoteConfig/refreshRemoteConfig.ts` around lines 40 - 45, In
refreshRemoteConfig where the code currently logs non-OK responses (the block
with console.warn('Failed to load remote config:', response.statusText)), ensure
all non-OK responses set failure state instead of only 401/403: clear
window.__CONFIG__ and remoteConfig.value and set remoteConfigState.value =
'error' for any response that is not response.ok (not just status 401/403) so
transient/non-auth errors don't leave the state as 'unloaded'.
…r proper re-login hand… (#8388) Backport of #8381 to `cloud/1.37` Automatically created by backport workflow. ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-8388-backport-cloud-1-37-fix-move-WorkspaceAuthGate-to-LayoutDefault-for-proper-re-login-ha-2f76d73d3650813ea84ed3fdf537986a) by [Unito](https://www.unito.io) Co-authored-by: Simula_r <18093452+simula-r@users.noreply.github.com> Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/extensions/core/cloudRemoteConfig.ts`:
- Around line 31-32: The polling setInterval unconditionally calls
refreshRemoteConfig which treats 401/403 as failures and clears config after
logout; change the interval handler to first check current auth state and only
call refreshRemoteConfig with auth when logged in, or when logged out call
refreshRemoteConfig({ useAuth: false }) so it won't treat 401/403 as an error
and clear config—modify the setInterval callback that references
refreshRemoteConfig to perform this guard and avoid clearing config on logout.
| // Poll for config updates every 10 minutes (with auth) | ||
| setInterval(() => void refreshRemoteConfig(), 600_000) |
There was a problem hiding this comment.
Guard authenticated polling when logged out to avoid clearing config.
refreshRemoteConfig treats 401/403 as error and clears config; the interval runs unconditionally, so after logout it can repeatedly drop config and set an error state. Gate the call on auth (or pass { useAuth: false } when logged out).
✅ Suggested fix (auth guard)
- setInterval(() => void refreshRemoteConfig(), 600_000)
+ setInterval(() => {
+ if (!isLoggedIn.value) return
+ void refreshRemoteConfig()
+ }, 600_000)🤖 Prompt for AI Agents
In `@src/extensions/core/cloudRemoteConfig.ts` around lines 31 - 32, The polling
setInterval unconditionally calls refreshRemoteConfig which treats 401/403 as
failures and clears config after logout; change the interval handler to first
check current auth state and only call refreshRemoteConfig with auth when logged
in, or when logged out call refreshRemoteConfig({ useAuth: false }) so it won't
treat 401/403 as an error and clear config—modify the setInterval callback that
references refreshRemoteConfig to perform this guard and avoid clearing config
on logout.
Backport of #8286 to `cloud/1.37` Automatically created by backport workflow. ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-8397-backport-cloud-1-37-Templates-Search-speed-2f76d73d3650810c96fdd00691f9b2f7) by [Unito](https://www.unito.io) Co-authored-by: Alexander Brown <drjkl@comfy.org>
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/composables/useTemplateFiltering.ts (1)
162-170: Handle empty array edge case inMath.max.When
templatesis empty,Math.max()returns-Infinity, which could cause unexpected behavior incomputeDefaultScoreorcomputePopularScorecalculations that uselargestUsageScore.🐛 Proposed fix
watch( filteredByRunsOn, (templates) => { - rankingStore.largestUsageScore = Math.max( - ...templates.map((t) => t.usage || 0) - ) + const usageValues = templates.map((t) => t.usage || 0) + rankingStore.largestUsageScore = + usageValues.length > 0 ? Math.max(...usageValues) : 0 }, { immediate: true } )
🤖 Fix all issues with AI agents
In `@src/composables/useTemplateFiltering.ts`:
- Line 87: Rename the misleading variable debouncedSearchQuery to
throttledSearchQuery to match the use of refThrottled; update its declaration
(where refThrottled(searchQuery, 50) is assigned) and all references to that
variable (e.g., the uses currently pointing to debouncedSearchQuery) so the code
consistently uses throttledSearchQuery instead.
| }) | ||
|
|
||
| const debouncedSearchQuery = refDebounced(searchQuery, 50) | ||
| const debouncedSearchQuery = refThrottled(searchQuery, 50) |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Rename variable to reflect throttle behavior.
The variable debouncedSearchQuery now uses refThrottled instead of refDebounced, making the name misleading. Rename to throttledSearchQuery for consistency with the actual implementation.
♻️ Proposed fix
- const debouncedSearchQuery = refThrottled(searchQuery, 50)
+ const throttledSearchQuery = refThrottled(searchQuery, 50)Also update the references at lines 90 and 94:
- if (!debouncedSearchQuery.value.trim()) {
+ if (!throttledSearchQuery.value.trim()) {- const results = fuse.value.search(debouncedSearchQuery.value)
+ const results = fuse.value.search(throttledSearchQuery.value)🤖 Prompt for AI Agents
In `@src/composables/useTemplateFiltering.ts` at line 87, Rename the misleading
variable debouncedSearchQuery to throttledSearchQuery to match the use of
refThrottled; update its declaration (where refThrottled(searchQuery, 50) is
assigned) and all references to that variable (e.g., the uses currently pointing
to debouncedSearchQuery) so the code consistently uses throttledSearchQuery
instead.
6 participants
PR to enable a live preview of the 1.37 cloud release branch.
┆Issue is synchronized with this Notion page by Unito