1.27.6 #5800
Closed
1.27.6 #5800
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
* lint: turn on type import rules setting up for verbatimModuleSyntax * lint: --fix for type imports
* [ci] ignore playwright mcp directory * [feat] add AssetBrowserModal And all related sub components * [feat] reactive filter functions * [ci] clean up storybook config * [feat] add sematic AssetCard * [fix] i love lucide * [fix] AssetCard layout issues * [fix] add AssetBadge type * [fix] simplify useAssetBrowser * [fix] modal layout * [fix] simplify useAssetBrowserDialog * [fix] add tailwind back to storybook * [fix] better reponsive layout * [fix] missed i18n string * [fix] missing i18n translations * [fix] remove erroneous prevent on keyboard.space * [feat] add asset metadata validation utilities * [fix] remove erroneous test code * [fix] remove forced min and max width on AssetCard * [fix] import statement nits
### Summary Adds desktop dialog framework with data-driven dialog definitions. ### Changes - Data-driven dialog structure in `desktopDialogs.ts` - Dynamic dialog view component with i18n support - Button action types: openUrl, close, cancel - Button severity levels for styling (primary, secondary, danger, warn) - Fallback invalid dialog for error handling - i18n collection script updated for dialog strings
This pull request improves the selection toolbox behavior during node dragging by ensuring that it correctly responds to both LiteGraph and Vue node drag events. The main changes introduce a reactive drag state for Vue nodes in the layout store and update the selection toolbox composable and Vue node component to use this state. **Selection toolbox behavior improvements:** * Added a helper function and separate watchers in `useSelectionToolboxPosition.ts` to hide the selection toolbox when either LiteGraph or Vue nodes are being dragged. This ensures consistent UI feedback regardless of node type. [[1]](diffhunk://#diff-57a51ac5e656e64ae7fd276d71b115058631621755de33b1eb8e8a4731d48713L171-R172) [[2]](diffhunk://#diff-57a51ac5e656e64ae7fd276d71b115058631621755de33b1eb8e8a4731d48713R212-R224) **Vue node drag state management:** * Added a reactive `isDraggingVueNodes` property to the `LayoutStoreImpl` class, along with getter and setter methods to manage Vue node drag state. This allows other components to reactively track when Vue nodes are being dragged. [[1]](diffhunk://#diff-80d32fe0fb72730c16cf7259adef8b20732ff214df240b1d39ae516737beaf3bR133-R135) [[2]](diffhunk://#diff-80d32fe0fb72730c16cf7259adef8b20732ff214df240b1d39ae516737beaf3bR354-R367) * Updated `LGraphNode.vue` to set and clear the Vue node dragging state in the layout store during pointer down and up events, ensuring the selection toolbox is hidden while dragging Vue nodes. [[1]](diffhunk://#diff-a7744614cf842e54416047326db79ad81f7c7ab7bfb66ae2b46f5c73ac7d47f2R357-R360) [[2]](diffhunk://#diff-a7744614cf842e54416047326db79ad81f7c7ab7bfb66ae2b46f5c73ac7d47f2R376-R378) **Dependency updates:** * Imported the `layoutStore` in `LGraphNode.vue` to access the new drag state management methods. * Added missing `ref` import in `layoutStore.ts` to support the new reactive property. https://github.com/user-attachments/assets/d6e9c15e-63b5-4de2-9688-ebbc6a3be545 --------- Co-authored-by: GitHub Action <[email protected]>
Currently, we set persistence method in the auth store setup. This creates pattern of using the default on init (indexed DB) up until the firebase store is initialized and `setPersistence` is called. For devices that don't support indexed DB or have the connection aggresively terminated or cleared, like [Safari](https://comfy-org.sentry.io/issues/6879071102/?project=4509681221369857&query=is%3Aunresolved&referrer=issue-stream), this can create problems with maintaing auth persistence. Fix by setting persistence method in the initialization in main.ts ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-5614-Move-VueFire-persistence-configuration-to-initialization-2716d73d3650817480e0c8feb1f37b9a) by [Unito](https://www.unito.io) --------- Co-authored-by: Claude <[email protected]>
This pull request improves the lazy loading behavior and caching
strategy for images in the `LazyImage.vue` component. The most
significant changes are focused on optimizing image rendering and
resource management, as well as improving code clarity.
**Lazy loading behavior improvements:**
* Changed the `<img>` element to render only when `cachedSrc` is
available, ensuring that images are not displayed before they are ready.
* Updated watchers in `LazyImage.vue` to use clearer variable names
(`shouldLoadVal` instead of `shouldLoad`) for better readability and
maintainability.
[[1]](diffhunk://#diff-3a1bfa7eb8cb26b04bea73f7b4b4e3c01e9d20a7eba6c3738fb47f96da1a7c95L80-R81)
[[2]](diffhunk://#diff-3a1bfa7eb8cb26b04bea73f7b4b4e3c01e9d20a7eba6c3738fb47f96da1a7c95L96-R96)
**Caching strategy enhancement:**
* Modified the `fetch` call in `mediaCacheService.ts` to use `{ cache:
'force-cache' }`, which leverages the browser's cache more aggressively
when loading media, potentially improving performance and reducing
network requests.
┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-5626-LazyImage-on-Safari-2716d73d365081eeb1d3c2a96be4d408)
by [Unito](https://www.unito.io)
## Summary See https://typescript-eslint.io/blog/project-service/ for context. Creates a browser_tests specific tsconfig so that they can be linted. Does not add a package.json script to do the linting yet, but `pnpm exec eslint browser_tests` should work for now. ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-5633-lint-add-tsconfig-for-browser_tests-fix-existing-violations-2726d73d3650819d8ef2c4b0abc31e14) by [Unito](https://www.unito.io)
…ger (#5635) ## Summary - Updated frontend to align with backend changes in ComfyUI core PR #7555 - Changed manager startup argument from `--disable-manager` (opt-out) to `--enable-manager` (opt-in) - Manager is now disabled by default unless explicitly enabled ## Changes - Modified `useManagerState.ts` to check for `--enable-manager` flag presence - Inverted logic: manager is disabled when the flag is NOT present - Updated all related tests to reflect the new opt-in behavior - Fixed edge case where `systemStats` is null ## Related - Backend PR: comfyanonymous/ComfyUI#7555 ## Test Plan - [x] All unit tests pass - [x] Verified manager state logic with different flag combinations - [x] TypeScript type checking passes - [x] Linting passes 🤖 Generated with [Claude Code](https://claude.ai/code) ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-5635-refactor-Change-manager-flag-from-disable-manager-to-enable-manager-2726d73d36508153a88bd9f152132b2a) by [Unito](https://www.unito.io)
## Summary Added comprehensive component test suite for WidgetImageCompare widget with 410 test assertions covering display, edge cases, and integration scenarios. ## Changes - **What**: Created [Vue Test Utils](https://vue-test-utils.vuejs.org/) test suite for [WidgetImageCompare component](src/renderer/extensions/vueNodes/widgets/components/WidgetImageCompare.vue) using [Vitest](https://vitest.dev/) testing framework ## Review Focus Test coverage completeness for string vs object value handling, accessibility attribute propagation, and edge case robustness including malformed URLs and empty states. ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-5549-test-Add-component-test-for-image-compare-widget-26e6d73d365081189fe0d010f87d1eec) by [Unito](https://www.unito.io) --------- Co-authored-by: DrJKL <[email protected]>
## Summary Some users were authenticating successfully but their email addresses weren't being extracted from the Firebase token. This happened because we weren't explicitly requesting the email scope during OAuth authentication. While Firebase's default configuration includes basic profile info, it doesn't guarantee email access for all account types - particularly Google Workspace accounts with restrictive policies or users with privacy-conscious settings. [Github Scopes](https://docs.github.com/en/apps/oauth-apps/building-oauth-apps/scopes-for-oauth-apps) ## Changes Adding email scope for Google + Github social OAuth. ## Review Focus N/A ## Screenshots (if applicable) ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-5638-Explicitly-add-email-scope-for-social-auth-login-2726d73d3650817ab356fc9c04f8641b) by [Unito](https://www.unito.io) --------- Co-authored-by: Alexander Brown <[email protected]>
Minor version increment to 1.28.0 ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-5640-1-28-0-2726d73d3650818e846fcf78cbf33b73) by [Unito](https://www.unito.io) Co-authored-by: benceruleanlu <[email protected]>
## Summary Integrated Vue node components with canvas panning mode to prevent UI interference during navigation. ## Changes - **What**: Added [canCapturePointerEvents](https://docs.comfy.org/guide/vue-nodes) computed property to `useCanvasInteractions` composable that checks canvas read-only state - **What**: Modified Vue node components (LGraphNode, NodeWidgets) to conditionally handle pointer events based on canvas navigation mode - **What**: Updated node event handlers to respect panning mode and forward events to canvas when appropriate ## Review Focus Event forwarding logic in panning mode and pointer event capture state management across Vue node hierarchy. ```mermaid graph TD A[User Interaction] --> B{Canvas in Panning Mode?} B -->|Yes| C[Forward to Canvas] B -->|No| D[Handle in Vue Component] C --> E[Canvas Navigation] D --> F[Node Selection/Widget Interaction] G[canCapturePointerEvents] --> H{read_only === false} H -->|Yes| I[Allow Vue Events] H -->|No| J[Block Vue Events] style A fill:#f9f9f9,stroke:#333,color:#000 style E fill:#f9f9f9,stroke:#333,color:#000 style F fill:#f9f9f9,stroke:#333,color:#000 style I fill:#e1f5fe,stroke:#01579b,color:#000 style J fill:#ffebee,stroke:#c62828,color:#000 ``` ## Screenshots https://github.com/user-attachments/assets/00dc5e4a-2b56-43be-b92e-eaf511e52542 ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-5574-Make-Vue-nodes-read-only-when-in-panning-mode-26f6d73d3650818c951cd82c8fe58972) by [Unito](https://www.unito.io) --------- Co-authored-by: GitHub Action <[email protected]>
#5024 added support for connecting primitive nodes to subgraph inputs. To accomplish this, it pulls WidgetLocator information from the node owning the widget. This `node` property does not exist on all IBaseWidget. `toConcrete` was used to instead have a BaseWidget which is guaranteed to have a node property. The issue that was missed, is that a widget which lacks this information (such as most implemented by custom nodes) sets the node value to the argument which was passed. Here that is the reference to the subgraph node. Sometimes, this `#setWidget` call is made multiple times, and when this occurs, the `input.widget` has itself set as the protoyep, throwing an error. This is resolved by instead taking an additional input which is unambiguous. For reference, this is a near minimal workflow using comfy_mtb that replicates the issue [cyclic.json](https://github.com/user-attachments/files/22412187/cyclic.json) Special thanks to @melMass for assistance discovering this issue. ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-5637-Fix-cyclic-prototype-errors-with-subgraphNodes-2726d73d365081fea356f5197e4c2b42) by [Unito](https://www.unito.io)
Implementing subgraph blueprints (#5139) included changes to saving to ensure that SaveAs generates a new workflow of the correct type. However this code failed to utilize the pre-prepared state when performing the actual save. This produced a couple of problems with both failing to detach the workflow and failing to apply the correct state This error is only encountered when using Save As from a non temporary workflow (one loaded from the workflows sidebar tab). As this state calculation code is only used in this code path, it has been moved into the saveAs function of the workflowStore. Resolves #5592 ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-5643-Fix-SaveAs-2726d73d3650818faa7af449d1f13c26) by [Unito](https://www.unito.io)
…led (#5647) If manager is disabled, it assumed all missing nodes are installed and immediately closes the missing nodes warning when loading a workflow with missing nodes. ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-5647-fix-don-t-immediately-close-missing-nodes-dialog-if-manager-is-disabled-2736d73d36508199a50bca2026528ab6) by [Unito](https://www.unito.io)
Allows for simple slot functionality in vue nodes mode. Has: - Drag new link from slot - Connect new link from dropping on slot Now: - Tests After: - Drop on reroute - Correct link color on connect - Drop on node - Hover effects ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-5628-Slot-functionality-for-vue-nodes-2716d73d365081c59a3cef7c8a5e539e) by [Unito](https://www.unito.io) --------- Co-authored-by: bymyself <[email protected]> Co-authored-by: AustinMroz <[email protected]> Co-authored-by: Claude <[email protected]>
## Summary - Added complete Turkish language translation for ComfyUI Frontend - Integrated Turkish locale into the i18n system - Added Turkish as a selectable language option in settings ## Implementation Details - Added Turkish translation files provided by @naxci1: - `src/locales/tr/main.json` - Main UI translations - `src/locales/tr/commands.json` - Command translations - `src/locales/tr/nodeDefs.json` - Node definitions translations - `src/locales/tr/settings.json` - Settings translations - Updated `src/i18n.ts` to import and register Turkish locale - Added Turkish option to language selector in `src/constants/coreSettings.ts` ## Test Plan - [ ] Verify Turkish translations load correctly - [ ] Test language switching to/from Turkish - [ ] Check all UI elements display properly in Turkish - [ ] Verify node descriptions and tooltips in Turkish - [ ] Test command palette in Turkish Fixes #5437 🤖 Generated with [Claude Code](https://claude.ai/code) ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-5438-feat-Add-Turkish-language-support-2686d73d36508184bbf2dc1e0cd15350) by [Unito](https://www.unito.io)
Enables manual backport triggering for scenarios where labels are added after PR merge. Adds workflow_dispatch trigger to the backport workflow with support for: - Specifying PR number to backport post-merge - Force rerun option to override duplicate detection - Proper handling of multi-version backport scenarios Solves the issue where adding version labels (e.g., 1.27) after a PR is already merged and backported (e.g., to 1.26) would not trigger additional backports. ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-5651-feat-add-manual-dispatch-to-backport-workflow-2736d73d365081b6ba00c7a43c9ba06b) by [Unito](https://www.unito.io)
## Summary - Enable `verbatimModuleSyntax` compiler option in TypeScript configuration - Update all type imports to use explicit `import type` syntax - This change will Improve tree-shaking and bundler compatibility ## Motivation The `verbatimModuleSyntax` option ensures that type-only imports are explicitly marked with the `type` keyword. This: - Makes import/export intentions clearer - Improves tree-shaking by helping bundlers identify what can be safely removed - Ensures better compatibility with modern bundlers - Follows TypeScript best practices for module syntax ## Changes - Added `"verbatimModuleSyntax": true` to `tsconfig.json` - Updated another 48+ files to use explicit `import type` syntax for type-only imports - No functional changes, only import/export syntax improvements ## Test Plan - [x] TypeScript compilation passes - [x] Build completes successfully - [x] Tests pass - [ ] No runtime behavior changes 🤖 Generated with [Claude Code](https://claude.ai/code) ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-5533-feat-enable-verbatimModuleSyntax-in-TypeScript-config-26d6d73d36508190b424ef9b379b5130) by [Unito](https://www.unito.io)
…rison (#5654) ## Summary - Fixes the Claude automated PR review comparing against wrong commits - Updates the comprehensive-pr-review.md command to use `$BASE_SHA` instead of `origin/$BASE_BRANCH` - Resolves issue where Claude was reviewing unrelated changes from other PRs ## Problem As identified in #5651 (comment #5651 (comment)), the Claude automated review was incorrectly analyzing changes that weren't part of the PR being reviewed. The review was mentioning Turkish language removal, linkRenderer changes, and other modifications that weren't in the actual PR diff. ## Root Cause Analysis ### The Issue Explained (from Discord discussion) When Christian Byrne noticed Claude was referencing things from previous reviews on other PRs, we investigated and found: 1. **The backport branch was created from origin/main BEFORE Turkish language support was merged** - Branch state: `main.A` - Backport changes committed: `main.A.Backport` 2. **Turkish language support was then merged into origin/main** - Main branch updated to: `main.A.Turkish` 3. **Claude review workflow checked out `main.A.Backport` and ran git diff against `origin/main`** - This compared: `main.A.Backport <> main.A.Turkish` - The diff showed: `+++Backport` changes and `---Turkish` removal - Because the common parent of both branches was `main.A` ### Why This Happens When using `origin/$BASE_BRANCH`, git resolves to the latest commit on that branch. The diff includes: 1. The PR's actual changes (+++Backport) 2. The reverse of all commits merged to main since the PR was created (---Turkish) This causes Claude to review changes that appear as "removals" of code from other merged PRs, leading to confusing comments about unrelated code. ## Solution Changed the git diff commands to use `$BASE_SHA` directly, which GitHub Actions provides as the exact commit SHA that represents the merge base. This ensures Claude only reviews the actual changes introduced by the PR. ### Before (incorrect): ```bash git diff --name-only "origin/$BASE_BRANCH" # Compares against latest main git diff "origin/$BASE_BRANCH" git diff --name-status "origin/$BASE_BRANCH" ``` ### After (correct): ```bash git diff --name-only "$BASE_SHA" # Compares against merge base git diff "$BASE_SHA" git diff --name-status "$BASE_SHA" ``` ## Technical Details ### GitHub Actions Environment Variables - `BASE_SHA`: The commit SHA of the merge base (where PR branched from main) - `BASE_BRANCH`: Not provided by GitHub Actions (this was the bug) - Using `origin/$BASE_BRANCH` was falling back to comparing against the latest main commit ### Alternative Approaches Considered 1. **Approach 1**: Rebase/update branch before running Claude review - Downside: Changes the PR's commits, not always desirable 2. **Approach 2**: Use BASE_SHA to diff against the merge base ✅ - This is what GitHub's PR diff view does - Shows only the changes introduced by the PR ## Testing The BASE_SHA environment variable is already correctly set in the claude-pr-review.yml workflow (line 88), so this change will work immediately once merged. ## Impact - Claude reviews will now be accurate and only analyze the actual PR changes - No false positives about "removed" code from other PRs - More reliable automated PR review process - Developers won't be confused by comments about code they didn't change ## Verification You can verify this fix by: 1. Creating a PR from an older branch 2. Merging another PR to main 3. Triggering Claude review with the label 4. Claude should only review the PR's changes, not show removals from the newly merged commits ## Credits Thanks to @christian-byrne for reporting the issue and @snomiao for the root cause analysis. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-authored-by: Claude <[email protected]>
## Summary
- Fixes Sentry issue CLOUD-FRONTEND-STAGING-29 (TypeError: nodes is not
iterable)
- Adds defensive guard to check if nodes is valid array before iteration
- Gracefully handles malformed workflow data by skipping node processing
## Root Cause
The `collectMissingNodesAndModels` function in `src/scripts/app.ts:1135`
was attempting to iterate over `nodes` without checking if it was a
valid iterable, causing crashes when workflow data was malformed or
missing the nodes property.
## Fix
Added null/undefined/array validation before the for-loop:
```typescript
if (\!nodes || \!Array.isArray(nodes)) {
console.warn('Workflow nodes data is missing or invalid, skipping node processing', { nodes, path })
return
}
```
Fixes CLOUD-FRONTEND-STAGING-29
┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-5660-fix-TypeError-nodes-is-not-iterable-when-loading-graph-2736d73d365081cfb828d27e59a4811c)
by [Unito](https://www.unito.io)
## Summary Fix TypeError in NodeTooltip component when `nodeDef` is undefined. This occurs when hovering over nodes whose type is not found in the nodeDefStore. ## Changes - Add optional chaining (`?.`) to `nodeDef.description` access on line 71 - Follows the same defensive pattern used in previous fixes for similar issues ## Context This addresses Sentry issue [CLOUD-FRONTEND-STAGING-1B](https://comfy-org.sentry.io/issues/6829258525/) which shows 19 occurrences affecting 14 users. The fix follows the same pattern as previous commits: - [290bf52](290bf52) - Fixed similar issue on line 112 - [e8997a7](e8997a7) - Fixed multiple similar issues ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-5659-fix-prevent-TypeError-when-nodeDef-is-undefined-in-NodeTooltip-2736d73d3650816e8be3f44889198b58) by [Unito](https://www.unito.io)
## Summary Reorganized custom nodes manager functionality from scattered technical layers into a cohesive domain-focused module following [domain-driven design](https://en.wikipedia.org/wiki/Domain-driven_design) principles. ## Changes - **What**: Migrated all manager code from technical layers (`src/components/`, `src/stores/`, etc.) to unified domain structure at `src/workbench/extensions/manager/` - **Breaking**: Import paths changed for all manager-related modules (40+ files updated) ## Review Focus Verify all import path updates are correct and no circular dependencies introduced. Check that [Vue 3 composition API](https://vuejs.org/guide/reusability/composables.html) patterns remain consistent across relocated composables. ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-5662-refactor-Migrate-manager-code-to-DDD-structure-2736d73d3650812c87faf6ed0fffb196) by [Unito](https://www.unito.io)
## Summary Added comprehensive component tests for FormSelectButton widget with 497 test cases covering all interaction patterns and edge cases. ## Changes - **What**: Created test suite for [FormSelectButton.vue](https://vuejs.org/guide/scaling-up/testing.html) component with full coverage of string/number/object options, PrimeVue compatibility, disabled states, and visual styling - **Dependencies**: No new dependencies (uses existing vitest, @vue/test-utils) ## Review Focus Test completeness covering edge cases like unicode characters, duplicate values, and objects with missing properties. Verify test helper functions correctly simulate user interactions. ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-5576-Add-Vue-FormSelectButton-widget-component-tests-26f6d73d36508171ae08ee74d0605db2) by [Unito](https://www.unito.io) --------- Co-authored-by: DrJKL <[email protected]>
## Summary Extension of #5659: Added optional chaining to NodeTooltip component to prevent TypeError when `nodeDef` is undefined for unknown node types. ## Changes - **What**: Added [optional chaining operator](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/Optional_chaining) (`?.`) to safely access `nodeDef` properties in NodeTooltip component ## Review Focus Error handling for node types not found in nodeDefStore and tooltip display behavior for unrecognized nodes. Fixes CLOUD-FRONTEND-STAGING-3N Co-authored-by: Claude <[email protected]>
## Summary Added tooltip support for Vue node components using PrimeVue's v-tooltip directive with proper data integration and container scoping. https://github.com/user-attachments/assets/d1af31e6-ef6a-4df8-8de4-5098aa4490a1 ## Changes - **What**: Implemented tooltip functionality for Vue node headers, input/output slots, and widgets using [PrimeVue v-tooltip](https://primevue.org/tooltip/) directive - **Dependencies**: Leverages existing PrimeVue tooltip system, no new dependencies ## Review Focus Container scoping implementation via provide/inject pattern for tooltip positioning, proper TypeScript interfaces eliminating `as any` casts, and integration with existing settings store for tooltip delays and enable/disable functionality. ```mermaid graph TD A[LGraphNode Container] --> B[provide tooltipContainer] B --> C[NodeHeader inject] B --> D[InputSlot inject] B --> E[OutputSlot inject] B --> F[NodeWidgets inject] G[useNodeTooltips composable] --> H[NodeDefStore lookup] G --> I[Settings integration] G --> J[i18n fallback] C --> G D --> G E --> G F --> G style A fill:#f9f9f9,stroke:#333,color:#000 style G fill:#e8f4fd,stroke:#0066cc,color:#000 ``` --------- Co-authored-by: DrJKL <[email protected]> Co-authored-by: GitHub Action <[email protected]>
## Summary Removed the informational "Use Legacy UI" tag from the ManagerHeader component while preserving all underlying legacy manager functionality. ## Changes - **What**: Removed Tag component displaying legacy UI information from ManagerHeader - **Breaking**: None - all legacy manager functionality remains intact - **Dependencies**: None ## Review Focus Visual cleanup only - the `--enable-manager-legacy-ui` CLI flag and all related functionality continues to work normally. Only the informational UI tag has been removed from the header.
…emver package (#5653) ## Summary - Replace custom `compareVersions()` with `semver.compare()` - Replace custom `isSemVer()` with `semver.valid()` - Remove deprecated version comparison functions from `formatUtil.ts` - Update all version comparison logic across components and stores - Fix tests to use semver mocking instead of formatUtil mocking ## Benefits - **Industry standard**: Uses well-maintained, battle-tested `semver` package - **Better reliability**: Handles edge cases more robustly than custom implementation - **Consistent behavior**: All version comparisons now use the same underlying logic - **Type safety**: Better TypeScript support with proper semver types Fixes #4787 ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-5653-refactor-Replace-manual-semantic-version-utilities-functions-with-semver-package-2736d73d365081fb8498ee11cbcc10e2) by [Unito](https://www.unito.io) --------- Co-authored-by: DrJKL <[email protected]> Co-authored-by: Claude <[email protected]>
Tests added - Should show a link dragging out from a slot when dragging on a slot - Should create a link when dropping on a compatible slot - Should not create a link when dropping on an incompatible slot(s) ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-5668-Add-playwright-tests-for-links-and-slots-in-vue-nodes-mode-2736d73d36508188a47dceee5d1a11e5) by [Unito](https://www.unito.io) --------- Co-authored-by: github-actions <[email protected]>
This pull request refactors the minimap rendering system to use a unified, extensible data source abstraction for all minimap operations. By introducing a data source interface and factory, the minimap can now seamlessly support multiple sources of node layout (such as the `LayoutStore` or the underlying `LiteGraph`), improving maintainability and future extensibility. Rendering logic and change detection throughout the minimap have been updated to use this new abstraction, resulting in cleaner code and easier support for new data models. **Core architecture improvements:** * Introduced a new `IMinimapDataSource` interface and related data types (`MinimapNodeData`, `MinimapLinkData`, `MinimapGroupData`) to standardize node, link, and group data for minimap rendering. * Added an abstract base class `AbstractMinimapDataSource` that provides shared logic for bounds and group/link extraction, and implemented two concrete data sources: `LiteGraphDataSource` (for classic graph data) and `LayoutStoreDataSource` (for layout store data). [[1]](diffhunk://#diff-ea46218fc9ffced84168a5ff975e4a30e43f7bf134ee8f02ed2eae66efbb729dR1-R95) [[2]](diffhunk://#diff-9a6b7c6be25b4dbeb358fea18f3a21e78797058ccc86c818ed1e5f69c7355273R1-R30) [[3]](diffhunk://#diff-f200ba9495a03157198abff808ed6c3761746071404a52adbad98f6a9d01249bR1-R42) * Created a `MinimapDataSourceFactory` that selects the appropriate data source based on the presence of layout store data, enabling seamless switching between data models. **Minimap rendering and logic refactoring:** * Updated all minimap rendering functions (`renderGroups`, `renderNodes`, `renderConnections`) and the main `renderMinimapToCanvas` entry point to use the unified data source interface, significantly simplifying the rendering code and decoupling it from the underlying graph structure. [[1]](diffhunk://#diff-3670f99330b2e24aca3cffeeac6600adf8abadd6dd585f596d60fde1dd093121L1-R11) [[2]](diffhunk://#diff-3670f99330b2e24aca3cffeeac6600adf8abadd6dd585f596d60fde1dd093121R33-R75) [[3]](diffhunk://#diff-3670f99330b2e24aca3cffeeac6600adf8abadd6dd585f596d60fde1dd093121L66-R124) [[4]](diffhunk://#diff-3670f99330b2e24aca3cffeeac6600adf8abadd6dd585f596d60fde1dd093121L134-R161) [[5]](diffhunk://#diff-3670f99330b2e24aca3cffeeac6600adf8abadd6dd585f596d60fde1dd093121L153-R187) [[6]](diffhunk://#diff-3670f99330b2e24aca3cffeeac6600adf8abadd6dd585f596d60fde1dd093121L187-L188) [[7]](diffhunk://#diff-3670f99330b2e24aca3cffeeac6600adf8abadd6dd585f596d60fde1dd093121R227-R231) [[8]](diffhunk://#diff-3670f99330b2e24aca3cffeeac6600adf8abadd6dd585f596d60fde1dd093121L230-R248) * Refactored minimap viewport and graph change detection logic to use the data source abstraction for bounds, node, and link change detection, and to respond to layout store version changes. [[1]](diffhunk://#diff-d92e448dee5e30782a66b9e66d8c8b05626dffd0b2ff1032f2612b9a9b9c51f6L2-R10) [[2]](diffhunk://#diff-d92e448dee5e30782a66b9e66d8c8b05626dffd0b2ff1032f2612b9a9b9c51f6R33-R35) [[3]](diffhunk://#diff-d92e448dee5e30782a66b9e66d8c8b05626dffd0b2ff1032f2612b9a9b9c51f6L99-R141) [[4]](diffhunk://#diff-d92e448dee5e30782a66b9e66d8c8b05626dffd0b2ff1032f2612b9a9b9c51f6R157-R160) [[5]](diffhunk://#diff-338d14c67dabffaf6f68fbf09b16e8d67bead2b9df340e46601b2fbd57331521L8-R11) [[6]](diffhunk://#diff-338d14c67dabffaf6f68fbf09b16e8d67bead2b9df340e46601b2fbd57331521L56-R64) These changes make the minimap codebase more modular and robust, and lay the groundwork for supporting additional node layout strategies in the future. ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-5547-Layoutstore-Minimap-calculation-26e6d73d3650813e9457c051dff41ca1) by [Unito](https://www.unito.io)
## Summary Reduce the number of DOM changes, but also restructure the logic to create a clean point of separation for when we can make DragAndScale reactive. ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-5767-Refactor-Viewport-Culling-improvements-2796d73d365081708f02cb8033aac9b1) by [Unito](https://www.unito.io)
## Summary Added mute state support to Vue nodes with visual feedback and keyboard shortcut functionality. ## Changes - **What**: Implemented mute state (mode 2) for Vue nodes with opacity styling and `Ctrl+M` hotkey support ## Review Focus Visual consistency between bypass and mute states, and keyboard shortcut conflict detection with existing hotkeys. ## Test Coverage - Single node mute/unmute with `Ctrl+M` hotkey - Multi-selection mute/unmute operations - Visual state verification with opacity changes ## Related - #5715 ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-5770-Add-muted-state-to-Vue-nodes-2796d73d36508143b3edfbcb782de7c1) by [Unito](https://www.unito.io)
## Summary Improve markdown node by making the textarea match the rendered output width and height. ## Screenshots (if applicable) https://github.com/user-attachments/assets/4701f947-0a4f-40f3-83c0-94e53cd10106 ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-5771-Fix-vue-nodes-markdown-2796d73d365081129428fe9c38178cc4) by [Unito](https://www.unito.io)
## Summary Split tailwind utils out to a shared package within the monorepo. ## Changes - Creates `@comfyorg/tailwind-utils` package - Does not require export, publishing, etc - Uses `export` to ensure this change does not impact other PRs (many imports to update) - If we _want_ to update all imports, there are two commits ready to be re-applied - e.g. `git revert 80960c2` ## Review Focus - Is this pattern desirable? - Should we just include this in a broader design-system split? I kind of vote yes, but also it's a good small, first step. ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-5777-Split-Tailwind-utility-functions-out-to-a-shared-package-2796d73d3650815f976fc73b4fb86ef3) by [Unito](https://www.unito.io)
…e node number widgets (#5776) ## Summary Makes the [useGrouping](https://primevue.org/inputnumber/#api.inputnumber.props.useGrouping) prop for number widgets disabled by default, aligning with the old UI (also requested via design). Node authors can still enable if they want by setting prop explicitly. ## Changes - **What**: Modified [WidgetInputNumberInput](https://primevue.org/inputnumber/) to disable `useGrouping` by default, requiring explicit opt-in via widget options - **Testing**: Added component tests covering value binding, component rendering, step calculations, and grouping behavior ## Review Focus UX impact on existing nodes that may have relied on default grouping behavior and test coverage for edge cases with precision calculations. ## Screenshots (if applicable) *Before*: <img width="1685" height="879" alt="Screenshot from 2025-09-25 11-34-34" src="https://github.com/user-attachments/assets/432097ab-203d-4f86-8ca0-721b27ee33de" /> *After*: <img width="1951" height="1175" alt="Screenshot from 2025-09-25 11-35-27" src="https://github.com/user-attachments/assets/74d35b62-612e-4dbf-b6e2-0ac17af03ea1" /> ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-5776-Disable-number-grouping-thousands-comma-separators-by-default-in-Vue-node-number-widget-2796d73d365081369ca6c155335d0d57) by [Unito](https://www.unito.io) --------- Co-authored-by: DrJKL <[email protected]> Co-authored-by: Alexander Brown <[email protected]> Co-authored-by: github-actions <[email protected]>
Adds light theme colors from the [designs](https://www.figma.com/design/31uH3r4x3xbIctuRWYW6NM/V3---Nodes?node-id=6267-16837&m=dev). ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-5786-Add-theme-aware-styling-for-Vue-node-tooltips-27a6d73d36508167979fe797efcbe3c0) by [Unito](https://www.unito.io)
…to names) (#5773) ## Summary Fixes #5697 by adding internationalization support for Vue node widget labels and output slot names with fallback to original names. ## Changes - **What**: Added `label` field to widget data structures and `localized_name` support for output slots - **Breaking**: None - all changes maintain backward compatibility with fallback to original names ## Review Focus Translation key resolution for node definitions and proper fallback chain for widget labels and slot names. ## Screenshots (if applicable) Observe that there is parity between Litegraph and Vue nodes w.r.t. localization of labels/names. <img width="3455" height="1417" alt="Screenshot from 2025-09-25 13-19-54" src="https://github.com/user-attachments/assets/93ee8fae-3671-4175-a941-6462f942d0f1" /> <img width="3455" height="1417" alt="Screenshot from 2025-09-25 13-19-38" src="https://github.com/user-attachments/assets/9573eb86-d74a-40ed-a0b5-e30afd3da6eb" /> ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-5773-Use-localized-labels-for-Vue-node-slots-and-inputs-outputs-fallback-to-names-2796d73d365081e08fb7d38464f4aa90) by [Unito](https://www.unito.io) --------- Co-authored-by: github-actions <[email protected]> Co-authored-by: Alexander Brown <[email protected]>
…ching between workflow tabs) (#5788) ## Summary Fixed Vue node output restoration by consolidating state management through the node output store. ## Changes - **What**: Refactored node output cleanup and restoration logic in `ChangeTracker` and `ComfyApp` to use centralized store methods - **Breaking**: Removed direct manipulation of `app.nodeOutputs` in favor of store-managed state ## Review Focus State synchronization between `app.nodeOutputs` and `nodeOutputs.value` during restore/reset operations, ensuring Vue reactivity is maintained. ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-5788-fix-restoring-outputs-in-Vue-nodes-persisting-node-outputs-when-switching-between-workfl-27a6d73d365081b39411fed7479d8ac5) by [Unito](https://www.unito.io)
## Refactor conflict detection system and move to manager extension ### Description This PR refactors the conflict detection system, moving it from the global composables to the manager extension folder for better code organization. Additionally, it improves test type safety and adds comprehensive test coverage for utility functions. ### Main Changes #### 📦 Code Organization - **Moved conflict detection to manager extension** - Relocated all conflict detection related composables, stores, and utilities from global scope to `/workbench/extensions/manager/` for better modularity (#5722) - **Moved from** `src/composables/useConflictDetection.ts` **to** `src/workbench/extensions/manager/composables/useConflictDetection.ts` - Moved related stores and composables to maintain cohesive module structure #### ♻️ Refactoring - **Extracted utility functions** - Split conflict detection logic into separate utility modules: - `conflictUtils.ts` - Conflict consolidation and summary generation - `systemCompatibility.ts` - OS and accelerator compatibility checking - `versionUtil.ts` - Version compatibility checking - **Removed duplicate state management** - Cleaned up redundant state and unused functions - **Improved naming conventions** - Renamed functions for better clarity - **Removed unused system environment code** - Cleaned up deprecated code #### 🔧 Test Improvements - **Fixed TypeScript errors** in all test files - removed all `any` type usage - **Added comprehensive test coverage**: - `conflictUtils.test.ts` - 299 lines of tests for conflict utilities - `systemCompatibility.test.ts` - 270 lines of tests for compatibility checking - `versionUtil.test.ts` - 342 lines of tests for version utilities - **Updated mock objects** to match actual implementations - **Aligned with backend changes** - Updated SystemStats structure to include `pytorch_version`, `embedded_python`, `required_frontend_version` #### 🐛 Bug Fixes - **Fixed OS detection bug** - Resolved issue where 'darwin' was incorrectly matched as 'Windows' due to containing 'win' substring - **Fixed import paths** - Updated all import paths after moving to manager extension - **Fixed unused exports** - Removed all unused function exports - **Fixed lint errors** - Resolved all ESLint and Prettier issues ### File Structure Changes ``` Before: src/ ├── composables/ │ └── useConflictDetection.ts (1374 lines) └── types/ After: src/ ├── utils/ │ ├── conflictUtils.ts (114 lines) │ ├── systemCompatibility.ts (125 lines) │ └── versionUtil.ts (enhanced) └── workbench/extensions/manager/ ├── composables/ │ ├── useConflictDetection.ts (758 lines) │ └── [other composables] └── stores/ └── conflictDetectionStore.ts ``` ### Testing All tests pass successfully: - ✅ **155 test files passed** - ✅ **2209 tests passed** - ⏩ 19 skipped (intentionally skipped subgraph-related tests) ### Impact - **Better code organization** - Manager-specific code is now properly isolated - **Improved maintainability** - Smaller, focused utility functions are easier to test and maintain - **Enhanced type safety** - No more `any` types in tests - **Comprehensive test coverage** - All utility functions are thoroughly tested ### Commits in this PR 1. OS detection bug fix and refactor 2. Remove unused system environment code 3. Improve function naming 4. Refactor conflict detection 5. Remove duplicate state and unused functions 6. Fix unused function exports 7. Move manager features to workbench extension folder 8. Fix import paths 9. Rename systemCompatibility file 10. Improve test type safety 11. Apply ESLint and Prettier fixes ## Screenshots (if applicable) [screen-capture.webm](https://github.com/user-attachments/assets/b4595604-3761-4d98-ae8e-5693cd0c95bd) ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-5436-Manager-refactor-conflict-detect-2686d73d36508186ba06f57dae3656e5) by [Unito](https://www.unito.io) --------- Co-authored-by: Claude <[email protected]>
This pull request updates event handling in the selection toolbox components to ensure that mouse wheel events are properly forwarded to the canvas, improving interaction consistency across the UI. **Event handling improvements:** * Changed the wheel event handler in `SelectionToolbox.vue` to use `canvasInteractions.forwardEventToCanvas` instead of `canvasInteractions.handleWheel`, ensuring wheel events are consistently forwarded to the canvas. * Added the wheel event handler to the `MoreOptions.vue` popover, forwarding wheel events to the canvas for submenu interactions. **Code organization updates:** * Imported `useCanvasInteractions` in `MoreOptions.vue` to access the new event forwarding method. * Instantiated `canvasInteractions` in `MoreOptions.vue` for use in event handling.## Summary https://github.com/user-attachments/assets/46046086-35e8-4cd1-a11a-365705beb9cd ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-5781-Wheel-Selection-Toolbox-and-Popover-27a6d73d3650812c9c4fe8440ff7dd1d) by [Unito](https://www.unito.io)
Adds a wait to ensure the dismissed state is saved to localstorage properly before `setup` (page reload). ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-5792-fix-broken-test-27a6d73d365081e69c1febbc246448fa) by [Unito](https://www.unito.io)
… browser context (#5775) ## The Problem The `collect-i18n-node-defs.ts` script started failing ~3 weeks ago when Vue nodes were introduced ([commit 006e6bd](006e6bd57), [PR #4263](#4263)). The issue stems from: 1. **Import chain bringing Vue components into Node.js context:** ``` collect-i18n-node-defs.ts ↓ imports ComfyNodeDefImpl (from nodeDefStore.ts) ↓ imports useSubgraphStore (from subgraphStore.ts) ↓ transitively imports executionStore.ts ↓ imports ChatHistoryWidget.vue (Vue component!) ``` 2. **TypeScript `declare` fields causing Babel errors:** ``` TypeScript 'declare' fields must first be transformed by @babel/plugin-transform-typescript ``` ## This Solution vs PR #5515 ### PR #5515 Approach (Complex) - Adds custom Babel plugins and configurations - Implements automatic browser globals injection - Requires **47,517 additions, 9,469 deletions** - Modifies the entire Playwright babel transformation pipeline ### This PR's Approach (Simple) - Uses dynamic imports to defer module loading until runtime - Avoids Babel compilation of problematic TypeScript/Vue files - **Only 40 lines changed** in a single file - No configuration changes needed ## How This Fix Works ```typescript // Instead of static import that Babel tries to compile: // import { ComfyNodeDefImpl } from '../src/stores/nodeDefStore' // We use: // 1. Type-only import (erased at runtime) import type { ComfyNodeDefImpl } from '../src/stores/nodeDefStore' // 2. Dynamic import at runtime (bypasses Babel) const { ComfyNodeDefImpl: ComfyNodeDefImplClass } = await import( '../src/stores/nodeDefStore' ) ``` --------- Co-authored-by: github-actions <[email protected]>
dosubot
bot
added
the
size:XS
🎨 Storybook Build Status✅ Build completed successfully! ⏰ Completed at: 09/26/2025, 12:42:26 PM UTC 🔗 Links🎉 Your Storybook is ready for review! |
🎭 Playwright Test Results⏰ Completed at: 09/26/2025, 12:52:40 PM UTC 📈 Summary
📊 Test Reports by Browser
🎉 Click on the links above to view detailed test results for each browser configuration. |
AustinMroz
requested review from
a team,
KarryCharon,
Yorha4D,
brucew4yn3rp,
jtydhr88,
shinshin86 and
trsommer
as code owners
dosubot
bot
added
size:L
|
Minor misunderstanding on my part. Choosing to run the version bump workflow from the core/1.27 branch created a PR that correctly set version, but was onto main. Since it included all the current 1.28 commits, simply changing the target branch was insufficient. I'm instead performing the release manually with #5801 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
14 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Patch version increment to 1.27.6