mirror of
https://github.com/Comfy-Org/ComfyUI_frontend.git
synced 2026-07-15 19:54:32 +00:00
8e0970e6d37bc371c19de32bbd466ec4ebd48b8e
1195 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
dante01yoon
|
8e0970e6d3 | Merge branch 'main' into jaewon/fe-729-delete-is-asset-api-enabled | ||
|
jaeone94
|
78a8d6f8fc |
test: stabilize node help locale e2e (#12998)
## Summary
Stabilizes the locale-specific Node Help E2E by setting the locale
through the existing Playwright settings fixture before app bootstrap
instead of racing a workflow reload pulse.
## Changes
- **What**: Removed the brittle in-page locale mutation helper and uses
`test.use({ initialSettings: { 'Comfy.Locale': 'ja' } })` for the
locale-specific documentation case.
- **What**: Keeps the Japanese and English doc routes local to the test,
then verifies the Japanese help content after loading the default
workflow.
- **Dependencies**: None.
## Review Focus
Please focus on whether the E2E now waits on the correct setup boundary.
The previous helper watched `ChangeTracker.isLoadingGraph` after
changing `Comfy.Locale`; once unrelated workflow-load work became
faster, that loading pulse could complete before the helper observed it.
Pre-boot `initialSettings` avoids that timing dependency and uses
existing test infrastructure.
Verification:
- `pnpm exec oxfmt --check browser_tests/tests/nodeHelp.spec.ts
browser_tests/fixtures/helpers/WorkflowHelper.ts`
- `pnpm exec eslint browser_tests/tests/nodeHelp.spec.ts
browser_tests/fixtures/helpers/WorkflowHelper.ts`
- `pnpm typecheck:browser`
- `PLAYWRIGHT_LOCAL=1 PLAYWRIGHT_TEST_URL=http://localhost:5176 pnpm
exec playwright test browser_tests/tests/nodeHelp.spec.ts --grep "Should
handle locale-specific documentation" --project=chromium
--repeat-each=10`
## Screenshots (if applicable)
N/A
|
||
|
jaeone94
|
cc41e3e1ac |
test: stabilize cloud template filtering e2e (#12999)
## Summary Stabilizes the cloud template filtering E2E by making the test own both startup asset API responses and the complete template universe it asserts against. ## Changes - **What**: Uses the existing `createCloudAssetsFixture([])` fixture so startup `/api/assets` calls do not create an unrelated error toast that can intercept the Clear Filters click. - **What**: Extends `TemplateHelper.mockIndex()` to also mock `/api/workflow_templates` as an empty custom-template map, so tests that configure a core template index do not accidentally include custom templates from the local backend. - **Dependencies**: None. ## Review Focus Please focus on fixture ownership. This spec asserts exact template counts, so `templateApi.mockIndex()` should isolate the core template index and the custom workflow-template endpoint together. The asset fixture change is intentionally scoped to this cloud spec and reuses existing infrastructure instead of dismissing arbitrary toasts or forcing clicks. Verification: - `pnpm exec oxfmt --check browser_tests/tests/templateFilteringCount.spec.ts browser_tests/fixtures/helpers/TemplateHelper.ts browser_tests/fixtures/helpers/WorkflowHelper.ts` - `pnpm exec eslint browser_tests/tests/templateFilteringCount.spec.ts browser_tests/fixtures/helpers/TemplateHelper.ts browser_tests/fixtures/helpers/WorkflowHelper.ts` - `pnpm typecheck:browser` - `PLAYWRIGHT_LOCAL=1 PLAYWRIGHT_TEST_URL=http://localhost:8188 PLAYWRIGHT_SETUP_API_URL=http://localhost:8188 pnpm exec playwright test browser_tests/tests/templateFilteringCount.spec.ts --grep "clear filters button resets" --project=cloud --repeat-each=5` - `PLAYWRIGHT_LOCAL=1 PLAYWRIGHT_TEST_URL=http://localhost:8188 PLAYWRIGHT_SETUP_API_URL=http://localhost:8188 pnpm exec playwright test browser_tests/tests/templateFilteringCount.spec.ts --project=cloud` ## Screenshots (if applicable) N/A |
||
|
jaeone94
|
444dc3fccd |
test: stabilize mask editor screenshot e2e (#13011)
<img width="1155" height="648" alt="스크린샷 2026-06-19 오후 11 24 27" src="https://github.com/user-attachments/assets/01ed2607-662f-4735-b0c2-2f1a2c8a8811" /> ## Summary Stabilizes the Mask Editor screenshot E2E by hiding the transient brush cursor before capturing the dialog. ## Changes - **What**: Moves the pointer from the mask editor pointer zone to the Brush Settings panel before the screenshot and asserts that the brush cursor is hidden. - **Dependencies**: None. ## Review Focus Please check that the test still exercises the dialog UI while excluding only cursor-position noise from the screenshot. The PR also includes the existing browser-test `AppMode` type import fix needed for `typecheck:browser` on branches that touch `browser_tests/**`. Validation: - `pnpm exec oxfmt --check browser_tests/tests/maskEditor.spec.ts browser_tests/fixtures/helpers/WorkflowHelper.ts` - `pnpm exec eslint browser_tests/tests/maskEditor.spec.ts browser_tests/fixtures/helpers/WorkflowHelper.ts` - `pnpm typecheck:browser` - `PLAYWRIGHT_LOCAL=1 PLAYWRIGHT_TEST_URL=http://localhost:5173 pnpm exec playwright test browser_tests/tests/maskEditor.spec.ts --grep "opens mask editor from image preview button" --project=chromium --repeat-each=10` --------- Co-authored-by: github-actions <github-actions@github.com> |
||
|
jaeone94
|
ed028a88be |
Fix LiteGraph hidden widget metadata handling (FE-1014) (#12916)
## Summary
Fixes FE-1014 by making legacy LiteGraph honor backend-provided hidden
widget metadata.
This PR adds a regression test for the Painter node and updates
LiteGraph widget construction so that a backend input spec with `hidden:
true` is mirrored onto the top-level `widget.hidden` property that the
legacy canvas renderer actually reads.
## Problem
Backend node definitions can mark inputs as hidden, for example with
`extra_dict={"hidden": True}`. That metadata already flows into the
frontend widget options as `widget.options.hidden`, which is why Vue
nodes correctly hide those fields.
Legacy LiteGraph, however, does not use `widget.options.hidden` for
canvas visibility. Its rendering, layout, and hit-testing paths check
top-level `widget.hidden` instead. As a result, a field could be hidden
in Vue nodes while still appearing as an editable control in the legacy
LiteGraph canvas.
For affected nodes, this exposes fields that are intended to be
implementation details, schema/version values, or other non-user-facing
inputs.
## Root Cause
The frontend widget construction path copied backend display metadata
into `widget.options`, including:
- `advanced`
- `hidden`
But it did not mirror backend `hidden` metadata into `widget.hidden`.
That created a renderer split:
- Vue nodes and the right panel use `widget.options.hidden`.
- Legacy LiteGraph uses top-level `widget.hidden`.
So backend-hidden widgets were hidden in Vue mode but still visible and
clickable in legacy LiteGraph mode.
## Implementation
The production change is intentionally small and scoped to
backend-provided hidden metadata:
- Continue assigning `inputSpec.hidden` to `widget.options.hidden` as
before.
- When `inputSpec.hidden` is explicitly defined, also assign it to
top-level `widget.hidden`.
This keeps Vue behavior unchanged while making the legacy LiteGraph
renderer receive the same backend hidden signal through the field it
already uses for visibility.
The fix deliberately does not mirror `advanced` into top-level
`widget.advanced`. While investigating this area, I found that many
backend inputs define `advanced`, and changing legacy advanced-widget
behavior would be a much broader behavioral change than FE-1014
requires. This PR only addresses hidden metadata.
## Test Coverage
This PR adds and tightens Painter regression coverage because Painter
currently provides a concrete backend-hidden widget case:
- In Vue mode, the test verifies hidden Painter widgets are not rendered
to the user.
- In legacy LiteGraph mode, the test disables Vue nodes, loads the
Painter workflow, clicks the rows where backend-hidden number widgets
used to be exposed, and verifies the legacy graph editor dialog does not
open.
The legacy test specifically covers the backend-hidden number widgets
`width` and `height`. It uses user-observable behavior rather than
asserting internal widget flags directly.
A follow-up discussion is ongoing about the broader contract between
`widget.options.hidden` and top-level `widget.hidden`, especially for
frontend-extension-only hiding such as Painter `bg_color`. This PR
intentionally keeps that broader renderer-contract question out of scope
and focuses on backend `hidden` metadata from FE-1014.
## Validation
Validated locally with targeted Playwright coverage:
```bash
PLAYWRIGHT_LOCAL=1 PLAYWRIGHT_TEST_URL=http://localhost:5173 pnpm exec playwright test browser_tests/tests/painter.spec.ts --project=chromium -g "Does not render hidden standard widgets|Does not open editors for backend-hidden number widget rows"
```
Result:
```text
2 passed
```
Also validated with linting:
```bash
pnpm eslint src/services/litegraphService.ts browser_tests/tests/painter.spec.ts
pnpm eslint browser_tests/tests/painter.spec.ts
```
The commit hooks also passed:
- `oxfmt`
- `oxlint`
- `eslint`
- `pnpm typecheck`
- `pnpm typecheck:browser`
## Notes
The new legacy test was confirmed red before the production fix and
green after the production fix, so it is not a vacuous assertion. The
final cleanup commit only tightens test naming and coordinate handling
while preserving the same regression intent.
|
||
|
imick-io
|
fc4d44c3db |
[feat] migrate website navbar to shadcn-vue + mobile sheet drill-down (#12861)
## Summary - Migrate the website's top nav from bespoke components (`SiteNav`, `MobileMenu`, `NavDesktopLink`, `PillButton`, `MaskRevealButton`) to shadcn-vue primitives (`NavigationMenu`, `Sheet`, `Button`), split into `HeaderMain` → `HeaderMainDesktop` + `HeaderMainMobile`. - Mobile nav becomes a `Sheet` with drill-down sub-navigation, scroll lock, sticky CTAs, sr-only i18n title/description, and a back-to-home logo. - Desktop nav uses `NavigationMenu` with shared viewport, featured cards, `NavColumn` extraction, and centralized nav data in `data/mainNavigation.ts`. - Adds i18n strings for nav labels, dropdown column headers, close/back affordances, and the mobile menu description. ## Test plan - [ ] Desktop: hover PRODUCTS / COMMUNITY / COMPANY — dropdowns open with featured card + columns, NEW badges render, external links show the arrow-up-right icon, viewport is shared between triggers. - [ ] Mobile (<lg): open hamburger sheet — verify logo + close, body scroll is locked while open, top-level nav scrolls if it overflows, CTAs stay pinned at bottom. - [ ] Tap COMMUNITY / PRODUCTS / COMPANY — sub-panel slides over root nav, in-sheet BACK returns to root, links navigate correctly. - [ ] Reload `?locale=zh-CN`: dropdown labels, sheet title/description, BACK / close affordances all localized. - [ ] No regressions on `/cloud`, `/cloud/pricing`, `/customers`, etc. — pages that swap CTAs (`BrandButton` → shadcn `Button`) still render at every breakpoint. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Co-authored-by: GitHub Action <action@github.com> Co-authored-by: github-actions <github-actions@github.com> Co-authored-by: Alexander Brown <drjkl@comfy.org> |
||
|
AustinMroz
|
7f25d28b71 |
Filter canvasOnly non-preview widgets in editor (#12957)
Non preview, `canvasOnly` widgets like `control_after_generate` could be displayed in the subgraph editor even though promoting them would have no visual or functional effect when in vue mode. In vue mode, these entries are now hidden from the list of candidate items for promotion to reduce confusion. |
||
|
Dante
|
67b884d0f7 |
fix(billing): route subscription/sign-in/credit preconditions to modal, out of error panel (FE-878) (#12785)
## Summary
Account preconditions (sign-in / subscription / credits) on running a
workflow now open their modal directly and stay out of the error panel +
error count — previously `subscription_required` fell through to a red
"1 ERROR — Subscription required to queue workflows" banner. This covers
**both** paths: the `execution_error` websocket event and the `POST
/prompt` 402 queue paywall (`{ type: "PAYMENT_REQUIRED", message:
"Subscription required to queue workflows" }`), which is the exact
payload reported in #12840.
## Changes
- **What**: `execution_error` is classified by a pure
`accountPreconditionRouting` resolver (precedence sign-in > subscription
> credits) and routed to the existing modal via
`useAccountPreconditionDialog`; `executionStore` returns early for
preconditions so they never populate `lastExecutionError` /
`lastPromptError` / `lastNodeErrors` → fully excluded from the panel and
`totalErrorCount`. Runtime credit error at a node → credits modal (out
of panel; can name the node).
- **Queue paywall**: the `queuePrompt` catch resolves the same
precondition from the `POST /prompt` 402 response and opens the modal,
short-circuiting before `lastPromptError`, so the queue paywall stays
out of the panel too. The runtime matcher learns the `"Subscription
required to queue workflows"` message.
- **Breaking**: none.
## Before / After
Free-tier queue paywall (`POST /prompt` → 402) on a cloud build:
**Before** — raw `Subscription required to queue workflows` surfaced in
the error panel (no actionable upgrade):
<img width="1600" height="873" alt="before-error-panel"
src="https://github.com/user-attachments/assets/1b76b742-16bf-47e3-9245-17e35f8f1e70"
/>
**After** — clean subscription modal opens; nothing in the error panel
or error count:
<img width="1600" height="873" alt="after-subscription-modal"
src="https://github.com/user-attachments/assets/13d238cb-20bf-4795-a530-5abcf9968dc7"
/>
## Review Focus
- **Routing-only — the run button is intentionally untouched.** The
original AC#3 ("no Subscribe-to-Run button") is superseded by the FE-978
run-lock decision (pre-emptive role-aware lock, Figma 3253-18671).
Complements #12786 (FE-978 run-lock); disjoint file sets.
- Tests: `accountPreconditionRouting` / `useAccountPreconditionDialog` /
`executionStore` — each precondition routes to its modal and is excluded
from the panel/count; precedence resolves on co-occurrence. Plus
Playwright `browser_tests/tests/subscriptionPaywallError.spec.ts` — the
queue paywall (402) stays out of the error panel, with a control
asserting ordinary queue errors still surface. typecheck / oxlint /
eslint / stylelint / oxfmt / knip clean.
Fixes FE-878
Fixes #12840
|
||
|
github-actions
|
4066b2aa61 | [automated] Update test expectations | ||
|
Dante
|
b0e0423c1b | Merge branch 'main' into jaewon/fe-729-delete-is-asset-api-enabled | ||
|
Alexander Brown
|
ab6c44aabf |
feat: remove deprecated group nodes, auto-convert to subgraphs on load (#12931)
## Summary Removes the deprecated Group Nodes feature and replaces it with a load-time migration that auto-converts any group nodes in a loaded workflow into Subgraphs (with accepted lossiness). ## Changes - **What**: - `groupNode.ts` is now a migration-only extension. `beforeConfigureGraph` registers temporary node types from `extra.groupNodes` so instances are created during `configure`; a new `afterConfigureGraph` hook converts every group node in the root graph to a subgraph (via `LGraph.convertToSubgraph`), re-scanning until none remain, then deletes `extra.groupNodes`. A failed conversion removes the offending node so loading never hangs or breaks. - Kept the minimum needed: `GroupNodeConfig` (builds the input/output/widget maps), a slimmed `GroupNodeHandler` exposing a rewritten `convertToNodes()` that no longer depends on the execution DTOs, the `globalDefs`/`addCustomNodeDefs` path, and the `nodeDefStore` `Object.assign` shim the migration relies on to detect group nodes. - Deleted: the Manage Group Nodes dialog (`groupNodeManage.ts`/`.css`), execution DTOs (`executableGroupNodeDto.ts`, `executableGroupNodeChildDTO.ts`), the create/builder flow, recreate, commands, keybindings, menus, the `isGroupNode` branches in the right-side panel / error grouping / focus composable, the group-node branches in node templates, dead i18n keys, and the now-unused `serialise` clipboard helper. - Rewrote `browser_tests/tests/groupNode.spec.ts` to assert auto-conversion; deleted the `ManageGroupNode` page object and `manageGroupNode()` helper. - Net: ~2,700 lines removed across 23 files (7 files deleted). - **Breaking**: Group nodes can no longer be created, managed, or executed. Existing workflows still load — their group nodes are converted to subgraphs on open. ## Review Focus - The load-time migration in `afterConfigureGraph` and the rewritten `GroupNodeHandler.convertToNodes()` (no longer uses the execution `getInnerNodes()` / DTOs; derives inner node type/index from `groupData.nodeData.nodes` and relies on `deserialiseAndCreate` + selection ordering). - Kept `nodeDefStore`'s `Object.assign(this, obj)` shim: the migration depends on it to propagate the group-node marker symbol onto the registered node definition. ### Accepted lossiness - Group nodes nested inside subgraphs (or inside other group nodes) convert into the root graph rather than their original container — essentially nonexistent in real legacy workflows since group nodes predate subgraphs. - Temporary `workflow>name` node types stay registered for the session; instantiating one auto-converts it to a subgraph. ## Verification `pnpm typecheck`, `typecheck:browser`, `knip`, `oxlint`, `eslint`, and `oxfmt` are green (also enforced by pre-commit hooks). Unit tests for the touched files could not be run locally due to a pre-existing environment error (`file:///assets/images/*.svg` passed to a Node filename API at import time, which also fails on unmodified test files); the browser spec requires a live server. --------- Co-authored-by: Amp <amp@ampcode.com> Co-authored-by: github-actions <github-actions@github.com> Co-authored-by: AustinMroz <austin@comfy.org> Co-authored-by: GitHub Action <action@github.com> |
||
|
Dante
|
2cdaead000 |
fix(cloud): stop bouncing working users to /cloud/survey mid-session (FE-739) (#12621)
## Summary
Cloud users get yanked to `/cloud/survey` mid-workflow with no user
action. The redirect is **downstream of auth**: when the Cloud token is
briefly stale (token rotation / auth-refresh / reconnect window), the
authenticated survey-status check 401s, and the gate turned that
transient 401 into "survey not completed" → redirect.
Surveys are currently disabled for everyone on cloud via dynamicconfig
as the live mitigation. This PR lets us re-enable them safely
**without** waiting on the auth rework.
## Root cause
`getSurveyCompletedStatus()` returned `false` ("not completed") on
**any** non-200 — including a transient 401/403/5xx or network error —
and consumers treat `false` as a redirect to the survey. So a
stale-token 401 (or the page force-reload a 401 triggers in
`GraphCanvas`) bounced a working, already-onboarded user to the survey.
The real root cause of the transient 401s is a separate, still-open
effort: **FE-963** (reactive 401 re-mint + single retry), **FE-950/951**
(unified Cloud JWT), **BE-1125**. This PR does **not** fix those; it
stops the survey from being their user-visible casualty.
## The fix
`getSurveyCompletedStatus` now distinguishes the responses instead of
failing closed on all of them:
- **404** → not completed (show survey). This is the genuine signal: the
cloud backend (`GetSettingById`) returns 404 for a survey key that was
never stored, and a 404 is only reachable after a successful
authenticated read (a stale token 401s, never 404s), so it can't be a
transient false signal.
- **transient 401/403/5xx/network** → treat as completed (fail-safe), so
a working user is never bounced.
- **200** → completed iff `value` is non-empty (unchanged).
**No router change.** The `/` onboarding guard is untouched (router.ts
matches main), so the existing UX is preserved — a not-completed user is
still gated to the survey on load; only the spurious transient-failure
bounce is removed.
## Why #12301 was reverted, and how this differs
#12301 shipped a **blanket** fail-safe (`!response.ok → true`, 404
included), which made the survey unreachable for genuinely-not-completed
users (404 → "completed") and was reverted in #12344. This PR
special-cases **404 as the real not-completed signal** and fails safe
only on transient/ambiguous responses, so onboarding still works.
## Tests
- **Unit** (`auth.test.ts`): 200 non-empty → true; 200 empty / `null` /
missing `value` key → false; **404 → false**; 401/403/500/network →
true.
- **E2E** (`browser_tests/tests/cloudSurveyGate.spec.ts`, `@cloud`): a
transient 401 on `/` does **not** bounce a working user; a genuine 404
on `/` **does** route to the survey.
Linear: FE-739. Root cause (separate): FE-963 / FE-950 / FE-951 /
BE-1125.
|
||
|
AustinMroz
|
1a27372e44 |
Fix 'insert as node' in sidebar tab (#12900)
When right clicking an output asset from the assets sidebar panel, the 'insert as node in workflow' action was twice bugged - The default type, as used for determining filename annotation, was set to the type of the file. This meant that annotations would never be applied to the filename - `temp` outputs would incorrectly be assigned the `output` type. - My fix for this one gives me a slightly bad taste in my mouth. Parsing URLs isn't great, but it's cleaner than needing to scan the (potentially sparse) full outputs to try and find the corresponding output. |
||
|
Dante
|
a738f75f13 | Merge branch 'main' into jaewon/fe-729-delete-is-asset-api-enabled | ||
|
AustinMroz
|
eafe2af91d |
Indicate in progress upload with spinner (#12673)
While a file is uploading to any of the file picker nodes (ie "Load Image"), the 'select folder' icon is replaced with a loading spinner. I had previously implemented this with a full progress bar indicating the rate of upload, but found it particularly uninformative when used on cloud. <img width="659" height="494" alt="image" src="https://github.com/user-attachments/assets/6d3ca82b-360b-44bc-b123-b276aae2c4d6" /> |
||
|
AustinMroz
|
4c870d84ed |
Fix disabling of linked widgets in props panel (#12896)
| Before | After | | ------ | ----- | | <img width="360" alt="before" src="https://github.com/user-attachments/assets/9d602ee3-ff10-48b9-95ca-4c7f5ca57a45" /> | <img width="360" alt="after" src="https://github.com/user-attachments/assets/36e96aff-60ec-4f8c-b7c9-b4d68e03884c" />| Making reactivity function is a little bit clunkier than I would like, but it'll get simplified in the future by east coast swing |
||
|
Maanil Verma
|
0c23e8305f |
fix: skip templates modal when opening a template from the URL (#12835)
## Summary On first launch, the templates modal flashed open for a split second before a deeplinked template (`?template=`) loaded, which felt broken. ## Changes - **What**: Gate the first-launch templates modal on template URL intent, alongside the existing shared-workflow (`?share=`) check. When a template is being opened directly from the URL, the template modal no longer opens. Behavior is unchanged when no template is in the URL — the template modal still shows for first-time users. - Test util: Added browser_tests/fixtures/utils/flashDetector.ts — installs a pre-navigation requestAnimationFrame sampler that flags if a [data-testid] element ever renders, even for a single frame. This catches a brief flash that toBeHidden() (final-state only) cannot. ## Review Focus `hasTemplateUrlIntent()` mirrors the existing `hasSharedWorkflowIntent()` (direct `route.query` check plus preserved-query fallback for the `/user-select` redirect path). Two regression tests cover both the URL-param and preserved-intent cases. **Coverage:** - Unit (useWorkflowPersistenceV2.test.ts): the modal is not opened when a template param is in the URL, and when template intent is preserved across the /user-select redirect. - E2E (templates.spec.ts): templates dialog never flashes when first-time user opens a template link — verified red-without-fix, green-with-fix. Screen Recording https://github.com/user-attachments/assets/636094d4-0ef0-4e42-af32-d4e6c7ec5731 closes #12836 |
||
|
Alexander Brown
|
c408f39cee |
test: harden assets media-type filter spec against VirtualGrid flake (#12897)
## Summary Harden the cloud assets media-type filter spec against a VirtualGrid virtualization flake that intermittently failed CI at the `waitForAssets(4)` precondition. ## Changes - **What**: Replace the 7 `waitForAssets(MIXED_JOBS.length)` preconditions in `assets-filter.spec.ts` with `waitForAssets()` (first card visible = data loaded), and document why. ## Review Focus CI artifact (`playwright-report-cloud`) from the failing run showed only 3 of 4 cards in the DOM — the 3D card was missing and the audio card rendered taller (inline player). `VirtualGrid.vue` sizes its render window from a single uniform `itemHeight` measured off the first card, so the taller audio card pushes the 3D card out of the initial window and it is virtualized out of the DOM until a re-measure (same cause as #11635). Requiring all 4 cards to be mounted simultaneously fights virtualization. `tab.open()` already waits for the first card (data loaded), and filtering reads the full asset store regardless of what is mounted, so the per-filter count assertions still provide the real coverage. No behavioral change to the tests — only the readiness/precondition strategy. Verified `pnpm exec eslint` and `pnpm typecheck:browser` pass. Cloud E2E could not be run locally (needs a running frontend + backend); relies on the cloud CI job for confirmation. Related to #11635 --------- Co-authored-by: Amp <amp@ampcode.com> |
||
|
Alexander Brown
|
e994e4df58 |
refactor: store-backed WidgetId subgraph host widgets; delete widgetValueIO layer (#12617)
## Summary Make `WidgetId` (`graphId:nodeId:name`) the single canonical widget identity and represent subgraph promoted host widgets as ordinary store-backed widgets addressed by it. This deletes two whole indirection layers — the `world/*` widget-entity-IO layer and the `PromotedWidgetView` runtime — leaving one model: a widget's data lives in `widgetValueStore` keyed by `WidgetId`, and a `SubgraphNode` input references it via `input.widgetId`. **Net +304 lines across 107 files** (5,044 added / 4,740 deleted): production code is net **−798** (1,521 added / 2,319 deleted) while tests are net **+1,102** (3,523 added / 2,421 deleted). 10 files deleted outright, 14 added. ## What got deleted The old design wrapped every promoted subgraph widget in a synthetic `IBaseWidget` "view" object with live getters that followed the source widget, plus a manager to keep view identities stable, plus an IO indirection layer over the store. All of it is gone: - `promotedWidgetView.ts` — the `PromotedWidgetView` class (draw / pointer / DOM-sync / projection / deepest-source resolution getters) - `PromotedWidgetViewManager.ts` — view reconciliation/caching - `world/widgetValueIO.ts` — the IO wrapper over `widgetValueStore` - `world/entityIds.ts` + `world/brand.ts` — the `WidgetEntityId` branded-id layer and the `entityId` field - `widgetNodeTypeGuard.ts` — only used by the deleted view - the per-`SubgraphNode` view machinery (`_promotedViewManager`, `_cacheVersion`, view-key generation, DOM position-override cleanup) and every now-dead `isPromotedWidgetView` branch across the panel, menu, store, and util consumers - `domWidgetStore` position-override APIs (`setPositionOverride` / `clearPositionOverride`), only used to render a promoted DOM widget on a different host node ## Why it's simpler - One source of truth. A promoted host widget is `WidgetState` in the store, seeded from the source at promotion (`registerWidget` with a deep-cloned snapshot) and independent thereafter. No synthetic widget objects, no runtime source-following, no view cache to invalidate. - Resolution is data-driven. `resolveConcretePromotedWidget` walks `SubgraphNode` inputs (`input.widgetId` + `resolveSubgraphInputTarget`) instead of chasing view objects through `node.widgets`. This also **fixes two-layer nested promotion** — the previously-skipped parity test now passes and resolves through to the deepest concrete widget. - The right-panel Parameters tab renders a subgraph node's promoted widgets through the **same** store-backed path as ordinary node widgets: display reads `WidgetState` via `widget.widgetId`, and value writes go through `widgetValueStore.setValue(widgetId)`. ## Changes - **What**: - `WidgetId` branded type + `widgetId()` / `parseWidgetId()` / `isWidgetId()` and a `WidgetState` type; `widgetValueStore` is `WidgetId`-native (`registerWidget` / `getWidget` / `setValue` / `deleteWidget`). - Promotion creates host `WidgetState` then an input projection (`input.widgetId`); demotion clears it; serialization and legacy `proxyWidgets` migration round-trip through `input.widgetId`. - `promotedInputWidget.ts` projects a store-backed ordinary widget from an input slot; `SubgraphNode.widgets` is now a projected getter over inputs (kept Litegraph-shaped so the canvas renderer and extensions still read `node.widgets`). `invalidatePromotedViews()` is retained as a no-op for extension compatibility. - `promotedWidgetControl.ts` applies `control_after_generate` (e.g. seed increment) on the host node, since the interior control widget is link-fed and its value is dead; `syncPromotedComboHostOptions` mirrors interior combo options onto host state. - `multilineTextarea.ts` extracts the reusable multiline DOM-widget behavior out of `useStringWidget` and adds promoted multiline materialization via a `createPromotedHostWidget` app-layer hook (keeping Litegraph core free of Vue/Pinia/DOM). - Late-bound `LiteGraph` singleton holder (`litegraphInstance.ts`) to break a widget-init import cycle. - **Breaking**: - `IBaseWidget.entityId` removed — use `widgetId`. - `SubgraphNode.widgets` no longer exposes the old `PromotedWidgetView` objects; promoted state lives in `widgetValueStore` keyed by `input.widgetId` and `widgets` is a projection of inputs. The `widget-promoted` event now carries the concrete interior widget. Extension code reading `entityId` or relying on `PromotedWidgetView` is affected. ## Review Focus - Promotion / demotion / serialization round-tripping through `input.widgetId` + `widgetValueStore`, incl. the legacy `proxyWidgets` migration. - Snapshot-at-promotion semantics (host widget does not follow the source after creation), and the combo-options exception via `syncPromotedComboHostOptions`. - Two-layer nested resolution in `resolveConcretePromotedWidget` + `SubgraphNode` nested-source resolution. - The unified Parameters tab (`TabSubgraphInputs` → `SectionWidgets`): value edit / rename / favorite / hide / reorder for promoted inputs are wired through the store but warrant a visual/e2e pass. - Litegraph-compat seams worth a careful read: projected `SubgraphNode.widgets`, the canvas-edit `callback` bridge back to the store, host-level `control_after_generate`, and the late-bound `LiteGraph` holder / `domWidget.ts` import ordering. --------- Co-authored-by: Amp <amp@ampcode.com> Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Co-authored-by: AustinMroz <austin@comfy.org> Co-authored-by: GitHub Action <action@github.com> |
||
|
AustinMroz
|
cb52a3821b |
fix groups dragging children with control held (#12867)
When control is held, an active drag operation should cease applying movements to nodes contained by selected groups. This functionality was broken in vue mode because of unnecessary reimplementation of the code for calculating items contained by groups during drag operations |
||
|
jaeone94
|
941f220582 |
fix: bind replacement node widgets to reused id (#12872)
## Summary Fixes a Nodes 2.0 node replacement regression where widgets that only exist on the replacement node were not registered with the widget value store, causing their Vue-rendered controls to fall back to component defaults such as `0` instead of the replacement node's real widget default. The root cause is that `replaceWithMapping()` replaces the placeholder node in-place by writing directly to `graph._nodes` and `graph._nodes_by_id`. That path intentionally preserves the old node id, but it also bypasses the normal `LGraph.add()` flow that binds widgets to their owning node id. As a result, newly introduced bindable widgets on the replacement node could exist on the LiteGraph node object while remaining absent from `useWidgetValueStore`, which is the state Vue Nodes reads from when rendering widget controls. ## Changes - **What**: Bind every bindable widget on the replacement node to the reused node id inside `replaceWithMapping()` after the replacement node is inserted into the graph maps and before widget values are transferred. - **What**: Preserve the existing widget value transfer behavior for mapped widgets. Because widgets are now bound before `newWidget.value = oldValue` runs, transferred values are written through the normal widget store state instead of only mutating the unbound widget object. - **What**: Add a focused unit regression check that verifies replacement-only widgets are bound with the reused node id during node replacement. - **What**: Extend the existing node replacement Playwright coverage to assert the Vue Nodes rendered input for `KSampler.denoise` keeps the expected replacement value after the replacement flow. - **Breaking**: None. - **Dependencies**: None. ## Review Focus Please focus on the placement of the widget binding in `replaceWithMapping()`. The binding happens after the new node has been assigned the reused id and inserted into the graph's node maps, but before mapped widget values are copied over from the old node. This mirrors the important part of the normal graph add flow for widgets while keeping the in-place replacement behavior intact. The tests intentionally avoid asserting replacement-node fixture defaults in isolation. The unit test verifies the actual new side effect that prevents the regression: `setNodeId()` is called for a bindable widget that was not present on the old node. The Playwright assertion then covers the user-visible Nodes 2.0 symptom: the replacement widget is rendered from the widget store instead of falling back to the Vue numeric default. Linear: FE-1070 ## Validation - `pnpm vitest run src/platform/nodeReplacement/useNodeReplacement.test.ts` - `pnpm typecheck:browser` - `PLAYWRIGHT_LOCAL=1 PLAYWRIGHT_TEST_URL=http://localhost:5174 pnpm exec playwright test --project=chromium browser_tests/tests/nodeReplacement.spec.ts -g "Widget values are preserved after replacement"` - `pnpm lint` - `pnpm typecheck` - Commit hook also reran staged formatting/linting and `pnpm typecheck` during the final amend. ## Screenshots (if applicable) Before https://github.com/user-attachments/assets/dc4e8137-d8aa-4a70-9973-5559ed84b90e After https://github.com/user-attachments/assets/4c70b9e4-d971-4e94-8d2f-12b0f2b00a09 |
||
|
Dante
|
75553fc214 |
fix(settings): widen the Settings dialog to 1280 (#12849)
## Summary The redesigned Settings dialog (Figma DES `3253-16079`) is **1280px** wide, but it rendered at **960px**. Root cause — the width was capped at 960 in **two** layers: 1. `useSettingsDialog.ts` → `SETTINGS_CONTENT_CLASS` (`max-w-[960px]`) sizes the Reka dialog shell. 2. `SettingDialog.vue` → `<BaseModalLayout size="sm">` (`SIZE_CLASSES.sm = max-w-[960px]`) sizes the modal content. Widening only the shell leaves the inner `BaseModalLayout` at 960 (empty space on the right). This sets both to **1280px** and lets `BaseModalLayout` fill the shell (`size="full"`). The dialog size is **not** a workspace-specific concern, so it applies to all Settings (OSS + cloud) — no feature-flag gate. Found during FE-768 designer QA. ## Verification - Live: dialog measures 1280px, content area 1006px (was 960 / 688). - `useSettingsDialog.test.ts`: `contentClass` is 1280px (`size: 'full'`). - `pnpm typecheck` / `lint` / `format` / unit tests green. ## Test Plan - [x] Settings dialog renders at 1280px with the content filling the dialog - [x] Unit test asserts the 1280px sizing ## Screenshots Settings ▸ Plan & Credits at **1280px** (content fills the dialog; was 960px shell / 688px content area): **Personal — Pro:** <img width="720" alt="Settings dialog at 1280px — personal Pro" src="https://github.com/user-attachments/assets/adc2fd9f-d249-469f-b947-1ec8f674cbb0" /> **Team:** <img width="720" alt="Settings dialog at 1280px — team" src="https://github.com/user-attachments/assets/e7378067-11a2-411b-b37b-98c8aecb82b1" /> --------- Co-authored-by: github-actions <github-actions@github.com> |
||
|
Alexander Brown
|
7438f004c1 |
test: add mask editor load/save round-trip browser tests (#11369)
*PR Created by the Glary-Bot Agent* --- ## Summary Adds `browser_tests/tests/maskEditorLoadSave.spec.ts` covering the untested image loading, save round-trip, canvas dimension verification, and error handling paths in the mask editor. ### Coverage gaps filled - `useImageLoader.ts` — image loads onto canvas with correct dimensions - `useMaskEditorSaver.ts` — save uploads non-empty mask data, round-trip preserves state - `useMaskEditorLoader.ts` — editor initialization, canvas dimension matching - Error handling — partial upload failure keeps dialog open ### Test cases (5 tests, 2 groups) | Group | Tests | Behavior | |---|---|---| | Save round-trip | 3 | Save with drawn mask uploads non-empty data, save-and-reopen preserves mask state, canvas dimensions match loaded image | | Load and error handling | 2 | Opening editor loads image onto canvas, partial upload failure keeps dialog open | ### References - Reuses patterns from existing `maskEditor.spec.ts` (`loadImageOnNode`, `openMaskEditorDialog`, `getMaskCanvasPixelData`, `drawStrokeOnPointerZone`, route mocking for upload endpoints) - Follows `browser_tests/AGENTS.md` directory structure - Follows `browser_tests/FLAKE_PREVENTION_RULES.md` assertion patterns ### Verification - TypeScript: clean - ESLint: clean - oxlint: clean - oxfmt: formatted ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-11369-test-add-mask-editor-load-save-round-trip-browser-tests-3466d73d3650818b8245c0b355011136) by [Unito](https://www.unito.io) --------- Co-authored-by: Glary-Bot <glary-bot@users.noreply.github.com> Co-authored-by: bymyself <cbyrne@comfy.org> |
||
|
AustinMroz
|
cdde1248d4 |
Resolve errant executionIds on workflow restore (#12659)
Node previews are stored by `locatorId`, but sent from the server by `executionId`. Normally, this difference is reconciled when the event is received, but this step is skipped when the workflow is backgrounded. Upon reloading the workflow, these backlogged `executionId`s were incorrectly mapped directly onto node outputs. Any outputs located inside a subgraph would then fail to display because `executionId`s are now `locatorId`s. This is solved by resolving any `executionId`s at time of output restoration. Because `executionId`s can only leak into the outputs of backgrounded workflows, it is safe for resolved `executionId`s to overwrite any pre-existing `locatorId`s. It might wind up cleaner to instead properly enforce that the nodeOutputs cached by change tracker resolve a `locatorId` at time of receipt. This would follow naturally for properly branded id types, but would then require resolving `locatorId` from suspended workflows which is a good bit more involved. |
||
|
Dante
|
4b979f4ad0 |
feat(dialog): migrate mask editor + 3D viewer dialogs to the Reka renderer (FE-578) (6a -1) (#12848)
## Summary Splits the **heavy, hard-to-test surface** out of the Phase 6 dialog cutover (#12593) into its own independently reviewable, independently testable PR — per @jtydhr88's review feedback that #12593 bundled too many concepts (3D, mask editor, and the renderer cutover) to test thoroughly at once. This PR migrates only the four style-string dialog callers that carry **Playwright screenshot baselines** and **maximize behavior** — the mask editor and the 3D viewers — plus the shared dialog infrastructure they need. **#12593 is rebased on top of this PR** and now contains only the renderer cutover. Parent: [FE-571](https://linear.app/comfyorg/issue/FE-571/dialog-system-migration-primevue-reka-ui-parent) This phase: [FE-578](https://linear.app/comfyorg/issue/FE-578/phase-6-remove-primevue-dialogconfirmdialog-imports-clean-up-css) ## Why this is safe to land alone **The global renderer default stays `'primevue'`.** Every caller migrated here sets `renderer: 'reka'` explicitly, and the infra additions are purely additive. So no other dialog changes behavior and there is no half-migrated state — the default flip and the remaining caller migrations all live in the stacked cutover (#12593). ## Changes **Heavy callers → `renderer: 'reka'` + `size`/`contentClass`:** - Mask editor (`useMaskEditor.ts`) — `mask-editor-dialog` hook class moves to `contentClass` so `browser_tests` selectors keep working unchanged - 3D viewers ×4 (`ViewerControls.vue`, `AssetsSidebarTab.vue`, `JobHistorySidebarTab.vue`, `load3d.ts`) **Infra to reach Reka parity (additive):** - `dialogStore`: `headerClass`/`bodyClass`/`footerClass` (Reka-path analogues of `pt.header`/`pt.content`/`pt.footer`) - `GlobalDialog`: forward the section classes; merge `bodyClass` into the body wrapper - `DialogContent`: maximized re-asserts its dimension classes after the caller's `contentClass` so maximize wins, mirroring `.p-dialog-maximized` `!important` - `tailwind-utils`: teach tailwind-merge the `max-h-none` class so maximize can release the caller's `max-height` - `rekaPrimeVueBridge`: keep a backgrounded reka dialog from dismissing when a stacked dialog opens on top of it - `maskeditor/useKeyboard`: capture keydown so undo/redo survive the Reka focus trap ## Quality gates - [x] `pnpm typecheck` — clean - [x] `pnpm lint` / `pnpm format` — clean (lint-staged) - [x] `GlobalDialog.test.ts` — 25 passing (incl. new section-class + maximize-override + stacked-dismiss tests) - [x] Changed-source unit tests (`useMaskEditor`, `useKeyboard`, `ViewerControls`, `load3d`) — 77 passing - [ ] CI Playwright — mask editor baselines refreshed for the Reka chrome (`browser_tests/tests/maskEditor.spec.ts-snapshots/*`) ## Out of scope (stacked in #12593) The renderer cutover: `showConfirmDialog` flip, remaining `dialogService`/composable callers (signin, top-up, workspace, subscription, publish, share, …), **the `createDialog` default flip to `'reka'`**, e2e selector retargeting, and the `ConfirmationService` removal. PrimeVue branch deletion remains Phase 6b. ## 📸 Screenshots — before (PrimeVue) → after (Reka) Captured via Chrome DevTools against this branch in cloud mode (`cloud.comfy.org` backend), with an input image / `cube.obj` loaded. Only the dialog **chrome** migrates (PrimeVue `Dialog` → Reka `DialogContent`); the editor/viewer content is unchanged. ### Mask editor (`useMaskEditor`) | Before (PrimeVue) | After (Reka) | |---|---| | <img width="430" alt="mask editor before" src="https://github.com/user-attachments/assets/267e63b5-0832-409e-9c41-edf5ff96561f" /> | <img width="430" alt="mask editor after" src="https://github.com/user-attachments/assets/073cd824-8b01-4c07-99e1-a3a054906c7a" /> | ### 3D viewer (`load3d` / `ViewerControls`) | Before (PrimeVue) | After (Reka) | |---|---| | <img width="430" alt="3D viewer before" src="https://github.com/user-attachments/assets/17b2cd2f-18e4-4d9a-9e0e-80ef833db216" /> | <img width="430" alt="3D viewer after" src="https://github.com/user-attachments/assets/9e20a7a5-4d22-40e6-8fa2-ece58b6e4d20" /> | ### 3D viewer — maximized (maximize-wins dimension re-assertion in `DialogContent`) | Before (PrimeVue) | After (Reka) | |---|---| | <img width="430" alt="3D viewer maximized before" src="https://github.com/user-attachments/assets/b705a4d5-4657-41ad-b6f3-95e54494ac9b" /> | <img width="430" alt="3D viewer maximized after" src="https://github.com/user-attachments/assets/188de427-ab58-45a9-8666-967b2908c320" /> | |
||
|
Dante
|
700ff4644f |
feat(workspace): switcher popover left of profile menu + DES-246 copy (FE-769) (#12763)
## Summary
Aligns the workspace switcher and creation flow to DES-246 (FE-769): the
switcher popover now opens to the **left** of the profile menu instead
of on top of it, team workspace rows drop the tier badge, and the
create-workspace dialog matches the design's copy and surface.
## Changes
- **What**:
- `CurrentUserPopoverWorkspace.vue`: replace the nested PrimeVue
`Popover` (rendered on top of the menu) with an inline panel anchored
left of the selector row (`right-full`, top-aligned, outside-click close
via VueUse)
- `WorkspaceSwitcherPopover.vue`: tier badge only renders on the
personal workspace row ("Remove the tier badge for team workspaces,
since there'll only be one plan now")
- Copy (`en/main.json`): switcher create label "Create a team workspace"
("Explicitly say 'team'"); create dialog message "Workspaces keep your
projects and files organized. Subscribe to a Team plan to invite
members.", label "Workspace name", placeholder "Ex: Comfy Org"
- `CreateWorkspaceDialogContent.vue`: surface matched to the Create
Workspace / Default frame — 512px width, muted name label, filled 40px
TextInput (`bg-secondary-background`, `rounded-lg`, `px-4`)
- Invite-flow copy deltas from DES-246: none — #12759 (FE-768) already
matches the design verbatim
Was stacked on #12762 (FE-778); that PR merged, so this is now rebased
onto `main` with only the FE-769 commits.
- Fixes
[FE-769](https://linear.app/comfyorg/issue/FE-769/updates-to-misc-ux)
## Review Focus
- The switcher panel now lives inside the profile popover DOM (no
teleport): clicking a row keeps the menu open, outside-click closes only
the panel
- The CREATOR badge on the profile-menu workspace selector row (visible
in the Figma frame) is intentionally not included — it needs
`is_original_owner` from the BE role-change work and ships with FE-770
- `leave-last-workspace -> auto-create` flow is deferred (not V1),
intentionally untouched
## Screenshots (if applicable)
| Before (overlaps menu, tier badges, "Create new workspace") | After
(left of menu, no team badges, "Create a team workspace") |
| --- | --- |
| <img width="700" alt="before"
src="https://github.com/user-attachments/assets/5522fcca-91b5-49e6-beaa-df1b88bed018"
/> | <img width="1100" alt="after"
src="https://github.com/user-attachments/assets/ce74d42e-19bd-4fe6-9477-b22e5964736d"
/> |
Create-workspace dialog with DES-246 copy and surface (512px, filled
input):
<img width="900" alt="create dialog after"
src="https://github.com/user-attachments/assets/e78eff0a-1c0e-4bbb-ac70-6cc1da996682"
/>
|
||
|
jaeone94
|
6d43320b93 |
Simplify missing model error presentation (#12793)
## Summary Simplifies the Missing Models error card as the fifth slice of the catalog-driven error-tab redesign. This PR is intentionally larger than the previous slices because Missing Models is the only remaining error type where the card UI, OSS download flow, Cloud import flow, and shared model-import dialog all have to move together to preserve the resolution path. The high-level goal is to make Missing Models behave like the other simplified error cards: show the exact missing item, show the affected nodes, keep locate actions predictable, and only expose actions that can actually resolve the problem. This follows the staged error-tab cleanup plan: 1. #12683 refined validation, runtime, and prompt error presentation. 2. #12705 simplified missing media error presentation. 3. #12735 simplified missing node pack error presentation. 4. #12768 simplified swap node error presentation. 5. This PR simplifies missing model presentation and the model-import handoff. ## Why This PR Is Larger Missing Models has more resolution paths than the previous error groups: - OSS can refresh model state, download individual models, and download all available models. - Cloud cannot download directly from the panel; it resolves supported rows through the model import dialog. - Some Cloud rows cannot be resolved through import at all because the node/widget cannot consume imported model assets. - Importing from Cloud needs to know the originating missing-model row so it can lock the expected model type and apply the imported model back to the affected widgets. - Already-imported files can still be unusable if they were imported under a different model type than the missing node expects. Because of those constraints, splitting the card layout from the dialog handoff would leave either a misleading Import button or an import dialog that does not know what it is resolving. This PR keeps that behavior in one reviewable unit. ## User-Facing Behavior ### Shared Missing Models Card - Replaces the older grouped presentation with compact model rows. - Shows each missing model as the primary row label. - Shows model metadata as a smaller sublabel instead of using large section headers. - Keeps locate-node controls visually consistent with the other simplified error cards. - Keeps rows expandable when multiple nodes reference the same missing model. - Shows the affected node rows under expanded models. - Allows single-reference rows to locate the affected node without rendering an extra duplicate child row. - Keeps unknown rows visible, including their affected nodes, instead of silently hiding them. - Removes the old library-select UI from the Missing Models card. ### OSS Behavior - Keeps the refresh action available from the Missing Models group header. - Keeps individual Download actions for downloadable models. - Moves file size out of the Download button label and into the row sublabel. - Keeps Download all when multiple downloadable models are available. - Places Download all at the bottom of the card rather than competing with the group header. - Leaves rows without a download URL as non-downloadable instead of rendering a broken action. ### Cloud Behavior - Shows Import only for missing models that can be resolved by importing a model asset of the required type. - Separates models that cannot be resolved through Cloud import into an Import Not Supported section. - Gives unsupported rows a direct explanation: nodes referencing those models do not support imported models, so users need to open the node and choose a supported built-in model or replace the node with a supported loader. - Treats unknown model type/directory as unsupported for Cloud import, because the import dialog cannot lock a valid model type and the node cannot safely consume the imported asset. - Keeps affected nodes visible in the unsupported section so users still have a path to locate and replace the node manually. ## Cloud Import Dialog Changes The shared model import dialog now accepts missing-model context when opened from the Missing Models card. When that context is present: - The dialog shows which missing model will be replaced. - The dialog lists the affected node/widget references that will be updated. - The model type selector is locked to the required model directory/type. - The Back/import-another path is disabled when it would break the targeted missing-model flow. - Import progress can be associated with the originating missing-model row. - After import completion, matching missing-model references are applied automatically where possible. - If the selected file is already imported under an incompatible model type, the dialog shows a targeted failure state explaining why this import cannot resolve the missing model. This keeps the generic import dialog reusable while adding only the context-specific behavior needed for Missing Models. ## Implementation Notes - `MissingModelCard.vue` owns the card-level grouping and OSS/Cloud section decisions. - `MissingModelRow.vue` owns per-model row rendering, expansion, locate actions, import/download actions, and row-level progress states. - `useMissingModelInteractions.ts` remains the interaction layer for locating nodes and applying resolved model selections. - `UploadModelDialog.vue`, `UploadModelConfirmation.vue`, `UploadModelFooter.vue`, `UploadModelProgress.vue`, and `useUploadModelWizard.ts` receive the missing-model context needed by the Cloud import handoff. - `MissingModelLibrarySelect.vue` is removed because the simplified card no longer exposes that inline selection path. - Locale and selector changes are limited to the new simplified row/section states and removed unused Missing Models strings. ## Tests Added / Updated - Unit coverage for Missing Models card grouping and row states. - Unit coverage for importable vs unsupported Cloud rows. - Unit coverage for model row expansion, locate actions, progress display, and action availability. - Unit coverage for upload confirmation/footer/progress behavior when a missing-model context is present. - Unit coverage for incompatible already-imported model handling. - E2E coverage for OSS Missing Models presentation. - E2E coverage for mode-aware Missing Models interactions. - Cloud E2E coverage for importable rows vs Import Not Supported rows. - Cloud E2E coverage for opening the import dialog with missing-model replacement context. ## Review Focus - Cloud import eligibility: unsupported or unknown model rows should not expose Import as if the row can be resolved automatically. - Missing-model context in the import dialog: the required model type should be locked, and the affected node/widget references should be clear. - OSS parity: OSS should keep refresh, individual Download, and Download all while visually matching the simplified Cloud card where possible. - Narrow side panel behavior: row labels may wrap, but link, primary action, and locate controls should not overlap. - Scope boundaries: this PR intentionally does not redesign Missing Node Pack / Swap Node / Missing Media again; visual parity issues shared across those cards can be handled in a follow-up unification pass if needed. ## Validation - `pnpm format` - `pnpm lint` - `pnpm typecheck` - Related unit tests: 8 files / 84 tests passed - `pnpm build` - OSS Missing Models E2E: `errorsTabMissingModels.spec.ts` passed, 8/8 - Mode-aware Missing Models E2E subset passed, 11/11, excluding unrelated local paste clipboard cases - `pnpm build:cloud` - Cloud Missing Models E2E: `errorsTabCloudMissingModels.spec.ts` passed, 3/3 - Final Claude review: no Blocker or Major findings ## Breaking / Dependencies - Breaking: none. - Dependencies: none. ## Screenshots OSS <img width="575" height="393" alt="스크린샷 2026-06-12 오전 12 25 27" src="https://github.com/user-attachments/assets/f5c44f95-711a-4d3d-99bd-f39ac2bb2012" /> <img width="659" height="351" alt="스크린샷 2026-06-12 오전 12 24 37" src="https://github.com/user-attachments/assets/4bb65a47-c1aa-408b-836b-a1998412f815" /> Cloud <img width="688" height="357" alt="스크린샷 2026-06-12 오전 12 23 59" src="https://github.com/user-attachments/assets/9330a7e7-9f22-420f-82b3-dde0fb2b3dd1" /> <img width="531" height="437" alt="스크린샷 2026-06-12 오전 12 21 13" src="https://github.com/user-attachments/assets/734bd911-f6f7-4872-8868-bb927ddeedd8" /> New import model flow https://github.com/user-attachments/assets/c094c670-62b9-47ce-bfe1-2d09f4f7359d |
||
|
AustinMroz
|
e138d17459 |
Fix themeing of nodes (#12712)
In moving styling to `documentElement`, #9516 introduced a regression preventing themes from styling nodes. | Before | After | | ------ | ----- | | <img width="360" alt="before" src="https://github.com/user-attachments/assets/0a27ea1b-ff15-4524-b491-a0de9fee6ed2" /> | <img width="360" alt="after" src="https://github.com/user-attachments/assets/36aee446-6b7d-4b05-96de-39b19989af0d" />| Note: Some elements (like the app mode toggle) are themed using different variables (`--secondary-background` instead of `--component-node-background`). I think the sanest approach is to define `--secondary-background` to be `--component-node-background` by default, but that feels better handled as a followup PR --------- Co-authored-by: Alexander Brown <drjkl@comfy.org> Co-authored-by: github-actions <github-actions@github.com> |
||
|
pythongosssss
|
193f23e8c2 |
Revert "feat: default search to essentials when graph is empty" (#12814)
Reverts Comfy-Org/ComfyUI_frontend#12377 |
||
|
AustinMroz
|
46526cfabd |
On mode toggle apply to group children (#12809)
When performing mode toggle operations (like bypass or mute) with a group (the colored rectangles) selected, nodes contained within the group will be considered selected and will have their state toggled. <img width="1024" height="1024" alt="AnimateDiff_00002" src="https://github.com/user-attachments/assets/c4e9db17-3fe8-4fd8-9012-0e9a0bc59707" /> |
||
|
jaeone94
|
c7797b201e |
Simplify swap node error presentation (#12768)
## Summary Simplifies the Swap Nodes error card as the fourth slice of the catalog/error-tab presentation refactor, aligning it with the newer compact error-row patterns while preserving the existing replace and locate behavior. This follows the staged rollout plan from the earlier error-tab PRs: 1. #12683 refined execution-style errors: validation, runtime, and prompt errors. 2. #12705 simplified missing media errors into flat, locatable rows. 3. #12735 simplified missing node pack errors and aligned grouped-row behavior. 4. This PR applies the same simplification pass to Swap Nodes errors. 5. A later PR is expected to handle Missing Models, which is larger and intentionally kept separate. After the Missing Models slice lands, a follow-up consistency PR will normalize the shared row/disclosure pattern across Missing Node Packs, Swap Nodes, and Missing Models together. That follow-up will cover parameterized i18n labels for disclosure controls, shared text-button styling, and consistent disclosure semantics/accessibility across those grouped rows. ## Changes - **What**: Reworks the Swap Nodes card rows so each replacement group is presented as a compact row with the source node type, replacement target, replace action, and locate action. - **What**: For a single affected node, the visible row label can be clicked to locate the node, matching the interaction model used by the newer missing-media and missing-node rows. - **What**: For multiple affected nodes with the same replacement target, the group renders a count badge and a disclosure row. Expanding the group shows the affected node rows, each with its own locate action. - **What**: Removes the old node-id badge path from Swap Nodes rows. Node-id badges remain available to the other error cards that still own that behavior. - **What**: Keeps replacement behavior unchanged: per-group replacement and replace-all still call through the existing node replacement store flow. - **What**: Adds regression coverage for the new grouped-row UI, including same-type grouping in both Vue Nodes and LiteGraph render modes. - **Breaking**: None. - **Dependencies**: None. ## Review Focus Please focus on the Swap Nodes presentation and interaction symmetry with the previous error-tab PRs: - Single-node groups should remain directly locatable via the row label and the locate icon. - Multi-node groups should expose the count and expand/collapse behavior without adding duplicate focusable disclosure controls. - The visible row labels intentionally keep their own accessible names, while the separate locate icon uses the generic `Locate node on canvas` accessible name. This mirrors the established pattern from the previous slices. - The newly added Playwright fixture covers two same-type replaceable nodes so duplicate group keys and grouped disclosure behavior are exercised end-to-end. ## Validation - `pnpm format` - `pnpm test:unit src/platform/nodeReplacement/components/SwapNodeGroupRow.test.ts` - `pnpm test:browser:local browser_tests/tests/nodeReplacement.spec.ts --project=chromium` - Pre-commit hook: lint-staged, stylelint, oxfmt, oxlint, eslint, `pnpm typecheck`, `pnpm typecheck:browser` - Pre-push hook: `pnpm knip --cache` - Additional parallel code review pass completed locally; no blocker or major issues remained. ## Screenshots (if applicable) This PR <img width="561" height="362" alt="스크린샷 2026-06-11 오전 3 46 06" src="https://github.com/user-attachments/assets/65395467-6c2f-4aa1-84c5-3d9614c00c80" /> old (Main) <img width="611" height="798" alt="스크린샷 2026-06-11 오전 3 46 32" src="https://github.com/user-attachments/assets/3862d5df-f839-40c0-9488-ce64b051378e" /> |
||
|
Dante
|
598cf33ab7 |
[bugfix] Truncate long workspace names in workspace switcher (#12762)
## Summary Long team workspace names wrapped onto multiple lines in the user-menu workspace switcher, overflowing the fixed 54px rows and breaking the dropdown layout. Applies the same single-line ellipsis pattern already used by the current-workspace header (`CurrentUserPopoverWorkspace.vue`). ## Changes - **What**: `truncate` on the switcher name span, `max-w-full` on the name row, `shrink-0` on avatar/tier badge/check icon so only the name shrinks (`WorkspaceSwitcherPopover.vue`, 5 lines) - Regression tests: Vitest component test + `@cloud` Playwright e2e measuring single-line render height Fixes [FE-778](https://linear.app/comfyorg/issue/FE-778/bug-team-workspace-names-wrapping-to-multiple-lines-display-poorly-in) ## Red-Green Verification | Commit | CI | Result | |---|---|---| | `30e04e2` test only | [Tests Unit](https://github.com/Comfy-Org/ComfyUI_frontend/actions/runs/27278378157) / [Tests E2E](https://github.com/Comfy-Org/ComfyUI_frontend/actions/runs/27278378213) | 🔴 new unit test + cloud e2e fail (proves tests catch the bug) | | `d8f9a5c` fix | [Tests Unit](https://github.com/Comfy-Org/ComfyUI_frontend/actions/runs/27279508881) / [Tests E2E](https://github.com/Comfy-Org/ComfyUI_frontend/actions/runs/27279508715) | 🟢 same tests pass | ## Screenshots | Before | After | |---|---| | <img width="320" alt="before — name wraps to 4 lines, rows collide" src="https://github.com/user-attachments/assets/90f3286a-5b50-4477-9b5c-9d32d0b026e4" /> | <img width="320" alt="after — single line with ellipsis, row height intact" src="https://github.com/user-attachments/assets/8e47bbb2-b5b1-4945-a008-68491f39dc46" /> | ## Review Focus - Truncation chain: the name span is a flex item, so `truncate` (overflow-hidden) zeroes its automatic min size; `max-w-full` caps the `items-start` row at the container width. Mirrors the header pattern — no new component. - Figma `Team Plan - Workspaces` (Workspaces Menu component, node 2045-14413) specifies compact single-line rows; long-name overflow was undesigned, truncation preserves the spec'd layout. |
||
|
jaeone94
|
1b14f4df8a |
Simplify missing node pack error presentation (#12735)
## Summary Simplify the Missing Node Packs error card so it follows the new error-tab item-row direction, with clearer pack rows, predictable locate behavior, and focused E2E coverage. This is the third PR in the staged error-tab simplification plan: 1. Merged: execution/prompt/validation error presentation and catalog grouping in #12683. 2. Merged: missing media presentation simplification in #12705. 3. This PR: missing node pack presentation simplification. 4. Planned next: swap-node presentation simplification. 5. Planned later: missing model presentation and action-flow simplification. ## Changes - **What**: Refactors Missing Node Packs rows so pack-level and node-level actions are easier to scan and more consistent with the rest of the refreshed Errors tab. - **What**: Removes the node-id badge from missing node pack rows, matching the simplified item-row direction. - **What**: Makes a single-node known pack row directly locatable from the pack label, rather than rendering an extra child row. - **What**: Keeps multi-node packs collapsed by default, with both the chevron and pack title toggling the child node list. - **What**: Keeps unknown packs expanded by default, including the single-node unknown-pack case, so users can still see the unresolved node type immediately. - **What**: Keeps per-node child rows clickable for locate-on-canvas behavior when a pack contains multiple affected nodes. - **What**: Replaces missing-node-pack action labels with shared `g.install` and `g.search` copy and removes now-unused English locale keys. - **What**: Adds targeted Playwright coverage for the simplified missing-node-pack card, including unknown-pack default rows, row-label locate behavior, and chevron/title expansion behavior. - **Breaking**: None. - **Dependencies**: None. ## Review Focus Please focus on the missing-node-pack row behavior: - Single known pack with one affected node should stay compact and locate the node from the pack label or locate icon. - Known packs with multiple affected nodes should show a count, start collapsed, and expand/collapse from either the chevron or title. - Unknown packs should expose the affected node rows immediately, including when there is only one affected node. - Locate actions should remain attached to the affected node rows, not to the parent pack when there are multiple nodes. - The E2E fixture intentionally uses two missing nodes with the same `cnr_id` and node sizes of `[400, 200]` to follow browser-test asset guidance. ## Validation - `pnpm format:check` - `pnpm lint` - `pnpm typecheck` - `pnpm knip --cache` via pre-push hook - `pnpm test:unit src/components/rightSidePanel/errors/MissingPackGroupRow.test.ts src/components/rightSidePanel/errors/MissingNodeCard.test.ts --run` - `pnpm test:browser:local browser_tests/tests/propertiesPanel/errorsTabMissingNodes.spec.ts --project=chromium` - Pre-commit hook: staged formatting, linting, `pnpm typecheck`, and `pnpm typecheck:browser` ## Screenshots This PR <img width="531" height="598" alt="스크린샷 2026-06-10 오전 1 54 31" src="https://github.com/user-attachments/assets/9c0addeb-92d2-4cef-a4f3-35a87bbad308" /> old (Main) <img width="509" height="807" alt="스크린샷 2026-06-10 오전 1 53 51" src="https://github.com/user-attachments/assets/b8488f73-d8ed-4356-bd4c-fc678ea205f7" /> |
||
|
jaeone94
|
14f8fdebdd |
fix: refresh promoted combo host options after missing model reload (#12692)
## Summary Fixes a Vue node subgraph case where the missing-model refresh flow clears the missing-model error, but the promoted combo widget remains in an invalid visual state because its hosted options snapshot is stale. ## Changes - **What**: After `reloadNodeDefs()` refreshes combo option lists and extension `refreshComboInNodes` hooks run, resync hosted options snapshots for promoted combo widgets so Vue-rendered subgraph nodes see the newly available model option. - **What**: Adds a focused E2E regression for the missing-model refresh path on a subgraph with a promoted `ckpt_name` widget. - **What**: Hardens the E2E by cleaning up its `/object_info` route override and asserting the widget's `aria-invalid` state rather than a Tailwind implementation class. - **Breaking**: None. - **Dependencies**: None. ## Review Focus This PR is intentionally a minimal patch for the missing-model refresh path, not a broader subgraph architecture change. Root cause: `reloadNodeDefs()` updates the live LiteGraph combo widget options, including widgets inside subgraphs. However, Vue nodes render promoted widgets from a hosted `WidgetState.options` snapshot. When a missing model is downloaded and the missing-model refresh button reloads node definitions, the source combo receives the updated model list, but the promoted host snapshot can still contain the old option list. The missing-model error and node-level state are cleared, while the Vue combo still computes itself as invalid from stale options. The fix keeps the existing host snapshot model intact. It simply resyncs promoted combo host options after the normal combo refresh pipeline finishes. This avoids changing promoted-widget ownership, `useProcessedWidgets` merge precedence, or broader subgraph internals while addressing the reported stale invalid state. Why the helper is in `app.ts`: this sync is currently a single-call-site post-step of `reloadNodeDefs()`, and the ordering is load-bearing. It must run after the core combo refresh loop and after extension `refreshComboInNodes` hooks so it captures both built-in and extension-provided option changes. Keeping the small private helper next to the refresh orchestration makes that sequence explicit and avoids adding a new public subgraph helper or introducing a more visible dependency cycle through `promotionUtils` for a narrow patch. This is stacked on `jaeone/fe-942-bug-error-indicators-persist-after-resolving-missing-model`, so it is opened as a draft until the base PR lands. ## Test Plan - `pnpm knip` - `pnpm exec oxfmt --check src/scripts/app.ts browser_tests/tests/propertiesPanel/errorsTabModeAware.spec.ts` - `pnpm exec eslint --cache --no-warn-ignored src/scripts/app.ts browser_tests/tests/propertiesPanel/errorsTabModeAware.spec.ts` - `pnpm typecheck` - `pnpm test:browser:local --grep "Refreshing a resolved promoted missing model clears the combo invalid state"` - First local run hit a `beforeEach` timeout while the dev server was still warming custom-node/conflict-detection output. - Re-running against the warmed dev server passed. ## Screenshots (if applicable) N/A. The behavioral E2E covers the visible invalid-state regression. |
||
|
jaeone94
|
1b90696459 |
Simplify missing media error presentation (#12705)
## Summary This is the second PR in the planned Workflow Overview error-tab modernization stack. It follows the merged execution-error slice in #12683 and focuses only on Missing Media / Missing Inputs. The intent is to simplify missing-media errors from media-type/file-resolution cards into compact node-level guidance. Missing media is now presented as a flat list of affected node inputs, matching the new catalog-driven item-row direction introduced by the first PR while intentionally avoiding the larger missing-node, swap-node, and missing-model redesigns. This PR also removes the upload/library selection workflow from the error tab. Missing media rows now help the user locate the affected node/input directly, rather than attempting to resolve the missing file inside the error card. ## Changes - **What**: Simplifies the Missing Inputs card into a flat list of affected node/widget rows. - **What**: Displays each missing media row with a catalog-resolved item label in the form `{nodeName} - {inputName}`. - **What**: Makes both the row label and right-aligned locate icon navigate to the affected node on canvas. - **What**: Adds item-specific accessible labels for locate icon buttons and an explicit focus-visible state for clickable row labels. - **What**: Uses the existing `list-scale` transition pattern so missing media rows animate consistently when they enter or leave. - **What**: Removes the old image/video/audio sub-sections from the missing-media card. - **What**: Removes missing-media upload/dropzone/library-selection/status/confirm/cancel UI, because resolution is no longer handled inside this card. - **What**: Removes stale E2E selectors and tests tied to the deleted upload/library controls. - **What**: Removes stale English missing-media locale keys for the deleted controls; translated locale files are intentionally left to the localization sync process. - **What**: Removes stale missing-media store interaction state (`expandState`, `uploadState`, `pendingSelection`) and the now-unused name-based removal API that only supported the deleted upload/library flow. - **What**: Keeps the remaining widget/node/prefix candidate-removal APIs used by the missing-media lifecycle. - **What**: Carries `nodeType` through missing-media referencing nodes so fallback labels remain correct when the same missing filename is referenced by multiple node types. - **Breaking**: None expected. This changes the missing-media error-tab UX, but does not change the workflow schema or backend APIs. - **Dependencies**: None. ## Planned stack This PR is part of the same staged redesign plan described in #12683. The goal is to keep each error category reviewable and stable before moving to the next slice. 1. Execution errors, merged in #12683: validation, runtime, and prompt error grouping/presentation. 2. Missing media, this PR: simplify image/video/audio missing-media cards into node-level catalog item rows with locate actions. 3. Missing node and swap node: align missing-pack rows, nested node references, install/replace actions, and locate behavior. 4. Missing model: unify OSS and Cloud presentation, simplify download/import actions, and improve import/download progress behavior. This PR intentionally does not change missing node packs, swap nodes, missing models, runtime errors, validation errors, or prompt errors. ## Review Focus - Missing media rows should now be node-level rows, not file-level rows and not image/video/audio sections. - If the same missing media filename is referenced by multiple nodes, each affected node/input should render as its own row. - Clicking either the row label or locate icon should focus the corresponding affected node. - Removed upload/library controls are intentional and should not be restored in this PR. - The missing-media store cleanup should not remove any state or API still used by the remaining missing-media lifecycle. - Only `rightSidePanel.missingMedia.missingMediaTitle` should remain in the English locale for the old missing-media UI namespace; new row copy lives in the error catalog. - This PR should stay scoped to Missing Media / Missing Inputs and should not include the larger missing model/node/swap redesign work. ## Screenshots (if applicable) This PR <img width="483" height="339" alt="스크린샷 2026-06-08 오후 8 26 43" src="https://github.com/user-attachments/assets/e029411e-b1db-41cd-aa6e-2bd5d86762f6" /> Before (Main) <img width="484" height="802" alt="스크린샷 2026-06-08 오후 8 32 19" src="https://github.com/user-attachments/assets/c183c18d-babe-4f4e-b4e0-706ce41298de" /> ## Validation - `pnpm format` - `pnpm test:unit src/platform/missingMedia/missingMediaStore.test.ts src/components/rightSidePanel/errors/TabErrors.test.ts src/platform/errorCatalog/errorMessageResolver.test.ts src/platform/missingMedia/missingMediaScan.test.ts src/platform/missingMedia/missingMediaAssetResolver.test.ts` - `pnpm typecheck` - `pnpm knip --cache` - `pnpm test:browser:local browser_tests/tests/propertiesPanel/errorsTabMissingMedia.spec.ts --project=chromium` - Pre-push hook: `pnpm knip --cache` ## Review notes Claude review was run after the main implementation and again after cleanup. No blocker or major issues remained. The only final minor finding was stale inline i18n fixture data in `TabErrors.test.ts`, which was removed in the final commit. |
||
|
Dante
|
b93338cac7 |
fix(keybindings): progressively truncate keybinding column at narrow widths (#12181)
## Summary - Progressive container-query truncation for the **Keybinding** column in the Settings → Keybinding panel so the **actions column stops getting clipped** at narrow widths. - Tiers (all driven by `@container/keybindings` on the cell content — no JS resize listeners): - `>= 16rem`: `Ctrl S , Ctrl Shift S + 1 more` (2 combos + `N-2 more`) - `>= 12rem`: `Ctrl S + 2 more` (1 combo + `N-1 more`) - `>= 8rem`: `Ctrl S + 2` (1 combo + compact `+N`) - smallest: first combo only - Actions column pinned with `whitespace-nowrap` so it always stays fully visible — the keybindings column compresses first, not the icons. - New `KeybindingList.vue` extracted from `KeybindingPanel.vue`; the prior inline rendering was inlined twice (column body + we'd need it elsewhere if expansion logic changes). - Adds `g.nMoreKeybindingsCompact` and `g.keybindingListAriaLabel` i18n keys (en only; other locales fall back to en). ## Before / after ### before https://github.com/user-attachments/assets/227cdd6a-fd5d-41ba-8af2-2a49e2d9cf15 ### after <img width="936" height="812" alt="Screenshot 2026-05-13 at 11 05 12 AM" src="https://github.com/user-attachments/assets/f750efd8-7d14-4a46-a92d-fa67e57c7909" /> <!-- Drag the PNGs from ~/Desktop/fe-523-screenshots/ into this section. Recommended order: wide → 16rem → 12rem → 8rem, before then after at each tier. Captured by temp/scripts/capture-fe523-screenshots.ts at 4 dialog widths. --> _screenshots pending — drag/drop from `~/Desktop/fe-523-screenshots/`_ The narrow tier shows the original FE-523 bug clearly: on `main` the Source and Actions columns are clipped without a scrollbar; with this PR the keybinding column compresses first, keeping at least the edit icon visible. Some action icons still get clipped at the absolute narrowest tier — full sticky-actions behavior is a follow-up. ## Why draft Manual visual verification of the four container-query tiers on a real backend still needs to happen — the snapshot script runs against a stubbed-out backend, so the dataset is sparse (the multi-combo rows aren't injected). Moving out of draft after that pass. ## References - Fixes [FE-523](https://linear.app/comfyorg/issue/FE-523/update-keybinding-panel-responsiveness) - Figma proposal: https://www.figma.com/design/ZNLRans6JLM3hvCz5LlCQA/Keyboard-Shortcut?node-id=625-17286 - Slack thread: https://comfy-organization.slack.com/archives/C075ANWQ8KS/p1776818615078629?thread_ts=1776808927.654249&cid=C075ANWQ8KS ## Test plan - [x] `pnpm test:unit` — new `KeybindingList.test.ts` covers all 4 tiers (0/1/2/3+ keybindings, count math `N-2` vs `N-1`, container class present, aria-label assembled). - [x] `pnpm typecheck` - [x] `pnpm lint` - [x] Snapshot diff at 4 widths (see Before / after section above once images are attached). - [ ] Manual on a real backend: command with 1 / 2 / 3 / 5 keybindings each renders the correct `+N more` / `+N` value at each tier. - [ ] Manual: confirm whether residual actions-column clipping at the absolute narrowest tier warrants a follow-up. |
||
|
AustinMroz
|
d7f0d75efd |
Support fast disco on subgraph IO (#12619)
Because subgraph links don't follow the normal link logic, the fast disconnection circle would not function when dragging input links that originate from a subgraph IO node. There's some sad duplication of state, but I think the flow is marginally cleaner now by not requiring `instanceof` checks. The test added by this PR also verifies behavior for the node-to-node case. |
||
|
jaeone94
|
74b9f16b62 |
Refine execution error presentation (#12683)
## Summary This is the first PR in a planned stack to modernize the Workflow Overview error tab. It focuses only on execution-style errors: validation errors, runtime errors, and known prompt errors. The intent is to establish the catalog-driven presentation model before touching the larger missing-resource cards. Validation and prompt errors are known product states, so this PR makes them read more like structured guidance instead of generic reportable failures. Runtime errors remain reportable, but their details are reorganized so the error log is easier to scan and copy. ## What changed - Groups validation errors by error catalog id instead of node class/type. - Adds an `unknown_validation_error` fallback catalog id so validation grouping can follow one rule without special-case missing catalog ids. - Shows validation group title and message once, then lists each affected input as a compact item row. - Adds per-item validation detail disclosure so detailed validation text is still available without repeating the group title/message for every item. - Keeps locate-node behavior available from validation rows, including keyboard/ARIA disclosure wiring. - Removes GitHub, copy, and help actions from validation/prompt errors because these are known, cataloged errors where the UI copy should guide the user directly. - Refines runtime error cards so the error log is visible by default, has its own header, and keeps copy/report actions inside the log area. - Removes the special full-panel singleton runtime layout so runtime errors keep the same fixed card rhythm as the other error groups. - Keeps runtime errors reportable via Get Help and Find on GitHub, because these can still represent unexpected execution failures. - Updates prompt error detail styling to match the darker runtime error-log treatment. - Restores display-message semantics for grouped execution messages: `displayMessage ?? message` is used for user-facing dedupe instead of raw backend-only messages. - Adds focused unit coverage for catalog grouping, fallback validation catalog ids, display-message grouping, runtime detail behavior, and the updated prompt/validation action model. ## Planned stack This PR intentionally keeps the first slice narrow. The broader redesign is planned as a sequence of follow-up PRs rather than one large change: 1. Execution errors, this PR: validation, runtime, and prompt error grouping/presentation. 2. Missing media: simplify image/video/audio missing-media cards around catalog item labels and locate actions. 3. Missing node and swap node: align missing-pack rows, nested node references, install/replace actions, and locate behavior. 4. Missing model: unify OSS and Cloud presentation, simplify download/import actions, and improve import/download progress behavior. The goal is to review and stabilize each slice before stacking the next one. This is especially important because later missing-model changes are much larger and should not obscure the catalog/error-card behavior introduced here. ## Review focus - Validation errors should now group by catalog id, not by node class. - Validation groups intentionally show one message per group, with individual affected inputs rendered as rows. - Prompt and validation errors intentionally no longer show report/copy/help actions. - Runtime errors intentionally still show report actions, but only inside the error-log panel. - Node id badges are intentionally not shown in these execution error rows; the follow-up missing-resource PRs will handle their own row treatments separately. - This PR does not change missing media, missing model, missing node pack, or swap node cards. ## Screenshots ### This PR Validation error <img width="457" height="362" alt="스크린샷 2026-06-07 오전 4 26 19" src="https://github.com/user-attachments/assets/4c35b9f3-57dd-4dae-b44a-6d2fd8547b7c" /> Runtime error <img width="454" height="545" alt="스크린샷 2026-06-07 오전 4 24 24" src="https://github.com/user-attachments/assets/b7d4482f-b35b-4ed2-90f2-0a62dafa3519" /> Prompt / Service error <img width="456" height="192" alt="스크린샷 2026-06-07 오전 4 27 58" src="https://github.com/user-attachments/assets/aeec0978-b47f-40c7-ab71-0a0d18ceb054" /> ### Old (main) Validation error <img width="457" height="853" alt="스크린샷 2026-06-07 오전 4 25 09" src="https://github.com/user-attachments/assets/185dd573-430d-4041-8b31-a8eb6346f1ff" /> Runtime error <img width="455" height="554" alt="스크린샷 2026-06-07 오전 4 24 58" src="https://github.com/user-attachments/assets/deb1c09d-ea58-4d6a-9ac6-d2a3a9832fbe" /> Prompt / Service error <img width="455" height="297" alt="스크린샷 2026-06-07 오전 4 28 14" src="https://github.com/user-attachments/assets/c68eef7c-6525-4a5b-858c-6482fe76ad27" /> ## Validation - `pnpm format:check` - `pnpm test:unit src/components/rightSidePanel/errors/TabErrors.test.ts src/components/rightSidePanel/errors/ErrorNodeCard.test.ts src/components/rightSidePanel/errors/useErrorGroups.test.ts src/platform/errorCatalog/errorMessageResolver.test.ts` - `pnpm typecheck` - `pnpm lint` - `pnpm knip` - `pnpm build` |
||
|
jaeone94
|
dbeb9cc10d |
fix: clear missing model on promoted widget change (#12677)
## Summary Fixes FE-942 by clearing missing model indicators when promoted subgraph widgets are changed through the legacy canvas path. ## Changes - **What**: Resolves promoted widget error-clearing targets in `useErrorClearingHooks`, including legacy canvas path events from interior widgets. - **Dependencies**: None. ## Review Focus - Promoted widget validation errors clear by resolved interior widget name, while missing model/media state clears by source widget name. - Same-named promoted widgets are value-gated so changing one promoted model widget does not clear unchanged sibling indicators. - Core promoted widget event emission remains unchanged; the fix is scoped to the error-clearing hook. ## Validation - `pnpm test:unit src/composables/graph/useErrorClearingHooks.test.ts` - `pnpm test:unit src/core/graph/subgraph/promotedWidgetView.test.ts src/lib/litegraph/src/subgraph/SubgraphWidgetPromotion.test.ts` - `pnpm exec oxfmt --check src/composables/graph/useErrorClearingHooks.ts src/composables/graph/useErrorClearingHooks.test.ts src/stores/executionErrorStore.ts` - `pnpm exec oxlint src/composables/graph/useErrorClearingHooks.ts src/composables/graph/useErrorClearingHooks.test.ts src/stores/executionErrorStore.ts --type-aware` - `pnpm exec eslint src/composables/graph/useErrorClearingHooks.ts src/composables/graph/useErrorClearingHooks.test.ts src/stores/executionErrorStore.ts` - `pnpm typecheck` - `pnpm test:browser:local browser_tests/tests/propertiesPanel/errorsTabModeAware.spec.ts --project=chromium --grep Subgraph` - pre-push `knip --cache` ## Screenshots (if applicable) N/A |
||
|
Dante
|
f8187cec4c |
test: e2e for on-node grid thumbnail previews (FE-741) (#12667)
## Summary Adds an end-to-end (Playwright) test for the on-node image-grid thumbnail behavior introduced in #12561 (FE-741), kept in its own PR so #12561 stays scoped to the fix + unit tests. #12561 makes small on-node grid cells request a lightweight thumbnail URL (`preview=webp;75`, server-resized via `res` on cloud) instead of downloading the full-resolution `/api/view` image. That PR covers the helper and component with unit tests. This PR adds the missing **integration** coverage: it drives a real `Preview Image` node in the browser, injects a multi-image grid over the websocket, and asserts the rendered grid `<img>` elements request thumbnails. It exercises the full path the unit tests can't: `executed` WS output → `nodeOutputStore.buildImageUrls` → `getGridThumbnailUrl` → rendered grid `<img>` `src` ## What it checks - A 4-image `Preview Image` grid renders 4 cells (`viewMode === 'grid'`). - Every grid cell `<img>` `src` carries the compact thumbnail format (`preview=webp;75`; the `;` may be percent-encoded as `%3B`). - Each `src` still points at the real `/api/view` URL for that output (`filename=grid-<n>.png`), confirming it's the thumbnailed view URL, not a placeholder/blob. Lives next to the existing batch-preview test in `browser_tests/tests/vueNodes/interactions/node/imagePreview.spec.ts`, reusing the `ExecutionHelper` + `webSocketFixture` injection pattern. ## Notes - **Stacked on #12561** (`jaewon/fe-741-onnode-grid-thumbnail-preview`). Without that fix the grid cells use the full-res URL and this test is red — i.e. it's a true regression guard. Retarget to `main` once #12561 merges. - Gallery/full-view URLs staying at full resolution is already covered by the unit tests in #12561 (`currentImageUrl` is left untransformed); this test deliberately scopes to the grid path to avoid depending on injected previews actually resolving on the backend. ## Test Plan - [x] `pnpm typecheck:browser` clean - [x] ESLint + oxlint clean - [ ] e2e CI (`ci-tests-e2e`) green on this PR |
||
|
Terry Jia
|
7e61358724 |
FE-905 fix(load3d): cache scene capture so unchanged runs hit backend cache (#12627)
## Summary The scene widget's serializeValue uploaded a fresh temp image on every queue, so the `image / mask / normal` filenames in the prompt JSON were new each run. The backend cache key (which hashes those input strings) never matched, forcing Load3D and every downstream node to re-execute even when the user changed nothing. Track a session-scoped dirty flag and last-output cache in module-level WeakMaps keyed by LGraphNode. serializeValue returns the cached output when nothing has changed; user actions that mutate the visible scene (scene/model/camera/light config, animation, recording, gizmo, camera orbit) mark dirty through useLoad3d watchers and event handlers. The model_file / width / height widget callbacks invalidate via a new optional onSceneInvalidated hook plumbed through Load3DConfiguration, so the captured screenshot stays consistent with the inputs the backend sees. ## Screenshots (if applicable) Before https://github.com/user-attachments/assets/5ee5f79f-dd38-401e-babe-4d6ea156e56d After https://github.com/user-attachments/assets/5e00beb4-937c-4c66-abb2-e455f5301de6 |
||
|
Alexander Brown
|
ff9e6415b5 |
fix(nodes-2): apply Textarea widget font-size setting in Vue Nodes 2.0 (#12386)
*PR Created by the Glary-Bot Agent* --- ## Summary `Settings → Appearance → Node Widget → Textarea widget font size` (`Comfy.TextareaWidget.FontSize`) was wired through the legacy LiteGraph textarea only. The Vue Nodes 2.0 `WidgetTextarea.vue` hardcoded Tailwind `text-xs`, so once Vue nodes were enabled the slider had no effect. `GraphView.vue` already writes the setting value to `--comfy-textarea-font-size` on `:root` for the legacy `.comfy-multiline-input` rule. This PR makes `WidgetTextarea` consume the same variable via Tailwind v4's parenthesized CSS-variable shorthand, keeping `GraphView` as the single source of truth. - `text-xs` → `text-(length:--comfy-textarea-font-size) leading-normal`. The `length:` type hint is required because `text-` is ambiguous between `font-size` and `color`. `leading-normal` keeps line-height proportional to font-size across the 8–24 px range so multi-line text doesn't clip at the high end. - Initialize `--comfy-textarea-font-size: 10px` on `:root` in the design-system stylesheet so isolated renders (Storybook, tests) that do not mount `GraphView` still pick up the documented default. - Fixes [FE-799](https://linear.app/comfyorg/issue/FE-799/bug-textarea-widget-font-size-setting-not-working-in-nodes-20) ## Verification - `pnpm typecheck`, `pnpm lint`, `pnpm exec stylelint`, `pnpm exec oxfmt --check`, `pnpm knip`, and `WidgetTextarea.test.ts` (20 tests) all pass. - Manual browser verification with Vue Nodes 2.0 enabled and a `CLIPTextEncode` node: - setting `8` → computed `font-size: 8px` - setting `22` → computed `font-size: 22px` - setting `24` → computed `font-size: 24px`, computed `line-height: 36px` (ratio 1.5, no clipping) - Confirmed the legacy LiteGraph path still resolves to `comfy-multiline-input` with `font-size: 22px` when Vue Nodes is disabled (no regression). - Confirmed the `:root` default resolves to `10px` when `GraphView`'s inline override is absent (Storybook-like environments). ## Out of scope (follow-up) `WidgetMarkdown.vue` (the Vue Nodes 2.0 markdown/tiptap widget) also hardcodes `text-sm`. The legacy `.comfy-markdown .tiptap` rule reads the same `--comfy-textarea-font-size` variable, so the setting historically governed markdown widgets in Nodes 1.0. Bringing that into line with this PR's approach is a follow-up the design team should weigh in on before changing. ## Screenshots    ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-12386-fix-nodes-2-apply-Textarea-widget-font-size-setting-in-Vue-Nodes-2-0-3666d73d365081fd8084e84a41ee357b) by [Unito](https://www.unito.io) --------- Co-authored-by: Glary-Bot <glary-bot@users.noreply.github.com> Co-authored-by: github-actions <github-actions@github.com> |
||
|
Willie
|
b907423526 |
feat: add Arrange action to multi-select toolbox (#12068)
## Summary Adds an Arrange popover to the multi-select toolbox that repositions selected nodes into vertical, horizontal, or grid layouts, with a follow-up slider to tune the spacing. https://github.com/user-attachments/assets/f9a55ef9-2619-462b-a83f-2b86eb076fe3 ## Changes - **What**: New `ArrangeButton` placed between the color picker and frame icons. Three layouts (vertical, horizontal, grid) sort selected nodes by current position and lay them out from the smallest-`x+y` anchor with a 12-unit gap. Visual bounds account for `LiteGraph.NODE_TITLE_HEIGHT` so titles don't overlap stacked bodies. After picking a layout, the popover swaps to a 0–48 spacing slider — drag previews live (rAF-throttled, no undo capture), release commits one undo entry. Closing the popover ends the session. - **Breaking**: none - **Dependencies**: none ## Review Focus - `useArrangeNodes.computeArrangement` — pure layout math separated from side effects; covered by 8 unit tests including the title-height handling and `TitleMode.NO_TITLE` case. - `useArrangeSession` — owns the slider's state machine. rAF-throttled `previewGap` collapses rapid drag events into one frame; `commitGap` cancels any pending preview before capturing undo. Covered by 5 unit tests. - Position mutations go through `useLayoutMutations().batchMoveNodes` (single batched layout-store transaction) followed by `changeTracker.captureCanvasState()` for undo — same pattern as drag and other selection-toolbox actions. - Anchor selection uses smallest `pos.x + pos.y` rather than min-x or min-y alone, to keep the layout origin stable across re-runs (slider drags). ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-12068-feat-add-Arrange-action-to-multi-select-toolbox-3596d73d365081e58c25ffac41dc0b2a) by [Unito](https://www.unito.io) --------- Co-authored-by: AustinMroz <austin@comfy.org> |
||
|
AustinMroz
|
7d99189211 |
Ensure dropdowns display over selection toolbox (#12513)
| Before | After | | ------ | ----- | | <img width="360" alt="before" src="https://github.com/user-attachments/assets/a7fc3432-3db3-40a5-b28e-11a309db76ce" /> | <img width="360" alt="after" src="https://github.com/user-attachments/assets/a6a702cd-2ba3-4900-8afa-227cd9d61492" />| Dropdowns were appended to self, but the selectionToolbox isappended to body. As a result, changes to z-index on the dropdown would not allow it to display above the selectionToolbox. Since dropdowns have been migrated to reka-ui, dropdowns can now be safely appended to body as well. Doing so cleans up a lot of no-longer-needed code. Of note WidgetSelectDropdown seemed to never actually bind the `appendTo` and is unaffected by the removal As a secondary consequence of this change, dropdowns will no longer scale with the current zoom level of the graph. Since litegraph would not scale the size of popovers, this had been reported as a regression by some users. | Before | After | | ------ | ----- | | <img width="360" alt="before" src="https://github.com/user-attachments/assets/0cb200c2-0811-4023-9ff1-aaa61113cbd5"/> | <img width="360" alt="after" src="https://github.com/user-attachments/assets/c82c087f-1f25-49c5-a85d-d9502b438526" />| |
||
|
jaeone94
|
82bea29dda |
fix: defer node auto-pan until drag starts (#12654)
## Summary Fix a Vue node drag edge case where holding the partially off-screen Advanced inputs button could continuously auto-pan the canvas even though the pointer had not moved into an actual drag. Linear: [FE-938](https://linear.app/comfyorg/issue/FE-938/holding-partially-off-screen-advanced-inputs-causes-continuous) ## Changes - **What**: Move Vue node auto-pan initialization from `startDrag()` to `handleDrag()`, so auto-pan starts only after the pointer interaction has become a real drag. - **What**: Keep the existing auto-pan behavior during active drags by creating the controller on the first `handleDrag()` call, updating its pointer position on later drag frames, and preserving the existing `onPan` position adjustments. - **What**: Add unit coverage for the important drag lifecycle invariants: no auto-pan on pointerdown/startDrag, auto-pan starts on handleDrag, the same controller is reused across handleDrag calls, and cleanup still stops auto-pan on endDrag. - **What**: Add a Playwright regression that places the Advanced inputs button partially beyond the visible canvas edge, holds the pointer down without moving, and verifies the canvas offset stays stable. - **What**: Add `data-testid="advanced-inputs-button"` to the Advanced inputs footer button variants so the regression test does not depend on translated button text. - **Breaking**: None. - **Dependencies**: None. ## Root Cause `useNodeDrag.startDrag()` created and started `AutoPanController` immediately on pointerdown. When the Advanced inputs button was partly outside the canvas bounds, a stationary pointer near the visible canvas edge was enough for auto-pan to begin, even before any drag movement occurred. The pointer interaction layer already distinguishes press/hold from real dragging before calling `handleDrag()`. Deferring auto-pan to `handleDrag()` aligns auto-pan startup with that drag threshold and prevents a plain hold from panning the canvas. ## Review Focus - Auto-pan should not start from `startDrag()`/pointerdown alone. - Auto-pan should still start promptly once `handleDrag()` runs for an actual drag. - Repeated `handleDrag()` calls should reuse the existing controller rather than recreate it. - Existing `onPan` behavior should continue to update drag start positions, selected node start positions, selected groups, and node positions during active drags. - The E2E intentionally asserts the canvas offset, not node bounds, because the reported bug is unintended canvas auto-pan while the pointer is stationary. ## Red-Green Verification - `a00b5d2fb test: add failing advanced button hold pan regression`: adds the Playwright regression and test id plumbing. This was verified red against the pre-fix production code. - `5c207ae28 fix: defer node auto-pan until drag starts`: adds the production fix and unit coverage. The same regression is verified green with the fix. ## Validation - `pnpm format` - `pnpm lint` - `pnpm typecheck` - `pnpm typecheck:browser` - `pnpm test:unit` - `pnpm test:unit src/renderer/extensions/vueNodes/layout/useNodeDrag.test.ts` - `PLAYWRIGHT_SETUP_API_URL=http://127.0.0.1:8188 pnpm test:browser:local browser_tests/tests/vueNodes/interactions/node/move.spec.ts --grep "should not pan while holding the Advanced button without dragging"` - Pre-push hook: `pnpm knip --cache` ## Screenshots (Before / After) Before https://github.com/user-attachments/assets/6080de2d-e2da-4b38-a1ed-1f1f88548c2d After https://github.com/user-attachments/assets/f331271a-9ea1-41ec-92cb-974bc57be56b |
||
|
jaeone94
|
d129f757c0 |
fix: keep connected advanced inputs visible (#12652)
## Summary Keep connected advanced widget inputs visible on Vue-rendered nodes when advanced inputs are collapsed. This fixes Linear [FE-924](https://linear.app/comfyorg/issue/FE-924/bug-connected-advanced-input-parameters-become-hidden-when-advanced), where a user could connect a noodle to an advanced input, collapse advanced inputs, and then lose visual access to the connected parameter even though it was actively used by the workflow. ## Changes - **What**: Treat a widget-backed input as visible when its slot is linked, even if the widget is advanced and the node-level advanced section is collapsed. - **What**: Move Vue node widget rendering to use the processed `widget.visible` value instead of reimplementing visibility in `NodeWidgets.vue`. - **What**: Keep the visibility decision as a single source of truth during widget processing, including the existing deduplication path. - **What**: Add unit coverage for the new linked-widget visibility behavior and the precedence rule that explicit hidden state still wins. - **What**: Add an E2E regression that connects a `PrimitiveFloat` to the advanced `max_shift` input on `ModelSamplingFlux`, collapses advanced inputs, and verifies the connected input remains visible while an unconnected advanced input remains hidden. - **Breaking**: None. - **Dependencies**: None. ## Review Focus The key behavior is that linked advanced widgets should be promoted into the visible widget set only while they are connected. Explicitly hidden widgets must remain hidden even when linked. The fix uses existing slot metadata from `useGraphNodeManager`; no new graph state is introduced. This keeps the change scoped to Vue node widget processing and rendering. ## Red-Green Verification | Commit | Purpose | Local result | | --- | --- | --- | | `4fa5932c6` | Adds the E2E regression only | Red: `max_shift` was not found after collapsing advanced inputs | | `e5d1ee06a` | Adds the production fix and focused unit coverage | Green: targeted E2E passed | ## Test Plan - `pnpm test:unit src/renderer/extensions/vueNodes/composables/useProcessedWidgets.test.ts` - `PLAYWRIGHT_LOCAL=1 PLAYWRIGHT_TEST_URL=http://127.0.0.1:5175 PLAYWRIGHT_SETUP_API_URL=http://127.0.0.1:8188 pnpm test:browser -g "should keep connected advanced widgets visible when advanced inputs are hidden" browser_tests/tests/vueNodes/widgets/advancedWidgets.spec.ts` - `pnpm typecheck && pnpm typecheck:browser` ## Screenshots (Before / After) Before https://github.com/user-attachments/assets/bf1e88f3-2983-4bef-9cef-48ffe6dbfd6d After https://github.com/user-attachments/assets/4dee7766-0252-478f-9b1c-4b801fc20eb2 |
||
|
jaeone94
|
874b486640 |
fix: resolve missing resource error messages (#12646)
## Summary Resolve missing resource error groups through the error catalog so missing nodes, replaceable nodes, missing models, and missing media use consistent panel and single-error overlay copy. ## Changes - **What**: Adds missing-resource resolvers for `missing_node`, `swap_nodes`, `missing_model`, and `missing_media` that provide `displayMessage`, `toastTitle`, and `toastMessage` alongside the existing group titles. The Errors tab now renders a group-level `displayMessage` under non-execution group headers, which gives grouped missing-resource cards the same explanatory message path used by validation/runtime errors without adding per-row detail fields that these grouped cards do not need. - **What**: Moves missing node and swap node explanatory copy out of card-local hardcoded text and into `errorCatalog.missingErrors.*` keys. `MissingNodeCard` and `SwapNodesCard` now focus on rendering their grouped rows and actions, while the shared error group header owns the explanatory copy. - **What**: Adds environment-aware copy for missing node packs and missing models. Cloud messages explain unsupported resources and replacement/import paths without suggesting local execution, while OSS messages point users toward installing or downloading the missing resources. - **What**: Adds single-error overlay/toast copy for missing resources. Missing media uses a concise input-focused title/message, missing models distinguish Cloud unsupported models from OSS missing files, and missing nodes/swap nodes use node-type-aware copy. - **What**: Deduplicates missing node and swap node toast decisions by distinct node type so repeated instances of the same missing/replaceable node do not accidentally switch the single-error copy to plural copy. - **What**: Preserves representative missing media candidate metadata so missing media toast copy can use a human-readable node name such as `Load Image is missing a required media file.` - **What**: Removes unused missing-resource resolver fields such as grouped `displayDetails`, grouped `displayItemLabel`, and the unused `mediaTypes` source parameter after deciding those fields do not fit grouped missing-resource cards. - **Breaking**: None. - **Dependencies**: None. ## Review Focus - Missing resource groups are still grouped cards. This PR intentionally gives them group-level display and toast copy, but does not split missing resources into one error item per underlying candidate. - Missing resource count semantics are intentionally not normalized here. Error overlay totals, store counts, and grouped card counts still follow the existing behavior; a follow-up PR can define those count units separately. - The Cloud/OSS message variants remain explicit in the resolver instead of being abstracted into a generic variant helper. That keeps this PR focused on the messaging behavior and avoids a broader resolver refactor. - Only `src/locales/en/main.json` is updated directly. Other locales should be synced by the existing localization flow. ## Screenshots (if applicable) <img width="668" height="245" alt="스크린샷 2026-06-05 오전 3 16 49" src="https://github.com/user-attachments/assets/98b50ac3-67e1-438d-8c37-e06c7bf465ee" /> <img width="666" height="195" alt="스크린샷 2026-06-05 오전 3 16 58" src="https://github.com/user-attachments/assets/92da95b1-03d6-4739-97e6-c573982bfec9" /> <img width="505" height="358" alt="스크린샷 2026-06-05 오전 3 17 27" src="https://github.com/user-attachments/assets/4d0e1a6e-13b9-4097-9fb5-19fe0c5331dc" /> <img width="507" height="324" alt="스크린샷 2026-06-05 오전 3 17 44" src="https://github.com/user-attachments/assets/054e42f8-0d0c-44b5-8a67-e467fc04f1fc" /> ## Validation - `pnpm format` - `pnpm lint` - `pnpm test:unit src/platform/errorCatalog/errorMessageResolver.test.ts src/components/rightSidePanel/errors/useErrorGroups.test.ts src/components/rightSidePanel/errors/TabErrors.test.ts` - `pnpm typecheck` - push hook: `knip --cache` |
||
|
Matt Miller
|
8a819fa2be |
refactor(assets): read content hash from the canonical hash field (#12638)
## Summary The assets API exposes an asset's content hash as `hash`. An older `asset_hash` field was a deprecated alias carrying the same value. This PR moves the frontend fully onto `hash` and removes `asset_hash` from the frontend entirely. ## Changes - Read `asset.hash` (no `?? asset_hash` fallback) across the asset consumers: - `useMediaAssetActions` — widget-value variants + cloud-mode stored-filename resolution - `assetsStore` — input-asset-by-filename map - `assetMetadataUtils.getAssetUrlFilename` - `missingMedia` resolver/scan and `missingModel` scan hash matching - `useComboWidget` / `useWidgetSelectItems` - `assetPreviewUtil.findOutputAsset` now queries `/assets?hash=` instead of the deprecated `?asset_hash=` param and matches on `a.hash`. - Removed `asset_hash` from the zod asset schema and the local `AssetRecord` type. Responses that still include the alias parse cleanly — zod strips unknown keys — so the declared field protected nothing once the reads were gone. - Purged `asset_hash` from all test fixtures/mocks; tests key on the canonical `hash`. ## Safety / rollout The API currently emits **both** `hash` and `asset_hash` with identical values, so reading `hash` is safe today. This is the frontend half of retiring the alias; the backend stops emitting `asset_hash` only after this ships and old bundles age out, so there is no window where the field the UI reads is absent. ## Verification - `pnpm typecheck`: clean. - Affected unit tests pass (asset utils, store, media/model scans, widget composables). - `grep -rn asset_hash src/`: zero matches. |
||
|
jaeone94
|
f9cbaf750f |
fix: simplify error overlay messaging (#12598)
## Summary Simplifies the error overlay so it presents one clear title, one clear message, and one stable details action instead of rendering a list of per-error messages. ## Changes - **What**: Extracts the error overlay view model into `useErrorOverlayState`, adds focused unit coverage for the overlay copy resolution rules, and updates the overlay E2E coverage to match the new behavior. - **Breaking**: None. - **Dependencies**: None. ### Behavior changes - The overlay body no longer renders a `<ul>` of individual error messages. It now always renders a single paragraph message. - Single-error overlays now prefer toast-specific copy when it exists. For execution errors, the overlay resolves the message in this order: `toastMessage`, `displayMessage`, raw `message`, group `displayMessage`, then group `displayTitle`. The title resolves from `toastTitle`, then `displayTitle`, then the group title. - Single non-execution groups use group-level toast/display copy. This lets grouped error types supply overlay-friendly copy without the overlay needing to understand each card implementation. - Multiple-error overlays now ignore individual error item copy in the overlay itself. The header becomes the pluralized count title, for example `7 errors found`, and the body becomes the fixed guidance message: `Resolve them before running the workflow.` - The overlay is hidden if the store reports an error count but no resolved overlay message exists. This avoids rendering a visible shell with an empty body. - The action button no longer varies by error type in normal app mode. Missing nodes, missing models, missing media, swap nodes, validation errors, and runtime errors all use `View details` instead of labels like `Show missing nodes`, `Show missing models`, `Show missing inputs`, or `See Errors`. - App mode keeps its existing `Show errors in graph` action label. - The overlay width now keeps the previous width as its minimum and allows a wider maximum, reducing avoidable wrapping in longer error headers. - The live region was softened from an assertive alert-style announcement to `role="status"` with `aria-live="polite"` so updates such as count changes are less disruptive. ### Tests - Adds component coverage for the rendered overlay shape and app-mode action label. - Adds composable coverage for single execution errors, runtime errors, grouped missing media errors, multiple-error aggregate copy, hidden empty-message state, and display-copy fallback behavior. - Updates `errorOverlay.spec.ts` so the E2E suite checks the new single-message overlay, the stable `View details` action, and the fixed multiple-error body guidance. - Removes the old type-specific button-label E2E expectations because that branch no longer exists in product behavior. ### Follow-up PR A follow-up PR is stacked on top of this one: `jaeone/fe-816-missing-resource-error-messaging`. That follow-up will wire missing resource error resolvers into the copy model consumed here. It covers missing node packs, missing models, missing media, and swap-node groups, including the group-level `toastTitle`, `toastMessage`, `displayMessage`, `displayDetails`, and item label copy those cards need. This PR intentionally keeps the overlay behavior separate so it can merge first without depending on the missing-resource resolver copy. ## Review Focus - Please check the single-error versus multiple-error overlay behavior, especially the fallback order for execution error copy. - Please check that the `View details` action is now intentionally error-type agnostic in normal app mode while app mode keeps `Show errors in graph`. - Please check the empty-message guard and the requirement that a single-error overlay only resolves a single group when the total error count and group list agree. - Please check the E2E reduction: the old type-specific action-label assertions were removed because the UI branch they tested was removed. ## Screenshots (if applicable) N/A |
||
|
jaeone94
|
4bfb0c36be |
Fix Cloud media input defaults (#12562)
## Summary Fix Cloud media loader widgets so `LoadImage`, `LoadVideo`, and `LoadAudio` resolve their default values from Cloud input assets instead of blindly accepting backend `object_info` combo options. When no matching Cloud input asset exists, the widgets now start empty instead of selecting a server-only value that immediately trips missing-input detection. ## Changes - **What**: Cloud media input widgets now derive their available values from `assetsStore.inputAssets`, filtered to the node's media type and to assets with a valid `asset_hash`. - **What**: Cloud media defaults now prefer an explicit default only when it matches an available Cloud input asset hash or name, then fall back to the first matching Cloud input asset, and otherwise use an empty value. - **What**: The media path keeps the existing model-widget implementation style by resolving Cloud asset state through the store internally, while preserving media-specific hash/name matching because media widgets submit Cloud `asset_hash` values. - **What**: Added regression coverage for adding empty Cloud `LoadImage`, `LoadVideo`, and `LoadAudio` nodes when backend `object_info` advertises server-only media options. - **What**: Expanded media widget unit coverage for image/video/audio inputs, empty defaults, first-asset fallback, default-by-hash matching, default-by-name matching, hashless asset filtering, unrelated media filtering, dynamic values, option labels, lazy input loading, and `control_after_generate` wiring. - **What**: Kept existing OSS and Cloud runtime missing-media E2E coverage, with Cloud fixtures consistently using the local setup backend for `object_info` so tests do not depend on live Cloud backend startup details. - **Breaking**: None. - **Dependencies**: None. ## Review Focus - Cloud media loader defaults should no longer be sourced from backend file lists unless the value also corresponds to a Cloud input asset. - Empty Cloud input-asset libraries should produce empty media widget values, not missing-input errors at node creation time. - Model asset-browser behavior is intentionally unchanged. The media path mirrors the store-access style, but media defaults still resolve to asset hashes because those are the values submitted by Cloud media widgets. - The Cloud E2E fixture stubs bootstrap endpoints and routes `object_info` through the local setup backend. This keeps the test focused on frontend behavior while still using realistic node definitions. ## Testing - `pnpm exec oxfmt browser_tests/tests/propertiesPanel/errorsTabMissingMediaRuntime.spec.ts src/renderer/extensions/vueNodes/widgets/composables/useComboWidget.test.ts src/renderer/extensions/vueNodes/widgets/composables/useComboWidget.ts` - `pnpm lint` - `pnpm typecheck` - `pnpm typecheck:browser` - `pnpm exec vitest run src/renderer/extensions/vueNodes/widgets/composables/useComboWidget.test.ts` - `PLAYWRIGHT_TEST_URL=http://127.0.0.1:5174 PLAYWRIGHT_SETUP_API_URL=http://127.0.0.1:8188 pnpm exec playwright test browser_tests/tests/propertiesPanel/errorsTabMissingMediaRuntime.spec.ts --project=chromium --workers=1` - `PLAYWRIGHT_TEST_URL=http://127.0.0.1:5175 PLAYWRIGHT_SETUP_API_URL=http://127.0.0.1:8188 pnpm exec playwright test browser_tests/tests/propertiesPanel/errorsTabMissingMediaRuntime.spec.ts --project=cloud --workers=1` - `git diff --check` - `pnpm knip` - `.claude/skills/reviewing-unit-tests/SKILL.md` red-flag review ## Screenshots Before https://github.com/user-attachments/assets/5df04036-d15c-4f94-bdcd-df8b26a29329 After https://github.com/user-attachments/assets/abe7caf5-a83b-4960-aa6f-65a377424a85 |
