mirror of
https://github.com/Comfy-Org/ComfyUI_frontend.git
synced 2026-05-13 17:26:22 +00:00
b2bba78ce07400a6915a41bb5626fb284f927dfc
1756 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
Christian Byrne
|
b2bba78ce0 |
test: add unit tests for usePanAndZoom composable (#11391)
## Summary Add 29 unit tests for the `usePanAndZoom` composable to improve mask editor test coverage. ## Changes - **What**: New test file `src/composables/maskeditor/usePanAndZoom.test.ts` covering all public API methods ## Review Focus Test coverage spans: - `initializeCanvasPanZoom` — landscape/portrait fit-to-view, panel width accounting, style application - `handlePanStart`/`handlePanMove` — panning state, offset updates, error on missing start - `zoom` — wheel in/out, clamping (0.2–10.0), missing canvas early return, cursor update - `updateCursorPosition` — pan offset application - `invalidatePanZoom` — missing image warning, store container fallback, rgbCanvas dimension sync - Touch handlers — single/two-finger start, double-tap undo, pen blocking, single-touch pan, pinch zoom, touch end states - `addPenPointerId`/`removePenPointerId` — add, dedup, remove, no-op ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-11391-test-add-unit-tests-for-usePanAndZoom-composable-3476d73d36508104b447d87471ce021b) by [Unito](https://www.unito.io) --------- Co-authored-by: Terry Jia <terryjia88@gmail.com> Co-authored-by: GitHub Action <action@github.com> |
||
|
Dante
|
3340b77908 |
test: add unit tests for value-control widget family (#11440)
## Summary Adds 42 unit tests across 5 files covering the value-control widget family — first batch of a broader effort to raise widget-test coverage. ## Changes - **What**: - `WidgetInputNumber.test.ts` (9) — variant selection by widget.type (int/float/slider/gradientslider), controlWidget wrapping in WidgetWithControl, modelValue forwarding. - `WidgetInputNumberGradientSlider.test.ts` (11) — initial value, min/max/disabled pass-through, default vs custom gradient stops, precision-derived step, WidgetLayoutField wrapping. - `WidgetWithControl.test.ts` (5) — renders passed component with widget/modelValue, initializes ValueControlButton mode from widget.controlWidget.value, calls controlWidget.update on mode change. - `ValueControlButton.test.ts` (11) — i18n aria-label per mode, text vs icon rendering, pointer and keyboard activation, `type="button"` safety. - `ValueControlPopover.test.ts` (6) — BEFORE/AFTER copy from settingStore, four option render, v-model updates on selection. ## Review Focus - Stack follows the existing widget-test pattern (`@testing-library/vue` + PrimeVue + `createI18n` where needed, no `@vue/test-utils`). - `createMockWidget` from `widgetTestUtils.ts` reused; no new helper extracted (YAGNI — per-file `renderComponent` stays ~10 lines). - `WidgetWithControl` watcher test asserts first-arg of `update` since the vue watch callback passes `(newVal, oldVal, onCleanup)`. - No changes to any widget component source — tests-only PR. This is one of several focused PRs in a widget-test-coverage sequence; subsequent PRs cover form-dropdown internals, utility widgets, media/graph/canvas widgets, and e2e value-type specs. ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-11440-test-add-unit-tests-for-value-control-widget-family-3486d73d3650813891e1fe8d45eaecaf) by [Unito](https://www.unito.io) --------- Co-authored-by: GitHub Action <action@github.com> Co-authored-by: bymyself <cbyrne@comfy.org> |
||
|
pythongosssss
|
4c892341e4 |
feat: Node search UX updates (#9714)
## Summary Addresses feedback from the initial v2 node search implementation for improved UI and UX ## Changes - **What**: - add root filter buttons - remove all extra tree categories leaving only "Most relevant" - replace input/output selection with popover - replace price badge with one from node header - add chevrons and additional styling to category tree - hide empty categories - fix bug with hovering selecting item under mouse automatically - fix tailwind merge with custom sizes removing them - keyboard navigation - general tidy/refactor/test ## Screenshots (if applicable) https://github.com/user-attachments/assets/db798dfa-e248-4b48-bb56-2fa7b6c5f65f ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-9714-feat-Node-search-UX-updates-31f6d73d365081cebd96c4253ad1ca53) by [Unito](https://www.unito.io) --------- Co-authored-by: GitHub Action <action@github.com> |
||
|
Terry Jia
|
0f66f76b87 |
test: add unit tests for mask editor control components (#11642)
## Summary
Add unit tests for the three mask editor control components
(`DropdownControl`, `SliderControl`, `ToggleControl`), raising each from
partial (20% / 37.5% / 44.4%) to 100% across all coverage dimensions.
## Changes
- **What**: Add three sibling test files under
`src/components/maskeditor/controls/`:
- `DropdownControl.test.ts` (5 tests): label render, normalization of
`string[]` options into `{label,value}`, pass-through of `{label,value}`
options, `modelValue` reflected as the selected option,
`update:modelValue` emitted on change.
- `SliderControl.test.ts` (4 tests): label render,
`min`/`max`/`step`/`modelValue` exposed on `<input type="range">`,
`step` defaults to `1` when omitted, `update:modelValue` emitted as a
`number` (not string) on input.
- `ToggleControl.test.ts` (5 tests): label render, `modelValue`
reflected as `checked`, `update:modelValue` emitted as `true` / `false`
on toggle.
## Review Focus
- Stack matches the project's prevailing Vue test pattern
(`@testing-library/vue` + `@testing-library/user-event`, e.g.
`Badge.test.ts`, `BatchCountEdit.test.ts`).
- `update:modelValue` is asserted via the `'onUpdate:modelValue'`
callback prop rather than `emitted()` — keeps tests focused on
observable behavior and avoids reaching into the wrapper.
- `SliderControl` uses `fireEvent.input` instead of
`userEvent.type/clear`: `<input type="range">` is non-editable for
`userEvent` (`clear() is only supported on editable elements`). The
single eslint-disable for `testing-library/prefer-user-event` is
annotated with the reason.
- `DropdownControl` test guards both branches of the `string` →
`{label,value}` normalization (string array path and pre-normalized
object array path), since that's the only meaningful logic in the file.
- Style aligned with sibling tests: `should ...` naming, per-file
`renderComponent` helper accepting a `props` override and an `onUpdate`
callback parameter.
┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-11642-test-add-unit-tests-for-mask-editor-control-components-34e6d73d3650812aba2ae34a760489e2)
by [Unito](https://www.unito.io)
|
||
|
Terry Jia
|
7bfbd0d7f3 |
test: add unit tests for mask editor settings panels (#11647)
## Summary
Add unit tests for the five mask editor settings panel components,
raising each from 0–35% to ~98–100% across coverage dimensions.
## Changes
- **What**: Add 5 sibling test files under `src/components/maskeditor/`
(47 tests total):
- `PaintBucketSettingsPanel.test.ts` (4): both sliders bind to / write
through to `setPaintBucketTolerance` / `setFillOpacity`. **100%**.
- `ColorSelectSettingsPanel.test.ts` (8): all 7 controls (4 sliders, 2
toggles, 1 dropdown) bind to and write through to the right store
fields/setters; method dropdown casts to `ColorComparisonMethod`.
**100%**.
- `BrushSettingsPanel.test.ts` (13): shape buttons (Arc/Rect), reset to
default, four numeric inputs (size/opacity/hardness/step), logarithmic
size slider including the cached-raw-value path
(`Math.round(Math.pow(250, x))`), color input v-model, color input ref
forwarded to / cleared from store on mount/unmount.
- `ImageLayerSettingsPanel.test.ts` (17): mask opacity slider + canvas
style sync, blend-mode select with all 3 enum values + default fallback,
three layer-visibility checkboxes (with and without canvas refs),
activate-layer button forwarding to `toolManager.setActiveLayer`,
disabled-when-active and "show paint button only when Eraser" branches,
base image preview src binding.
- `SettingsPanelContainer.test.ts` (5): tool → component routing
(`MaskBucket` → bucket panel, `MaskColorFill` → color panel, anything
else → brush panel).
## Review Focus
- Stack matches sibling control tests (`@testing-library/vue` +
`userEvent` + stub child controls). Each panel's child `SliderControl` /
`ToggleControl` / `DropdownControl` is replaced by a minimal `<button>`
stub that emits `update:modelValue` on click, so panel logic is
decoupled from control internals (already covered by their own tests).
- Two panels (`Brush`, `ImageLayer`) need real reactivity
(`brushSettings.size` changes through a setter must propagate to the
slider's modelValue computed; `currentTool` / `activeLayer` mutations
between tests). They use `reactive()` from Vue at top level + a `let
mockStore` reset in `beforeEach`. Plain object mocks would either
silently no-op or leak state between tests.
- The two reactive panels enable file-level `eslint-disable
testing-library/no-container, testing-library/no-node-access` because
the unlabeled DOM (shape divs, unlabeled `<input type="number">`/`<input
type="color">`/`<input type="checkbox">`/`<select>`) genuinely can't be
queried via `screen.getByRole/Label`. Each disable has a comment
explaining why.
- `BrushSettingsPanel` has a non-trivial cached-value branch in
`brushSizeSliderValue` computed: when `rawSliderValue` and `brushSize`
are in sync, the getter returns the raw float instead of recomputing
`log(size)/log(250)`. Test stubs `setBrushSize` to actually write back
so the next read takes the cached path.
- `ImageLayerSettingsPanel` has a `display: 'block' | 'none'` style
binding. happy-dom doesn't populate `el.style.display` from inline style
strings reliably, so the test asserts via `el.getAttribute('style')`
instead.
- `SettingsPanelContainer` uses inline component stubs that render
unique text — assertion is just `textContent.toContain('brush-panel')`
etc. No need for component-instance probing.
- Style aligned with sibling tests: `should ...` naming, `describe`
grouped by feature (button group, slider, etc.), `vi.hoisted` for mock
state where reactivity isn't required.
┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-11647-test-add-unit-tests-for-mask-editor-settings-panels-34e6d73d36508117911bc0850ce085e1)
by [Unito](https://www.unito.io)
|
||
|
Terry Jia
|
32b266f3e9 |
test: add unit tests for SidePanel, MaskEditorButton, and BrushCursor (#11648)
## Summary
Add unit tests for three small mask editor Vue components (`SidePanel`,
`MaskEditorButton`, `BrushCursor`), all reaching **100%** across
coverage dimensions.
## Changes
- **What**: Add 3 sibling test files (17 tests total):
- `SidePanel.test.ts` (3): renders both `SettingsPanelContainer` and
`ImageLayerSettingsPanel`; forwards `toolManager` prop through to
`ImageLayerSettingsPanel`; works with the prop omitted.
- `MaskEditorButton.test.ts` (3): renders with localized `aria-label`
when `isSingleImageNode` is true; hidden via `v-show` (style `display:
none`) when false; click executes `Comfy.MaskEditor.OpenMaskEditor`
command.
- `BrushCursor.test.ts` (11): opacity 1 / 0 from `brushVisible`; size =
2 × effective brush size × zoom (with hardness scaling); border-radius
50% / 0% for Arc / Rect; position from `cursorPoint + panOffset -
radius`, with optional `containerRef.getBoundingClientRect` offset;
gradient preview hidden / shown; flat fill at `effectiveHardness === 1`.
## Review Focus
- `SidePanel` test stubs both child panels to bare `<div data-testid>`
so the test verifies wiring (prop forwarding, child rendering) without
being affected by the children's i18n / store dependencies.
- `MaskEditorButton` mocks `useCommandStore.execute` and
`useSelectionState.isSingleImageNode` (as a Vue ref). Real i18n via
`createI18n` + `globalInjection: true` — the template uses `$t(...)`
which requires global injection in composition mode.
- For the v-show hidden case, `getByLabelText('...', { selector:
'button' })` works where `getByRole('button', { hidden: true })` doesn't
reliably resolve through the `<Button>` wrapper component.
- `BrushCursor` uses `reactive()` mock store and queries the brush /
gradient elements by id (`#maskEditor_brush`,
`#maskEditor_brushPreviewGradient`). File-level eslint-disable for
`testing-library/no-node-access` because the component's anchor elements
are styled divs without ARIA roles or labels.
- The `radial-gradient` (hardness < 1) branch of `gradientBackground` is
intentionally **not** asserted via rendered DOM: the computed returns a
multi-line template literal that happy-dom's CSS parser drops entirely.
The math (`getEffectiveBrushSize` / `getEffectiveHardness`) is covered
by `brushUtils.test.ts`. v8 reports 100% branch coverage because Vue
evaluates the computed regardless of whether the resulting style string
is parsed by the DOM.
- Style aligned with sibling tests: `should ...` naming, `describe`
grouped by feature, `vi.hoisted` for command-store / selection-state
mocks, real i18n via `createI18n`.
┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-11648-test-add-unit-tests-for-SidePanel-MaskEditorButton-and-BrushCursor-34e6d73d365081e38c4afee32ddf2b0b)
by [Unito](https://www.unito.io)
|
||
|
Terry Jia
|
2d4ca9c387 |
test: add unit tests for PointerZone and ToolPanel (#11649)
## Summary
Add unit tests for `PointerZone` and `ToolPanel` mask editor components,
raising each from 0% to **91.89% / 100%** respectively.
## Changes
- **What**: Add 2 sibling test files (24 tests total):
- `PointerZone.test.ts` (13): mount exposes root element to
`store.pointerZone`; pointer events (down/move/up) forward to
`toolManager.handlePointer*`; `pointerleave` hides brush + clears
cursor; `pointerenter` calls `toolManager.updateCursor`; touch events
(start/move/end) forward to `panZoom.handleTouch*`; wheel zooms then
updates cursor with wheel coords; `isPanning` watcher sets cursor to
"grabbing" then back via `updateCursor`; contextmenu's default is
prevented.
- `ToolPanel.test.ts` (11): one container rendered per `allTools`; icon
HTML for each tool; current tool highlighted with
`maskEditor_toolPanelContainerSelected` class while others are not;
clicking a container calls `toolManager.switchTool` with the matching
enum value; zoom indicator rounds `displayZoomRatio * 100`; dimensions
text reflects `image.width × image.height` (or empty when no image);
clicking the zoom indicator calls `store.resetZoom`.
## Review Focus
- `PointerZone` mocks `useToolManager` / `usePanAndZoom` to plain
function bags via factory helpers. The component is mostly forwarding,
so the test's value is in *event mapping* (which event triggers which
handler), not handler internals.
- happy-dom doesn't propagate `clientX` / `clientY` through the
`WheelEvent` constructor; the wheel test sets them via
`Object.defineProperty` after construction. Without this,
`updateCursorPosition` reads `undefined`.
- `PointerZone` ends at 91.89% statements / 70% branch — uncovered lines
are the `onMounted` early-return when `pointerZoneRef.value` is
`undefined` (always set in tests) and the watcher's same guard. Both
unreachable in normal use, intentionally not covered.
- `ToolPanel` mocks `iconsHtml` to `<svg data-testid="icon-${tool}" />`
markers, letting tests assert per-tool rendering via `getByTestId`. Real
i18n via `createI18n` for the reset-zoom tooltip text.
- `getToolContainers` queries by class because tool buttons are
unlabeled `<div>`s with click handlers (no role / aria); a file-level
`eslint-disable testing-library/no-node-access` covers this and the
dimensions-span placeholder query.
- Style aligned with sibling tests: `should ...` naming, `describe`
grouped by feature, `reactive()` mock store + `let mockStore` reset in
`beforeEach`, real i18n where keys are user-visible.
┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-11649-test-add-unit-tests-for-PointerZone-and-ToolPanel-34e6d73d3650817daf85c0d33d16899d)
by [Unito](https://www.unito.io)
|
||
|
Terry Jia
|
6f6fc88b0f |
test: add unit tests for MaskEditorContent main container (#11651)
## Summary
Add unit tests for `MaskEditorContent` (the mask editor's main
orchestration container), raising coverage from 0% to **94.11% / 83.72%
/ 83.33% / 94.11%** (statements / branches / functions / lines).
## Changes
- **What**: Add `src/components/maskeditor/MaskEditorContent.test.ts`
(12 tests) covering:
- **Mount**: keyboard listeners attached, ResizeObserver observes the
container, all 5 canvas refs assigned to the store before init runs.
- **Init flow**: `loader.loadFromNode` → `imageLoader.loadImages` →
`panZoom.initializeCanvasPanZoom` → `canvasHistory.saveInitialState` →
`brushDrawing.initGPUResources` → `initPreviewCanvas` chain runs in
order; child UI (`ToolPanel` / `PointerZone` / `SidePanel` /
`BrushCursor`) only renders after init succeeds; GPU preview canvas
resolution matches the mask canvas.
- **Init errors**: rejection from `loader.loadFromNode` or
`panZoom.initializeCanvasPanZoom` is caught, logged, and triggers
`dialogStore.closeDialog()`.
- **ResizeObserver**: callback invokes `panZoom.invalidatePanZoom()`
(captured the constructor argument to call it manually).
- **Drag**: `Ctrl+drag` is preventDefault'd; plain drag is not.
- **Unmount**: cleanup runs `brushDrawing.saveBrushSettings`,
`keyboard.removeListeners`, `canvasHistory.clearStates`,
`store.resetState`, `dataStore.reset`.
## Review Focus
- Heavy mock surface (10 modules): the 3 stores, 5 composables, plus 4
child Vue components and `LoadingOverlay`. All mocks are `vi.hoisted`
module-level. `mockStore` is `reactive()` because the source mutates
`activeLayer` (visible in template binding), `maskCanvas`, etc.; the
rest are plain function bags.
- Child components are stubbed to bare `<div data-testid>` so init
reveal can be asserted via `screen.findByTestId(...)` without engaging
their real implementations (each has its own test file).
- `MockResizeObserver` captures the constructor callback in module-level
`lastResizeCallback`. The "invalidate on resize" test invokes it
manually with empty args — that's enough to exercise the source's `if
(panZoom) { await panZoom.invalidatePanZoom() }` branch since the
callback only consumes `panZoom` from closure.
- happy-dom doesn't propagate `ctrlKey` through the `DragEvent`
constructor, so the drag tests set it via `Object.defineProperty(event,
'ctrlKey', { value })` (same pattern used in `PointerZone.test.ts` for
wheel `clientX/Y`).
- 94.11% line coverage — the two uncovered blocks (`containerRef`
missing, canvas refs missing) are early-return error paths unreachable
when Vue successfully mounts; not worth constructing a fixture to
trigger.
- Style aligned with sibling tests: `should ...` naming, `describe`
grouped by feature, `vi.hoisted` mocks reset via `beforeEach`,
`screen.findByTestId` for async render assertions.
┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-11651-test-add-unit-tests-for-MaskEditorContent-main-container-34e6d73d365081b38af2e057cb7daf9e)
by [Unito](https://www.unito.io)
|
||
|
Terry Jia
|
492bec28c8 |
test: add unit tests for TopBarHeader (#11650)
## Summary
Add unit tests for `TopBarHeader` mask editor dialog component, raising
coverage from 0% to **100%** across statements, branches, functions, and
lines.
## Changes
- **What**: Add `src/components/maskeditor/dialog/TopBarHeader.test.ts`
(17 tests) covering:
- Localized title rendering.
- Undo / Redo buttons forward to `store.canvasHistory.{undo,redo}`.
- Four transform buttons (rotate left / right, mirror horizontal /
vertical) call the matching `canvasTransform` action — parametrized via
`it.each`.
- All four transform error paths: rejected promise is caught, swallowed,
and logged with the right `[TopBarHeader] ... failed:` prefix.
- Invert calls `canvasTools.invertMask`; Clear calls both
`canvasTools.clearMask` and `store.triggerClear`.
- Save: hides brush, awaits `saver.save()`, closes the dialog on
success; switches button text to "Saving" while in-flight; restores
brush + button label and logs on save failure.
- Cancel: closes the dialog with the `global-mask-editor` key.
## Review Focus
- All five composable / store dependencies are mocked at module level
via `vi.hoisted`: `useMaskEditorStore`, `useDialogStore`,
`useCanvasTools`, `useCanvasTransform`, `useMaskEditorSaver`. Only the
store needs `reactive()` (`brushVisible` flips during save flow); the
rest are plain function bags.
- `Button.vue` is stubbed to a thin `<button :disabled>` so role queries
(`getByRole('button', { name: ... })`) resolve cleanly without dragging
in the real UI button's classes / variants.
- Real i18n via `createI18n`. Icon buttons rely on `:title` for their
accessible name; text buttons rely on slot text — both are reachable
through `getByRole('button', { name: ... })`.
- The "Saving" text test uses an unresolved promise to keep the
in-flight state observable; `waitFor` + `void user.click(...)` lets us
assert without awaiting the click. The dangling promise resolves at the
end so vitest doesn't complain.
- `it.each` parametrizes the four transform success paths and
(separately) the four error paths, keeping the file tight.
- Style aligned with sibling tests: `should ...` naming, `describe`
grouped by feature, `vi.hoisted` for cross-test mocks, real i18n with
`createI18n`.
┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-11650-test-add-unit-tests-for-TopBarHeader-34e6d73d365081adab66e460cf56accb)
by [Unito](https://www.unito.io)
|
||
|
Terry Jia
|
ba6dd2a09c |
refactor(load3d): extract viewport math to load3dViewport (#11624)
## Summary Pull two pure helpers out of `Load3d` into a sibling module. Mechanical refactor — no behavior change. Second of four small PRs splitting up the https://github.com/Comfy-Org/ComfyUI_frontend/pull/11495. ## Changes - **What**: New `load3dViewport.ts` exports two pure functions: - `computeLetterboxedViewport({ width, height }, targetAspectRatio)` returns `{ offsetX, offsetY, width, height }` — the aspect-ratio fitting math that was duplicated inline in three places (`renderMainScene`, `setBackgroundImage`, `handleResize`). - `isLoad3dActive(flags)` consumes the activity-flag struct used by the rAF tick gate; `Load3d.isActive()` now delegates to it. ## Review Focus - The three call sites in `Load3d.ts` produce byte-identical viewport rectangles for any `(containerWidth, containerHeight, targetAspect)` triple — same branch on aspect-ratio comparison, same offset derivation. Simplest way to verify: diff the math. - `isLoad3dActive` ORs the same six flags as before in the same order. Old implementation read `this.STATUS_MOUSE_ON_NODE` etc. directly; new one reads them through a flags object built at the call site. - 13 unit tests cover the helpers: the "wider" / "taller" / "matching" aspect cases for letterboxing, and each individual flag flipping `isLoad3dActive` on by itself. ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-11624-refactor-load3d-extract-viewport-math-to-load3dViewport-34d6d73d365081ab9af5fc445bf5bd5a) by [Unito](https://www.unito.io) --------- Co-authored-by: GitHub Action <action@github.com> |
||
|
Kelly Yang
|
4a9001f675 |
test: add E2E tests for image crop widget Levels 4-7 (#11571)
## Summary
Adds 12 Playwright E2E tests for the `ImageCropV2` widget covering
aspect ratio selection, lock/unlock behavior, constrained resize, and
BoundingBox numeric input — all of which had zero test coverage.
## Changes
**Level 4 — Aspect Ratio Selection** (`with source image after
execution`)
- Selecting 16:9 preset adjusts crop height proportionally via
`applyLockedRatio`
- Selecting Custom unlocks the ratio and restores all 8 resize handles
**Level 5 — Lock/Unlock** (`without source image` + `with source image
after execution`)
- Selecting a preset auto-enables the lock (aria-label changes to
"Unlock aspect ratio")
- Unlocking after a preset reverts the dropdown display to "Custom"
- Full lock→unlock round-trip verifies handle count (4 → 8) and
aria-label on both transitions
**Level 6 — Constrained Resize** (`with source image after execution`)
- NW corner drag grows origin (x, y decrease) and dimensions while
maintaining ratio
- SE corner drag beyond image edge clamps to boundary
- NW corner drag beyond (0, 0) clamps x/y to image boundary
- Inward SE corner drag enforces `MIN_CROP_SIZE` (16px minimum)
**Level 7 — BoundingBox Numeric Input** (`with source image after
execution`)
- X increment button increments crop x by 1
- Width increment button increments crop width by 1
- BoundingBox inputs reflect updated position after a drag
No source code was modified.
<!-- CURSOR_SUMMARY -->
---
> [!NOTE]
> **Low Risk**
> Low risk: changes are limited to Playwright E2E coverage plus adding
`data-testid` attributes to BoundingBox inputs, with no behavioral logic
changes expected.
>
> **Overview**
> **Expands E2E coverage for the `ImageCropV2` widget** by adding new
Playwright tests for ratio preset selection, lock/unlock behavior,
constrained resizing (including boundary clamping and min size), and
BoundingBox numeric input updates.
>
> **Improves testability of BoundingBox controls** by adding
`data-testid` attributes to the `WidgetBoundingBox.vue`
`ScrubableNumberInput` fields (`x`, `y`, `width`, `height`) so E2E tests
can target increment/input elements reliably.
>
> <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit
|
||
|
Dante
|
9cf035879f |
test: add WidgetCurve unit tests (#11469)
## Summary Splits the WidgetCurve test coverage out of #11446 so this widget can be reviewed independently. ## Changes - **What**: Adds WidgetCurve unit tests covering point forwarding, interpolation updates, disabled-state behavior, and upstream value handling. ## Review Focus Focused test-only PR extracted from #11446. Validated with `pnpm test:unit -- --run src/components/curve/WidgetCurve.test.ts`. ## Screenshots (if applicable) N/A ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-11469-test-add-WidgetCurve-unit-tests-3486d73d365081c2a68bc8403fa0265f) by [Unito](https://www.unito.io) |
||
|
pythongosssss
|
125c11b3d0 |
fix: translate blueprint label (#11573)
## Summary Fixes hardcoded "Blueprint" text and adds e2e coverage to test visibility, resolving #11473 ## Changes - **What**: - add translation - add test ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-11573-fix-translate-blueprint-label-34b6d73d365081009215e06be6aa1fa0) by [Unito](https://www.unito.io) |
||
|
jaeone94
|
17d980dbc8 |
feat: add inline-CTA nightly survey for error panel (#11591)
## Summary Adds an inline-CTA Typeform survey for the redesigned error panel, targeting nightly localhost users. Reuses the existing node-search survey infrastructure rather than introducing a parallel stack. ## Changes - **What**: - `surveyRegistry` gains optional `presentation: 'floating' | 'inline-cta'` and a `getFloatingSurveys()` helper; controller filters by it. - `NightlySurveyPopover` accepts `mode` + `v-model:open`. Manual mode skips the eligibility watcher, drops the "Not Now" button, and leaves open/close/markSeen to the parent. - New `ErrorPanelSurveyCta.vue` renders a CTA in the error tab footer once `useExecutionErrorStore.hasAnyError` has transitioned `null → value` at least 3 times. - Popover lives in `NightlySurveyController` (session-persistent). Shared state via module-level singleton (`useErrorSurveyPopoverState`) so the iframe survives error-tab unmounts during workflow switches. - `useTypeformEmbed` centralises script loading (singleton Promise, 10s timeout, explicit `window.tf.load()` on each new container). Necessary because the embed's DOMContentLoaded auto-scan only fires once; late consumers need an explicit re-scan to render. - CTA and feedback-gate strings added under `errorPanelSurvey.*`. ## Review Focus - Manual-mode flow in `NightlySurveyPopover.vue` — CTA click is routed through the module singleton; popover stays mounted after the first open (`hasOpenedOnce` + `v-show`) to preserve the iframe across repeated open/close cycles and workflow switches. - `useTypeformEmbed.ts` — the `window.tf.load()` trick (verified against the CDN `embed.js`) is what lets two surveys coexist; without it Typeform's one-shot DOM scan misses whichever element mounts second. - `NightlySurveyController.vue` guards against double-mount by requiring `presentation === 'inline-cta'` on `errorPanelConfig`. - Scope is nightly+localhost only (`isNightlyLocalhost` in `useSurveyEligibility`); async component gate in `TabErrors.vue` keeps this out of Cloud/Desktop/stable bundles. ## Test plan - [x] `IS_NIGHTLY=true pnpm dev`, clear `Comfy.SurveyState` + `Comfy.FeatureUsage`, trigger 3 failed runs → CTA appears in error tab footer. - [x] Click "Give feedback" → Typeform popover opens and renders the form. - [x] Close popover, switch workflow (error tab unmounts), trigger a new error → CTA reappears and reopening the popover shows the same iframe. - [x] Open error-panel popover, close, then trigger 3 node searches → node-search auto-popup renders its own iframe correctly (two surveys coexist). - [x] CTA × dismisses and persists (`seenSurveys['error-panel']`). - [x] "Don't ask again" inside popover sets `optedOut: true` and hides all nightly surveys. - [x] Cloud/Desktop/stable builds: CTA never renders, controller's manual popover doesn't mount. ## Screenshot https://github.com/user-attachments/assets/91145f23-fd1e-4caf-b6cc-4b97d33ed6b7 ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-11591-feat-add-inline-CTA-nightly-survey-for-error-panel-34c6d73d3650817d9f95fddcf64633de) by [Unito](https://www.unito.io) |
||
|
Kelly Yang
|
bba3c38ae1 |
test: E2E coverage for painter widget (Levels 1–5) (#11551)
## Summary
Adds 7 new E2E tests for the Painter widget covering the remaining
untested items from Levels 1–5 of the widget test plan. All new tests
pass typecheck, lint, and the full pre-commit hook suite.
## Changes
**`browser_tests/tests/painter.spec.ts`**
- **Level 1.2** — assert node size ≥ 450×550 via the graph API to verify
the painter extension enforces its minimum dimensions
- **Level 1.3** — assert `width`, `height`, and `bg_color` widgets have
`options.hidden = true` so they don't appear as standard widget controls
- **Level 2.4** — cursor element becomes visible on pointer enter, its
CSS transform updates as the mouse moves across the canvas, and it hides
on pointer leave; uses `expect.poll` on transform to avoid the Vue
microtask race
- **Level 4.2** — set brush color to `#ff0000` via input event, draw a
stroke, sample a 40×40 region at canvas center and assert red pixels (R
> 200, G < 50, B < 50)
- **Level 4.3** — set opacity to 50%, draw a stroke, sample pixel alpha
values and assert semi-transparency (50 < α < 230)
- **Level 4.4** — focus the hardness slider, press ArrowLeft ×10, assert
the display value changes from `100%` to `90%`
- **Level 5.4** — set background color input to `#ff0000` via input
event, assert the canvas container div has `background-color: rgb(255,
0, 0)`
**`src/components/painter/WidgetPainter.vue`**
- Added `data-testid="painter-canvas-container"` to the inner canvas
wrapper div
- Added `data-testid="painter-cursor"` to the brush cursor div
- Added `data-testid="painter-bg-color-row"` to the background color
control row
- Added `data-testid="painter-hardness-value"` to the hardness display
span (mirrors the existing `painter-size-value` pattern)
<!-- CURSOR_SUMMARY -->
---
> [!NOTE]
> **Low Risk**
> Low risk: changes are limited to Playwright E2E coverage plus a few
`data-testid` attributes to stabilize selectors, with no functional
logic changes to the painter behavior.
>
> **Overview**
> Adds **7 new Playwright E2E tests** for the Painter widget, covering
minimum node sizing/hidden standard widgets, cursor
visibility/positioning behavior, brush hardness display updates,
color/opacity effects on rendered strokes, and background color
application.
>
> Updates `WidgetPainter.vue` to add a handful of **`data-testid`
hooks** (canvas container, cursor, background color row, hardness value)
used by the new tests.
>
> <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit
|
||
|
pythongosssss
|
cc73baaf57 |
test: Test subgraph breadcrumbs (#11472)
## Summary Add test coverage to subgraph breadcrumbs ## Changes - **What**: - Add subgraph breadcrumb helpers - Add tests for entering/navigating/menu/collapsing ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-11472-test-Test-subgraph-breadcrumbs-3486d73d365081e1a75bf57404eaa63b) by [Unito](https://www.unito.io) |
||
|
Dante
|
30c07dc9ec |
fix: cancel-subscription dialog renders Invalid Date for ISO fractional seconds (#11539)
## Summary `CancelSubscriptionDialogContent` calls `new Date(dateStr)` directly on both `cancelAt` and `subscription.value?.endDate`. Strict ISO 8601 parsers (Safari and some WebViews) reject fractional seconds whose length is anything other than 3 digits, so a Go-style backend timestamp such as `2026-04-18T10:04:55.6513Z` rendered as `Your access continues until Invalid Date.` in a destructive billing flow. PR #11358 already added the tolerant `parseIsoDateSafe` helper and applied it to the Secrets panel. This PR closes the same gap in the cancellation dialog and adds regression coverage that exercises the strict-parser code path (V8 alone is too lenient to fail without it). ## Changes - `CancelSubscriptionDialogContent.vue` — pipe both date sources through `parseIsoDateSafe`; collapse the two-step null check into one. When the value is missing OR unparseable, fall back to the existing `subscription.cancelDialog.endOfBillingPeriod` translation instead of emitting `Invalid Date`. - `CancelSubscriptionDialogContent.test.ts` (new) — wraps the assertions in the same `withStrictMillisecondParser` shim used by `dateTimeUtil.test.ts`, so 1-, 4-, and 9-digit fractional inputs actually exercise the broken path. Also covers the missing/unparseable fallbacks and the `cancelAt`-takes-precedence ordering. ## Red-green proof (local) Confirmed locally before splitting the commits: | Commit | `pnpm exec vitest run …CancelSubscriptionDialogContent.test.ts` | |---|---| | `c8aecd07f test: regression cover …` (test-only) | 5 failed / 1 passed — DOM rendered `"Your access continues until Invalid Date."` | | `f24cb903f fix: parse cancel-subscription dialog ISO timestamps …` | 6 passed | CI on this branch only runs on PR HEAD; happy to force-push a transient red commit if you want a recorded red CI run alongside the green one. ## Test plan - [x] `pnpm exec vitest run src/components/dialog/content/subscription/CancelSubscriptionDialogContent.test.ts` (6 passing on green) - [x] `pnpm typecheck` - [x] `pnpm exec eslint src/components/dialog/content/subscription/CancelSubscriptionDialogContent.{vue,test.ts}` - [x] `pnpm exec oxfmt --check` on changed files - [ ] Manual repro on Safari with a Go-emitted cancel timestamp (out of scope here; the unit test asserts the equivalent strict-parser behavior) ## Origin Surfaced by `/codex:adversarial-review` as the gap left after PR #11358 (Secrets panel) — the same `new Date(...)` hazard survived in a destructive billing flow with no regression coverage. ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-11539-fix-cancel-subscription-dialog-renders-Invalid-Date-for-ISO-fractional-seconds-34a6d73d365081e4bdd6c941afd8cef3) by [Unito](https://www.unito.io) |
||
|
Christian Byrne
|
7d1d7c8315 |
fix: remove deleted workflow from search results in sidebar (#11425)
*PR Created by the Glary-Bot Agent* --- ## Summary Deleting a workflow in the sidebar while search is active left the deleted workflow visible in the search results. Interacting with the stale entry (e.g. duplicate) caused undefined behavior. ## Root Cause `filteredWorkflows` in `BaseWorkflowsSidebarTab.vue` was a `ref` populated once per search event — a static snapshot that never reacted to store mutations. When `workflowStore.deleteWorkflow()` removed a workflow from `workflowLookup`, the search-panel's `filteredWorkflows` still held a stale reference. ## Fix Convert `filteredWorkflows` from a `ref` to a `computed` that reactively derives from `searchQuery` and `workflowStore.workflows`. This follows the same pattern already used by `filteredPersistedWorkflows` and `filteredBookmarkedWorkflows` in the same component. `handleSearch` is simplified to only manage tree expansion (its only remaining side effect). ## Test Plan Three e2e regression tests added to `workflowSearch.spec.ts`: - **Delete during search removes from results** — deletes a workflow while search is active, asserts it disappears - **Delete during search preserves siblings** — asserts all other matched workflows remain visible after one is deleted - **Clear search after delete shows correct browse view** — deletes during search, clears search, verifies browse view is consistent ## Screenshots   ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-11425-fix-remove-deleted-workflow-from-search-results-in-sidebar-3476d73d365081f19ef6c6b9261a1ee9) by [Unito](https://www.unito.io) --------- Co-authored-by: Glary-Bot <glary-bot@users.noreply.github.com> |
||
|
Christian Byrne
|
739d4b6136 |
fix: move template distribution filter from v-show to data pipeline (#11418)
*PR Created by the Glary-Bot Agent* --- ## Summary - Moves distribution-based template filtering from a CSS-level `v-show` gate into the `useTemplateFiltering` composable's data pipeline, guaranteeing that templates not meant for the current distribution never reach the view layer - Fixes "Showing 19 of 419" count mismatch when only 2 templates are visible on Cloud with "Wan 2.2" filter active - Derives `availableModels` and `availableUseCases` from distribution-visible templates so filter dropdowns don't show options that only exist on other distributions - Always prunes `activeModels`/`activeUseCases` against available options to prevent stale persisted selections from causing zero-result filtering ## Root Cause The template selector dialog used `v-show="isTemplateVisibleOnDistribution(template)"` to hide templates that don't match the current distribution (cloud/desktop/local). But `filteredCount` and `totalCount` were computed upstream in the pipeline before this visual filter, so the count text showed all matching templates regardless of distribution visibility. ## Changes - **`useTemplateFiltering.ts`**: Added `visibleTemplates` computed that applies distribution filter at the top of the pipeline. All downstream computeds (`fuse`, `availableModels`, `availableUseCases`, `filteredBySearch`, counts) now operate on this distribution-filtered set. `activeModels`/`activeUseCases` always prune against available options. - **`WorkflowTemplateSelectorDialog.vue`**: Passes `distributions` ref to composable, removes `v-show` gate and `isTemplateVisibleOnDistribution` function. - **`useTemplateFiltering.test.ts`**: 10 new unit tests covering distribution filtering, filter composition (search + model + use case + runsOn), stale persisted selections, multi-distribution templates, and Mac distribution. - **`templateFilteringCount.spec.ts`**: 5 new `@cloud` e2e tests verifying count/card consistency, DOM leak prevention, and filter reset behavior with mocked template data. ## Verification - 22 unit tests passing (12 existing + 10 new) - `pnpm typecheck` clean - `pnpm typecheck:browser` clean - `oxlint` + `eslint` clean on all changed files - E2E tests tagged `@cloud` — designed for CI cloud build execution ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-11418-fix-move-template-distribution-filter-from-v-show-to-data-pipeline-3476d73d365081c3ba09fc8a42eb4c9b) by [Unito](https://www.unito.io) --------- Co-authored-by: Glary-Bot <glary-bot@users.noreply.github.com> Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai> |
||
|
Christian Byrne
|
ef59f46495 |
refactor: migrate cn imports from @/utils/tailwindUtil shim to @comfyorg/tailwind-utils directly (#11453)
*PR Created by the Glary-Bot Agent* --- ## Summary - Replace all `cn` / `ClassValue` imports from the `@/utils/tailwindUtil` re-export shim with direct imports from `@comfyorg/tailwind-utils` across 198 source files in `src/` and 3 in `apps/desktop-ui/` - Delete both shim files (`src/utils/tailwindUtil.ts` and `apps/desktop-ui/src/utils/tailwindUtil.ts`) - Add explicit `@comfyorg/tailwind-utils` dependency to `apps/desktop-ui/package.json` - Update documentation references in `AGENTS.md`, `docs/guidance/design-standards.md`, and `docs/guidance/vue-components.md` Fixes #11288 ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-11453-refactor-migrate-cn-imports-from-utils-tailwindUtil-shim-to-comfyorg-tailwind-utils--3486d73d365081ec92cce91fbf88e6e4) by [Unito](https://www.unito.io) --------- Co-authored-by: Glary-Bot <glary-bot@users.noreply.github.com> Co-authored-by: GitHub Action <action@github.com> Co-authored-by: Alexander Brown <drjkl@comfy.org> |
||
|
pythongosssss
|
9522f68ae6 |
test: additional load3d e2e coverage (#11521)
## Summary Expands the e2e coverage for the load3d node ## Changes - **What**: - test recording, grid and background upload ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-11521-test-additional-load3d-e2e-coverage-3496d73d3650814595e1eabe97448993) by [Unito](https://www.unito.io) --------- Co-authored-by: github-actions <github-actions@github.com> Co-authored-by: GitHub Action <action@github.com> |
||
|
guill
|
a9efd4de62 |
fix: render edit pencil icon correctly in properties panel header (#11487)
*PR Created by the Glary-Bot Agent* --- ## Summary The edit pencil button next to the selected node's name at the top of the properties panel (rightSidePanel) rendered as a dark filled square instead of a pencil icon. ## Root cause The button was given `size-4` (16×16) while the inner iconify `<i class="icon-[lucide--pencil] size-4">` was also 16×16. The icon overflowed the button and was clipped, and `content-center` has no effect on a default `<button>` element, so the icon wasn't centered either. Since iconify icons render via `background-color` masked by the SVG, a clipped mask rendered as a partial/solid block that reads as a dark square. ## Fix Remove `size-4` and `content-center` from the button; use `inline-flex items-center justify-center` so the button sizes naturally around the 16×16 icon and centers it properly. ```diff -class="relative top-[2px] ml-2 size-4 shrink-0 cursor-pointer content-center text-muted-foreground hover:text-base-foreground" +class="relative top-[2px] ml-2 inline-flex shrink-0 cursor-pointer items-center justify-center text-muted-foreground hover:text-base-foreground" ``` One-line change in `src/components/rightSidePanel/RightSidePanel.vue`. No behavior change — clicking the button still switches the title into edit mode via `isEditing = true`. Existing e2e coverage in `browser_tests/tests/propertiesPanel/titleEditing.spec.ts` exercises click behavior. ## Visual verification Before — dark filled square:  After — pencil icon renders correctly:  Full panel view after the fix:  ## Quality gates - `pnpm lint` — clean (pre-existing unrelated warning in a test file) - `pnpm typecheck` — clean - `pnpm format` — no-op - Manual verification: clicking the pencil still opens the editable title input ## Context Reported by Alex in Slack `#bug-dump`. Present in `main`, unrelated to #11414. Bug tracker: [Notion — Icon next to node name in properties panel is broken](https://www.notion.so/Bug-Icon-next-to-node-name-in-properties-panel-is-broken-3486d73d365081919d7ae96dbe260ab4) ## Screenshots    ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-11487-fix-render-edit-pencil-icon-correctly-in-properties-panel-header-3496d73d36508157ba08f4a7a6e31fdd) by [Unito](https://www.unito.io) --------- Co-authored-by: Glary-Bot <glary-bot@users.noreply.github.com> |
||
|
pythongosssss
|
ac728b92ae |
fix: fix webcam node not showing preview in nodes 2.0 (#11549)
## Summary Adds test coverage for webcam node & fixes issue found in testing where the captured image does not show in nodes 2.0 ## Changes - **What**: - call `setNodePreviewsByNodeId` alongside `node.imgs = [img]` - add tests for general coverage ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-11549-fix-fix-webcam-node-not-showing-preview-in-nodes-2-0-34a6d73d3650810c89eee9c25cd07700) by [Unito](https://www.unito.io) --------- Co-authored-by: GitHub Action <action@github.com> |
||
|
Dante
|
bea72410fd |
test: add unit tests for utility widgets (#11442)
## Summary Adds 26 unit tests across 3 files covering BatchNavigation, FormSearchInput, and WidgetLayoutField. Part of a widget-test-coverage sequence. ## Changes - **What**: - \`BatchNavigation.test.ts\` (10) — hidden when count ≤ 1, counter formatted as 1-based \`current / total\`, prev/next navigation, disabled states at range boundaries. - \`FormSearchInput.test.ts\` (8) — v-model binding as the user types, clear-button visibility based on trimmed-query, debounced searcher invocation with fake timers (250ms debounce, 1000ms maxWait). - \`WidgetLayoutField.test.ts\` (8) — widget.name vs widget.label preference, empty-name suppression, \`HideLayoutFieldKey\` injection hides label but preserves slot, slot receives \`borderStyle\` scoped prop. ## Review Focus - Fake timers used in FormSearchInput tests for \`refDebounced\` — the debounce assertion depends on the 250ms/1000ms window in the component staying unchanged. - \`HideLayoutFieldKey\` provided via \`global.provide\` using the Symbol key. - No changes to any source component. ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-11442-test-add-unit-tests-for-utility-widgets-3486d73d365081a891cafe21b09b91c0) by [Unito](https://www.unito.io) --------- Co-authored-by: GitHub Action <action@github.com> Co-authored-by: bymyself <cbyrne@comfy.org> |
||
|
Dante
|
2c772077e0 |
test: add E2E tests for billing dialogs (CancelSubscription, TopUpCredits) (#10969)
## Summary - Add Playwright E2E tests for `CancelSubscriptionDialogContent` and `TopUpCreditsDialogContentLegacy` - CancelSubscription tests: dialog display with date formatting, keep subscription dismiss, confirm cancel with mocked API, error handling on API failure - TopUpCredits tests: dialog display with preset amounts, insufficient credits variant, preset selection, close button dismiss, pricing link visibility Part of the FixIt Burndown test coverage initiative (Untested Dialogs). ## Test plan - [ ] Verify tests pass in CI against OSS build - [ ] `pnpm test:browser:local -- browser_tests/tests/dialogs/cancelSubscriptionDialog.spec.ts` - [ ] `pnpm test:browser:local -- browser_tests/tests/dialogs/topUpCreditsDialog.spec.ts` ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-10969-test-add-E2E-tests-for-billing-dialogs-CancelSubscription-TopUpCredits-33c6d73d36508164b268c08c99464ca1) by [Unito](https://www.unito.io) |
||
|
Dante
|
00c294297e |
test: add WidgetImageCrop unit tests (#11470)
## Summary Splits the WidgetImageCrop test coverage out of #11446 so this widget can be reviewed independently. ## Changes - **What**: Adds WidgetImageCrop unit tests covering empty/loading/loaded states, ratio-control gating, bounding-box delegation, and disabled upstream behavior. ## Review Focus Focused test-only PR extracted from #11446. Includes small test-only cleanups from the earlier review: shared crop mock defaults, accessible image querying, and reactive upstream mock setup. Validated with `pnpm test:unit -- --run src/components/imagecrop/WidgetImageCrop.test.ts`. ## Screenshots (if applicable) N/A ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-11470-test-add-WidgetImageCrop-unit-tests-3486d73d365081ff9a1eed159a8eb9a3) by [Unito](https://www.unito.io) --------- Co-authored-by: GitHub Action <action@github.com> |
||
|
Dante
|
65e27b5cdf |
test: add unit tests for graph-level widgets (#11445)
## Summary Adds 22 unit tests across 3 files covering WidgetDOM, MultiSelectWidget, and TextPreviewWidget. Part of a widget-test-coverage sequence. ## Changes - **What**: - \`WidgetDOM.test.ts\` (4) — mounts the resolved DOMWidget element into the container, empty container when no host node resolves, skips mount when resolved widget is not a DOM widget, visible root for pointer-event capture. - \`MultiSelectWidget.test.ts\` (8) — forwards \`inputSpec.options\`, falls back to empty options, placeholder from \`multi_select.placeholder\`, default placeholder, chip vs comma display, initial selection forwarding. - \`TextPreviewWidget.test.ts\` (10) — plain text, newline→\`<br>\`, bare-URL auto-linking, \`[[label|url]]\` http link with target/rel safety, non-http falls back to escaped label (XSS-safe), skeleton visibility transitions via mocked executionStore. ## Review Focus - \`WidgetDOM\` mocks \`useCanvasStore\`, \`resolveWidgetFromHostNode\`, and \`isDOMWidget\` at the module boundary; test asserts identity of the mounted element (same \`HTMLElement\` reference) rather than canvas-side-effects. - \`TextPreviewWidget\` replaces \`useExecutionStore\` with a \`reactive()\` proxy held in a hoisted holder so watcher assertions see real reactive mutations (plain \`vi.hoisted\` objects don't trigger Vue effects). - No changes to any source component. ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-11445-test-add-unit-tests-for-graph-level-widgets-3486d73d3650816180d5f31a523f5c22) by [Unito](https://www.unito.io) --------- Co-authored-by: GitHub Action <action@github.com> Co-authored-by: bymyself <cbyrne@comfy.org> |
||
|
Dante
|
dd16e7a9ea |
test: add WidgetBoundingBox unit tests (#11468)
## Summary Splits the WidgetBoundingBox test coverage out of #11446 so this widget can be reviewed independently. ## Changes - **What**: Adds WidgetBoundingBox unit tests covering labels, initial values, min constraints, immutable v-model updates, and disabled propagation. ## Review Focus Focused test-only PR extracted from #11446. Validated with `pnpm test:unit -- --run src/components/boundingbox/WidgetBoundingBox.test.ts`. ## Screenshots (if applicable) N/A ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-11468-test-add-WidgetBoundingBox-unit-tests-3486d73d365081a682f8c5090e376ec6) by [Unito](https://www.unito.io) --------- Co-authored-by: GitHub Action <action@github.com> |
||
|
pythongosssss
|
9ed7a7bd87 |
test: Add tests for help center (#11475)
## Summary Test coverage for help center & associated popups ## Changes - **What**: - Adds HelpCenterHelper for mocking endpoints and locators - Tests for popup, menu items & positioning ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-11475-test-Add-tests-for-help-center-3486d73d365081af91a2eb7465e503fe) by [Unito](https://www.unito.io) |
||
|
Dante
|
78630f5485 |
test: add WidgetRange unit tests (#11471)
## Summary Splits the WidgetRange test coverage out of #11446 so this widget can be reviewed independently. ## Changes - **What**: Adds WidgetRange unit tests covering value pass-through, display propagation, disabled-state handling, upstream overrides, and histogram derivation. ## Review Focus Focused test-only PR extracted from #11446. Validated with `pnpm test:unit -- --run src/components/range/WidgetRange.test.ts`. ## Screenshots (if applicable) N/A ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-11471-test-add-WidgetRange-unit-tests-3486d73d365081d7a684ca3ff02320d6) by [Unito](https://www.unito.io) --------- Co-authored-by: GitHub Action <action@github.com> |
||
|
Christian Byrne
|
4b5c15fc7d |
fix: show credits in legacy user popover on non-cloud distributions (#11463)
*PR Created by the Glary-Bot Agent* --- ## Summary Credits no longer showed in the current user popover on local/desktop builds. Root cause: the credits row in `CurrentUserPopoverLegacy.vue` was gated behind `isCloud && isActiveSubscription`, and `isCloud` is a compile-time constant that resolves to `false` on local (`DISTRIBUTION='localhost'`) — so the element never rendered and `fetchBalance()` never fired (no network request, no console logs). This fix decouples the credits balance row from the `isCloud` gate. Subscription-specific UI (subscribe button, partner nodes, plans & pricing, manage plan, upgrade-to-add-credits) remains gated by `isCloud` as intended by PR #9958. ## Changes - `CurrentUserPopoverLegacy.vue`: credits row `v-if` changed from `isCloud && isActiveSubscription` → `isActiveSubscription`. On non-cloud, `isActiveSubscription` resolves to `true` via `isSubscribedOrIsNotCloud` in `useSubscription.ts`, so credits display for logged-in users. - `CurrentUserPopoverLegacy.vue`: `upgrade-to-add-credits` button now requires `isCloud && isFreeTier` (subscription-tier concept only meaningful on cloud). The `add-credits` top-up button remains available everywhere. - `CurrentUserPopoverLegacy.test.ts`: updated non-cloud tests to assert credits balance is visible and add-credits button renders, while upgrade-to-add-credits and other subscription UI stay hidden. Mirrors the behavior of `CurrentUserPopoverWorkspace.vue`, which never had the `isCloud` gate on its credits row. ## Verification - `pnpm vitest run src/components/topbar/CurrentUserPopoverLegacy.test.ts`: **21/21 passing**, including new non-cloud assertions - `pnpm typecheck`: clean - `pnpm lint` / `pnpm format:check`: clean - Live frontend dev server renders on localhost with `__DISTRIBUTION__='localhost'` (the previously-failing scenario). Attached screenshot shows the app running on local distribution; the popover itself only appears for logged-in users, so its contents are exercised by the unit tests. Fixes FE-219 ## Screenshots  ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-11463-fix-show-credits-in-legacy-user-popover-on-non-cloud-distributions-3486d73d365081c587d8ee7eae9a5c3d) by [Unito](https://www.unito.io) --------- Co-authored-by: Glary-Bot <glary-bot@users.noreply.github.com> |
||
|
pythongosssss
|
35bfe509b3 |
test: add/update terminal tests (#11239)
## Summary Adds test coverage for the integrated terminal ## Changes - **What**: - refactor and simplify existing tests - add new tests for xterm integration ┆Issue is synchronized with this [Notion page](https://app.notion.com/p/PR-11239-test-add-update-terminal-tests-3426d73d365081c99445c35d8808afb4) by [Unito](https://www.unito.io) |
||
|
Dante
|
07ce7123c8 |
test: cover useErrorActions and useErrorReport (#11320)
Closes coverage gaps in \`src/components/rightSidePanel/errors/\` as
part of the unit-test backfill.
## Testing focus
\`useErrorActions\` is thin (telemetry + command + \`window.open\`), but
\`useErrorReport\` is a real async watcher with multiple store
dependencies, \`@vueuse/core\`'s \`until(...)\`, and a cancellation
guard. The tricky part is keeping \`until\` reactive without mocking
\`@vueuse/core\`.
### \`useErrorActions\` (8 tests)
- Three functions × telemetry-fired × command/window invocation × the
\`telemetry?.\` null-safe branch.
- \`findOnGitHub\` encoding: verifies \`encodeURIComponent\` runs on the
error message and \` is:issue\` is appended.
- \`window.open\` stubbed via \`vi.spyOn\`, restored in \`afterEach\`.
### \`useErrorReport\` (9 tests)
- **Reactive \`until()\`.** \`@vueuse/core\` is **not** mocked. The
\`useSystemStatsStore\` mock creates real Vue \`ref\`s and exposes them
via getter/setter so \`until(() => isLoading).toBe(false)\` resolves
through actual reactivity.
- **\`__setSystemStats\` / \`__setIsLoading\` helpers** on the mocked
store let tests mutate state from the outside without leaking global
mutable state beyond \`vi.hoisted\`.
- **Cancellation guard.** Manually-resolvable deferred \`getLogs\`
promise — while it's pending, the \`cardSource\` ref is swapped. The
previous run's results must **not** mutate \`enrichedDetails\`.
Regressions here would cause race-dependent UI state when users switch
between error cards quickly.
- **Fallback paths.** Missing \`exceptionType\` →
\`FALLBACK_EXCEPTION_TYPE\` ('Runtime Error'). \`serialize()\` throws →
early return. \`generateErrorReport\` throws → \`displayedDetailsMap\`
falls back to the raw \`error.details\`.
- **Watcher cleanup.** Swapping the card ref clears stale
\`enrichedDetails\` before re-enrichment.
- \`console.warn\` spy suppresses noise; restored in \`afterEach\`.
## Principles applied
- No mocks of \`vue\` or \`@vueuse/core\` — only our own modules
(\`api\`, \`app\`, \`systemStatsStore\`, \`errorReportUtil\`).
- \`@vue/test-utils\` isn't installed; a local \`flushPromises\` helper
is used (matches the existing pattern in
\`useNodeHelpContent.test.ts\`).
- All 17 tests pass; typecheck/lint/format clean. Test-only; no
production code touched.
|
||
|
Christian Byrne
|
a3893a593d |
refactor: move select components from input/ to ui/ component library (#11378)
*PR Created by the Glary-Bot Agent* --- ## Summary Reconciles `src/components/input/` (older select components) into `src/components/ui/` (internal component library), eliminating the separate `input/` directory entirely. ## Changes - **Move MultiSelect** → `src/components/ui/multi-select/MultiSelect.vue` - **Move SingleSelect** → `src/components/ui/single-select/SingleSelect.vue` - **Extract shared resources** → `src/components/ui/select/types.ts` (SelectOption type) and `src/components/ui/select/select.variants.ts` (CVA styling variants) - **Update 7 consuming files** to use new import paths - **Update 1 test file** (AssetFilterBar.test.ts mock paths) - **Move stories and tests** alongside their components - **Delete `src/components/input/`** directory ## Context The `input/` directory contained only MultiSelect and SingleSelect — two well-built components that already used the same stack as `ui/` (Reka UI, CVA, Tailwind 4, Composition API). MultiSelect even imported `ui/button/Button.vue`. Moving them into `ui/` removes the split and consolidates all reusable components in one place. No API changes — all component props, slots, events, and behavior are preserved exactly. ## Verification - `pnpm typecheck` ✅ - `pnpm build` ✅ - `pnpm lint` (stylelint + oxlint + eslint) ✅ - All 15 relevant tests pass (MultiSelect: 5, SingleSelect: 2, AssetFilterBar: 8) ✅ - `pnpm knip` — no dead exports ✅ - No stale `@/components/input/` references remain ✅ - Pre-commit hooks pass ✅ - Git detected all moves as renames (97-100% similarity) ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-11378-refactor-move-select-components-from-input-to-ui-component-library-3476d73d3650810e99b4c3e0842e67f3) by [Unito](https://www.unito.io) Co-authored-by: Glary-Bot <glary-bot@users.noreply.github.com> |
||
|
Terry Jia
|
deba72e7a0 |
gizmo controls (#11274)
## Summary Add Gizmo transform controls to load3d - Remove automatic model normalization (scale + center) on load; models now appear at their original transform. The previous auto-normalization conflicted with gizmo controls — applying scale/position on load made it impossible to track and reset the user's intentional transform edits vs. the system's normalization - Add a manual Fit to Viewer button that performs the same normalization on demand, giving users explicit control - Add Gizmo Controls (translate/rotate) for interactive model manipulation with full state persistence across node properties, viewer dialog, and model reloads - Gizmo transform state is excluded from scene capture and recording to keep outputs clean ## Motivation The gizmo system is a prerequisite for these potential features: - Custom cameras — user-placed cameras in the scene need transform gizmos for precise positioning and orientation - Custom lights — scene lighting setup requires the ability to interactively position and aim light sources - Multi-object scene composition — positioning multiple models relative to each other requires per-object transform controls - Pose editor — skeletal pose editing depends on the same transform infrastructure to manipulate individual bones/joints Auto-normalization was removed because it silently mutated model transforms on load, making it impossible to distinguish between the original model pose and user edits. This broke gizmo reset (which needs to know the "clean" state) and would corrupt round-trip transform persistence. ## Screenshots (if applicable) https://github.com/user-attachments/assets/621ea559-d7c8-4c5a-a727-98e6a4130b66 ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-11274-gizmo-controls-3436d73d365081c38357c2d58e49c558) by [Unito](https://www.unito.io) |
||
|
Dante
|
40083d593b |
test: cover Button, Textarea, Slider components (#11325)
Closes coverage gaps in \`src/components/ui/\` as part of the unit-test
backfill. Uses \`@testing-library/vue\` +
\`@testing-library/user-event\` for user-centric, behavioral assertions.
## Testing focus
Three Reka-UI primitives. The challenge is testing the contract — not
the library internals — given happy-dom's gaps and Reka's
\`useMounted()\`-based async initialization.
### \`Button\` (7 tests)
- Slot rendering + click event propagation.
- \`loading=true\`: three invariants hold **simultaneously** — slot
hidden, \`pi-spin\` spinner present, button is \`toBeDisabled()\`.
- \`disabled=true\` alone: button disabled, no spinner.
- \`as="a"\`: polymorphic root tag (Reka \`Primitive\`'s \`as\` prop
switches the rendered element).
- Variant class pass-through: **one** deliberate style assertion because
the variant-system wiring is part of the component's public contract. No
other styling/class checks (AGENTS.md bans class-based tests).
### \`Textarea\` (6 tests)
- \`v-model\` two-way binding: \`user.type()\` updates the bound ref;
initial value populates the textarea.
- \`disabled\` asserted **behaviorally** — typing is blocked when
disabled, not just the attribute presence.
- Pass-through: \`placeholder\`, \`rows\`, \`class\`.
### \`Slider\` (8 tests)
- Thumb count matches \`modelValue.length\` (range support).
- ARIA: \`aria-valuemin\` / \`aria-valuemax\` / \`aria-valuenow\`.
**Caveat:** Reka's \`SliderRoot\` uses \`useMounted()\`, so
\`aria-valuenow\` is absent on the first render tick. The tests use a
two-tick \`flush()\` helper (\`await nextTick()\` twice) to wait it out
— no mocking of Reka required.
- Keyboard drag: \`user.keyboard('{ArrowRight}')\` / \`'{ArrowLeft}'\`
moves the value; with \`step: 10\` starting from 50, ArrowRight produces
exactly \`[60]\`.
- \`disabled\` → no emit on keyboard events.
### Reka integration limit
Pointer-driven \`slide-start\` / \`slide-end\` gestures in happy-dom
would require faking \`getBoundingClientRect\` and \`setPointerCapture\`
— that crosses into mocking Reka internals. Keyboard-drag paths are
covered instead (the user-facing contract); the \`pressed\` CSS state is
exercised implicitly by surviving a full mount + update cycle.
## Principles applied
- No mocks of Vue, Reka, or \`@vueuse/core\`.
- Queries via \`getByRole\` / \`getByLabelText\`; **no** class-name or
Tailwind-token queries (per AGENTS.md).
- All 21 tests pass; typecheck/lint/format clean. Test-only; no
production code touched.
|
||
|
Kelly Yang
|
6dba67da6b |
refactor: remove @ts-expect-error suppressions in sidebar components (#11338)
…(issue #11092 phase 4a)
## Summary
Part of #11092 — Phase 4a: remove 10 @ts-expect-error suppressions from
three sidebar component files.
## Changes
3 files in the sidebar had `@ts-expect-error` suppressions that all
traced back to the same root cause: **optional properties on generic
interfaces that TypeScript cannot narrow through indirect conditions.**
`TreeExplorerNode<T>` declares `data?: T` — optional by design, since
folder nodes may carry no payload. Every `handleClick`, `handleDrop`,
and `handleDelete` method that accessed `this.data` was relying on the
runtime invariant that leaf nodes always have data, but TypeScript has
no way to derive `data !== undefined` from `this.leaf === true`. The fix
was to make the invariant explicit in the condition (`if (this.leaf &&
this.data)`) or add an early-return guard (`if (!nodeDefToAdd) return`).
The same pattern appeared in a closure in `ModelLibrarySidebarTab.vue`:
`model` was `ComfyModelDef | null` from an outer const, and `if
(this.leaf)` inside a method cannot narrow a captured variable. Widening
the condition to `if (this.leaf && model)` resolved it. Two additional
suppressions in that file covered `addNodeOnGraph`'s nullable return and
its optional `widgets` property, both fixed with optional chaining.
The remaining suppression was an unannotated function parameter inferred
as `any`; adding the explicit type from the `filters` ref removed it.
<!-- CURSOR_SUMMARY -->
---
> [!NOTE]
> **Low Risk**
> Low risk: changes are TypeScript-safety refactors (extra
null/undefined guards) plus new unit tests; runtime behavior should only
differ in edge cases where `data`/`model`/`widgets` are unexpectedly
missing.
>
> **Overview**
> Removes several `@ts-expect-error` suppressions in sidebar library
tabs by making leaf-node invariants explicit (`if (this.leaf &&
this.data/model)`), adding early returns for missing drag-drop payloads,
and using optional chaining for nullable `addNodeOnGraph`/`widgets`
access.
>
> Adds new Vitest coverage for `ModelLibrarySidebarTab`,
`NodeLibrarySidebarTab`, and `NodeBookmarkTreeExplorer` to validate
click-to-add-node behavior, folder expansion toggling, filter add/remove
flow, bookmark drag/drop, and safe no-op paths when required data is
absent.
>
> <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit
|
||
|
Johnpaul Chiwetelu
|
beaa269a63 |
feat: polish settings dialog layout and keybinding display (#11241)
Polish keybinding display. Based on #11212 with adjustments: left-aligned content (no centering), key uppercase moved to UI layer. - Reduce settings content font size to 14px - Increase spacing between setting sections with cleaner dividers - Consistent min-height for form items (toggle, slider, dropdown) - Capitalize keybinding badges via CSS `uppercase` instead of mutating data model - Remove '+' separator between keybinding badges - Unbold keybinding badges with fixed min-width Supersedes #11212 ┆Issue is synchronized with this [Notion page](https://app.notion.com/p/PR-11241-feat-polish-settings-dialog-layout-and-keybinding-display-3426d73d3650812a97e4d941a76a4fe9) by [Unito](https://www.unito.io) --------- Co-authored-by: Alex <alex@Mac.localdomain> Co-authored-by: github-actions <github-actions@github.com> |
||
|
Kelly Yang
|
cf98013c18 |
test: expand Image Crop E2E and fix loading overlay deadlock (#11193)
## Summary
Expands Playwright coverage for the **ImageCropV2** widget (Levels 1–3
from the image crop E2E plan), fixes **loading / image mount** behavior
when `imageUrl` changes, adds **stable resize-handle selectors**, and
adds a **small Vitest** for URL→loading transitions.
## Changes
- [x] **Level 1 (E2E)** — Empty state: assert resize handle hidden;
screenshot baseline `image-crop-empty-state.png`; pointer drag on empty
state does not change widget bounds.
- [x] **Level 1 (E2E)** — After run: assert **8 visible** resize handles
with `data-testid` + `filter({ visible: true })`; broken `img.src`
returns to empty state (`crop-empty-state`, no overlay).
- [x] **Level 1 (E2E)** — **Slow `/api/view`** route (delay only
`example.png`) to assert **“Loading…”** then hidden after image loads;
comment clarifies delay is in the route handler, not
`page.waitForTimeout`.
- [x] **Level 2 (E2E)** — Drag clamps to **right/bottom** and
**top-left** image bounds via `setCropBounds` + `expect.poll` on natural
bounds.
- [x] **Level 3 (E2E)** — Free resize: right / left / bottom / top
edges; SE and NW corners; `MIN_CROP_SIZE` (16px); right-edge boundary
clamp; **8 handles** screenshot `image-crop-eight-handles.png`; SE/NW
screenshots (`image-crop-resize-se.png`, `image-crop-resize-nw.png`).
- [x] **E2E helpers** — Shared `getCropValue`, `setCropBounds`,
`dragOnLocator`, `POINTER_OPTS`; drag regression uses **`expect.poll`**
instead of `toPass` where appropriate.
- [x] **`WidgetImageCrop.vue`** — When `imageUrl` is set, **always
render `<img>`**; show loading as an **absolute overlay** (fixes
deadlock where `isLoading` blocked `<img>` so `@load` never ran); add
**`data-testid="crop-resize-{direction}"`** on resize handles.
- [x] **`useImageCrop.ts`** — Watch `imageUrl` and drive `isLoading`;
extract **`imageCropLoadingAfterUrlChange`** (`boolean | null`) for
clear semantics and tests.
- [x] **`useImageCrop.test.ts`** — Vitest coverage for
`imageCropLoadingAfterUrlChange` (null URL, URL change, first URL,
unchanged URL).
## Screenshot / CI notes
- [ ] **Linux screenshot expectations** for new/updated
`toHaveScreenshot(...)` names must be produced on **CI (Linux)** — add
the **`New Browser Test Expectation`** label (or equivalent workflow);
**do not** commit local **Darwin** golden files.
- [x] Existing Linux baselines under `imageCrop.spec.ts-snapshots/` for
prior tests are unchanged where applicable; new baselines are expected
from CI after merge workflow.
## Files
- [x] `browser_tests/tests/vueNodes/widgets/imageCrop.spec.ts`
- [x] `src/components/imagecrop/WidgetImageCrop.vue`
- [x] `src/composables/useImageCrop.ts`
- [x] `src/composables/useImageCrop.test.ts` (new)
<!-- CURSOR_SUMMARY -->
---
> [!NOTE]
> **Medium Risk**
> Touches interactive crop UI rendering and `isLoading` state
transitions, which can affect user-visible behavior and input handling;
changes are mitigated by extensive new E2E and unit tests.
>
> **Overview**
> Improves the `WidgetImageCrop` loading behavior by always rendering
the preview `<img>` when `imageUrl` is set and showing “Loading…” as an
absolute overlay, preventing a deadlock where `isLoading` could block
the `@load` event. Adds stable `data-testid="crop-resize-{direction}"`
selectors for resize handles and hardens pointer-capture handling in
`useImageCrop`.
>
> Greatly expands automated coverage: the Playwright spec now tests
empty-state rendering/screenshot, drag/resize interactions (edge/corner,
min size, and clamping to image bounds), aspect-ratio lock handle
visibility, slow `/api/view` loading overlay behavior, and broken image
fetch recovery. Adds a new Vitest suite for `useImageCrop` (including
`imageCropLoadingAfterUrlChange`) to unit-test URL→loading transitions
and core drag/resize/aspect-ratio logic.
>
> <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit
|
||
|
Jedrzej Kosinski
|
ff4c812d08 |
feat: show sign-in button via server feature flag (#11298)
## Summary Show the sign-in button in the frontend when the `show_signin_button` server feature flag is set, without requiring a special desktop distribution build. ## Changes - Add `SHOW_SIGNIN_BUTTON` to `ServerFeatureFlag` enum - Add `showSignInButton` getter in `useFeatureFlags` composable (returns `boolean | undefined`) - Update `WorkflowTabs.vue` to use `flags.showSignInButton ?? isDesktop` - the server flag takes precedence when set, falls back to compile-time `isDesktop` for legacy desktop support ## Related - Comfy-Org/ComfyUI-Desktop-2.0-Beta#415 - Backend: Comfy-Org/ComfyUI `feature/generic-feature-flag-cli` - Launcher: Comfy-Org/ComfyUI-Desktop-2.0-Beta#418 Co-authored-by: Amp <amp@ampcode.com> |
||
|
jaeone94
|
a1e6fb36d2 |
refactor: harden ChangeTracker lifecycle with self-defending API (#10816)
## Summary Harden the `ChangeTracker` lifecycle to eliminate the class of bugs where an inactive workflow's tracker silently captures the wrong graph state. Renames `checkState()` to `captureCanvasState()` with a self-defending assertion, introduces `deactivate()` and `prepareForSave()` lifecycle methods, and closes a latent undo-history corruption bug discovered during code review. ## Background ComfyUI supports multiple workflows open as tabs, but only one canvas (`app.rootGraph`) exists at a time. When the user switches tabs, the old workflow's graph is unloaded and the new one is loaded into this shared canvas. The old `checkState()` method serialized `app.rootGraph` into `activeState` to track changes for undo/redo. It had no awareness of *which* workflow it belonged to -- if called on an inactive tab's tracker, it would capture the active tab's graph data and silently overwrite the inactive workflow's state. This caused permanent data loss (fixed in PR #10745 with caller-side `isActive` guards). The caller-side guards were fragile: every new call site had to remember to add the guard, and forgetting would reintroduce the same silent data corruption. Additionally, `beforeLoadNewGraph` only called `store()` (viewport/outputs) without `checkState()`, meaning canvas state could be stale if a tab switch happened without a preceding mouseup event. ### Before (fragile) ``` saveWorkflow(workflow): if (isActive(workflow)) <-- caller must remember this guard workflow.changeTracker.checkState() <-- name implies "read", actually writes ... beforeLoadNewGraph(): activeWorkflow.changeTracker.store() <-- only saves viewport, NOT graph state ``` ### After (self-defending) ``` saveWorkflow(workflow): workflow.changeTracker.prepareForSave() <-- handles active/inactive internally ... beforeLoadNewGraph(): activeWorkflow.changeTracker.deactivate() <-- captures graph + viewport together ``` ## Changes - Rename `checkState` to `captureCanvasState` with active-tracker assertion - Add `deactivate()` and `prepareForSave()` lifecycle methods - Fix undo-history corruption: `captureCanvasState()` guarded by `_restoringState` - Fix viewport regression during undo: `deactivate()` skips `captureCanvasState()` during undo/redo but always calls `store()` to preserve viewport (regression from PR #10247) - Log inactive tracker warnings unconditionally at warn level (not DEV-only) - Deprecated `checkState()` wrapper for extension compatibility - Rename `checkState` to `captureCanvasState` in `useWidgetSelectActions` composable - Add `appModeStore.ts` to manual call sites documentation - Add `checkState()` deprecation note to architecture docs - Add 16 unit tests covering all guard conditions, lifecycle methods, and undo behavior - Add E2E test: "Undo preserves viewport offset" ## New ChangeTracker Public API | Method | Caller | Purpose | |--------|--------|---------| | `captureCanvasState()` | Event handlers, UI interactions | Snapshots canvas into activeState, pushes undo. Asserts active tracker. | | `deactivate()` | `beforeLoadNewGraph` only | `captureCanvasState()` (skipped during undo/redo) + `store()`. Freezes state for tab switch. | | `prepareForSave()` | Save paths only | Active: `captureCanvasState()`. Inactive: no-op. | | `checkState()` | **Deprecated** -- extensions only | Wrapper that delegates to `captureCanvasState()` with deprecation warning. | | `store()` | Internal to `deactivate()` | Saves viewport, outputs, subgraph navigation. | | `restore()` | `afterLoadNewGraph` | Restores viewport, outputs, subgraph navigation. | | `reset()` | `afterLoadNewGraph`, save | Resets initial state (marks as "clean"). | ## Test plan - [x] Unit tests: 16 tests covering all guard conditions, state capture, undo queue behavior - [x] E2E test: "Undo preserves viewport offset" verifies no viewport drift on undo - [x] E2E test: "Prevents captureCanvasState from corrupting workflow state during tab switch" - [x] Existing E2E: "Closing an inactive tab with save preserves its own content" - [ ] Manual: rapidly switch tabs during undo/redo, verify no viewport drift - [ ] Manual: verify extensions calling `checkState()` see deprecation warning in console |
||
|
Dante
|
e5c81488e4 |
fix: include focusMode in splitter refresh key to prevent panel resize (#11295)
## Summary When the properties panel is open, toggling focus mode on then off causes the panel to resize unexpectedly. The root cause is that `splitterRefreshKey` in `LiteGraphCanvasSplitterOverlay.vue` does not include `focusMode`, so the PrimeVue Splitter component instance is reused across focus mode transitions and restores stale panel sizes from localStorage. Fix: add `focusMode` to `splitterRefreshKey` so the Splitter is recreated when focus mode toggles. ## Red-Green Verification | Commit | CI Status | Purpose | |--------|-----------|---------| | `test: add failing test for focus mode toggle resizing properties panel` | 🔴 Red | Proves the test catches the bug | | `fix: include focusMode in splitter refresh key to prevent panel resize` | 🟢 Green | Proves the fix resolves the bug | ## demo ### AS IS https://github.com/user-attachments/assets/95f6a9e3-e4c7-4aba-8e17-0eee11f70491 ### TO BE https://github.com/user-attachments/assets/595eafcd-6a80-443d-a6f3-bb7605ed0758 ## Test Plan - [ ] CI red on test-only commit - [ ] CI green on fix commit - [ ] E2E regression test added in `browser_tests/tests/focusMode.spec.ts` ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-11295-fix-include-focusMode-in-splitter-refresh-key-to-prevent-panel-resize-3446d73d365081b7bc3ac65338e17a8f) by [Unito](https://www.unito.io) |
||
|
pythongosssss
|
06686a1f50 |
test: App mode - additional app mode coverage (#11194)
## Summary Adds additional test coverage for empty state/welcome screen/connect outputs/vue nodes auto switch ## Changes - **What**: - add tests ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-11194-test-App-mode-additional-app-mode-coverage-3416d73d365081ca91d0ed61de19f840) by [Unito](https://www.unito.io) |
||
|
jaeone94
|
693b8383d6 |
fix: missing-asset correctness follow-ups from #10856 (#11233)
Follow-up to #10856. Four correctness issues and their regression tests. ## Bugs fixed ### 1. ErrorOverlay model count reflected node selection `useErrorGroups` exposed `filteredMissingModelGroups` under the public name `missingModelGroups`. `ErrorOverlay.vue` read that alias to compute its model count label, so selecting a node shrank the overlay total. The overlay must always show the whole workflow's errors. Exposed both shapes explicitly: `missingModelGroups` / `missingMediaGroups` (unfiltered totals) and `filteredMissingModelGroups` / `filteredMissingMediaGroups` (selection-scoped). `TabErrors.vue` destructures the filtered variant with an alias. Before https://github.com/user-attachments/assets/eb848c5f-d092-4a4f-b86f-d22bb4408003 After https://github.com/user-attachments/assets/75e67819-c9f2-45ec-9241-74023eca6120 ### 2. Bypass → un-bypass dropped url/hash metadata Realtime `scanNodeModelCandidates` only reads widget values, so un-bypass produced a fresh candidate without the url that `enrichWithEmbeddedMetadata` had previously attached from `graphData.models`. `MissingModelRow`'s download/copy-url buttons disappeared after a bypass/un-bypass cycle. Added `enrichCandidateFromNodeProperties` that copies `url`/`hash`/`directory` from the node's own `properties.models` — which persists across mode toggles — into each scanned candidate. Applied to every call site of the per-node scan. A later fix in the same branch also enforces directory agreement to prevent a same-name / different-directory collision from stamping the wrong metadata. Before https://github.com/user-attachments/assets/39039d83-4d55-41a9-9d01-dec40843741b After https://github.com/user-attachments/assets/047a603b-fb52-4320-886d-dfeed457d833 ### 3. Initial full scan surfaced interior errors of a muted/bypassed subgraph container `scanAllModelCandidates`, `scanAllMediaCandidates`, and the JSON-based missing-node scan only check each node's own mode. Interior nodes whose parent container was bypassed passed the filter. Added `isAncestorPathActive(rootGraph, executionId)` to `graphTraversalUtil` and post-filter the three pipelines in `app.ts` after the live rootGraph is configured. The filter uses the execution-ID path (`"65:63"` → check node 65's mode) so it handles both live-scan-produced and JSON-enrichment-produced candidates. Before https://github.com/user-attachments/assets/3032d46b-81cd-420e-ab8e-f58392267602 After https://github.com/user-attachments/assets/02a01931-951d-4a48-986c-06424044fbf8 ### 4. Bypassed subgraph entry re-surfaced interior errors `useGraphNodeManager` replays `graph.onNodeAdded` for each existing interior node when the Vue node manager initializes on subgraph entry. That chain reached `scanSingleNodeErrors` via `installErrorClearingHooks`' `onNodeAdded` override. Each interior node's own mode was active, so the caller guards passed and the scan re-introduced the error that the initial pipeline had correctly suppressed. Added an ancestor-activity gate at the top of `scanSingleNodeErrors`, the single entry point shared by paste, un-bypass, subgraph entry, and subgraph container activation. A later commit also hardens this guard against detached nodes (null execution ID → skip) and applies the same ancestor check to `isCandidateStillActive` in the realtime verification callback. Before https://github.com/user-attachments/assets/fe44862d-f1d6-41ed-982d-614a7e83d441 After https://github.com/user-attachments/assets/497a76ce-3caa-479f-9024-4cd0f7bd20a4 ## Tests - 6 unit tests for `isAncestorPathActive` (root, active, immediate-bypass, deep-nested mute, unresolvable ancestor, null rootGraph) - 4 unit tests for `enrichCandidateFromNodeProperties` (enrichment, no-overwrite, name mismatch, directory mismatch) - 1 unit test for `scanSingleNodeErrors` ancestor guard (subgraph entry replaying onNodeAdded) - 2 unit tests for `useErrorGroups` dual export + ErrorOverlay contract - 4 E2E tests: - ErrorOverlay model count stays constant when a node is selected (new fixture `missing_models_distinct.json`) - Bypass/un-bypass cycle preserves Copy URL button (uses `missing_models_from_node_properties`) - Loading a workflow with bypassed subgraph suppresses interior missing model error (new fixture `missing_models_in_bypassed_subgraph.json`) - Entering a bypassed subgraph does not resurface interior missing model error (shares the above fixture) `pnpm typecheck`, `pnpm lint`, 206 related unit tests passing. ## Follow-up Several items raised by code review are deferred as pre-existing tech debt or scope-avoided refactors. Tracked via comments on #11215 and #11216. --- Follows up on #10856. |
||
|
Dante
|
5807e03c74 |
test: expand E2E coverage for toolbox actions (#10968)
## Summary - Adds `selectionToolboxMoreActions.spec.ts` with E2E coverage for previously untested toolbox actions - Covers: pin/unpin, minimize/expand, adjust size, copy, duplicate, refresh button, align (top/left), distribute (horizontal/vertical), alignment options hidden for single selection, multi-node bypass toggle - Part of the FixIt Burndown test coverage initiative (toolbox actions) ## Test plan - [ ] All new tests pass in CI - [ ] No regressions in existing selectionToolbox tests ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-10968-test-expand-E2E-coverage-for-toolbox-actions-33c6d73d3650811286cefdd0eb4f5242) by [Unito](https://www.unito.io) --------- Co-authored-by: Alexander Brown <drjkl@comfy.org> |
||
|
AustinMroz
|
988a546721 |
Add missing dialog tests (#11133)
Leveraging the fancy coverage functionality of #10930, this PR aims to add coverage to missing dialogue models. This has proven quite constructive as many of the dialogues have since been shown to be bugged. - The APINodes sign in dialog that displays when attempting to run a workflow containing Partner nodes while not logged in was intended to display a list of nodes required to execute the workflow. The import for this component was forgotten in the original commit (#3532) and the backing component was later knipped - Error dialogs resulting are intended to display the file responsible for the error, but the prop was accidentally left out during the refactoring of #3265 - ~~The node library migration (#8548) failed to include the 'Edit Blueprint' button, and had incorrect sizing and color on the 'Delete Blueprint' button.~~ - On request, the library button changes were spun out to a separate PR ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-11133-Add-missing-dialog-tests-33e6d73d3650812cb142d610461adcd4) by [Unito](https://www.unito.io) --------- Co-authored-by: GitHub Action <action@github.com> |
||
|
AustinMroz
|
25ac047b58 |
Fix node library action buttons (#11232)
The node library migration (#8548) failed to include the 'Edit Blueprint' button, and had incorrect sizing and color on the 'Delete Blueprint' button. - Re-add edit blueprint which was missed in the migration - Fix incorrect sizing on delete blueprint - Fix color (lucide uses background, not text) - Migrate all action buttons use our capital 'B' Button component and use standardized variants where possible Spun out of #11133 ┆Issue is synchronized with this [Notion page](https://app.notion.com/p/PR-11232-Fix-node-library-action-buttons-3426d73d365081339cafc7457c0b5421) by [Unito](https://www.unito.io) |
||
|
jaeone94
|
521019d173 |
fix: exclude muted/bypassed nodes from missing asset detection (#10856)
## Summary Muted and bypassed nodes are excluded from execution but were still triggering missing model/media/node warnings. This PR makes the error system mode-aware: muted/bypassed nodes no longer produce missing asset errors, and all error lifecycle events (mode toggle, deletion, paste, undo, tab switch) are handled consistently. - Fixes Comfy-Org/ComfyUI#13256 ## Behavioral notes - **Tab switch overlay suppression (intentional)**: Switching back to a workflow with missing assets no longer re-shows the error overlay. This reverses the behavior introduced in #10190. The error state is still restored silently in the errors tab — users can access it via the properties panel without being interrupted by the overlay on every tab switch. ## Changes ### 1. Scan filtering - `scanAllModelCandidates`, `scanAllMediaCandidates`, `scanMissingNodes`: skip nodes with `mode === NEVER || BYPASS` - `collectMissingNodes` (serialized data): skip error reporting for muted/bypassed nodes while still calling `sanitizeNodeName` for safe `configure()` - `collectEmbeddedModelsWithSource`: skip muted/bypassed nodes; workflow-level `graphData.models` only create candidates when active nodes exist - `enrichWithEmbeddedMetadata`: filter unmatched workflow-level models when all referencing nodes are inactive ### 2. Realtime mode change handling - `useErrorClearingHooks.ts` chains `graph.onTrigger` to detect `node:property:changed` (mode) - Deactivation (active → muted/bypassed): remove missing model/media/node errors for the node - Activation (muted/bypassed → active): scan the node and add confirmed errors, show overlay - Subgraph container deactivation: remove all interior node errors (execution ID prefix match) - Subgraph container activation: scan all active interior nodes recursively - Subgraph interior mode change: resolve node via `localGraph.getNodeById()` then compute execution ID from root graph ### 3. Node deletion - `graph.onNodeRemoved`: remove missing model/media/node errors for the deleted node - Handle `node.graph === null` at callback time by using `String(node.id)` for root-level nodes ### 4. Node paste/duplicate - `graph.onNodeAdded`: scan via `queueMicrotask` (deferred until after `node.configure()` restores widget values) - Guard: skip during `ChangeTracker.isLoadingGraph` (undo/redo/tab switch handled by pipeline) - Guard: skip muted/bypassed nodes ### 5. Workflow tab switch optimization - `skipAssetScans` option in `loadGraphData`: skip full pipeline on tab switch - Cache missing model/media/node state per workflow via `PendingWarnings` - `beforeLoadNewGraph`: save current store state to outgoing workflow's `pendingWarnings` - `showPendingWarnings`: restore cached errors silently (no overlay), always sync missing nodes store (even when null) - Preserve UI state (`fileSizes`, `urlInputs`) on tab switch by using `setMissingModels([])` instead of `clearMissingModels()` - `MissingModelRow.vue`: fetch file size on mount via `fetchModelMetadata` memory cache ### 6. Undo/redo overlay suppression - `silentAssetErrors` option propagated through pipeline → `surfaceMissingModels`/`surfaceMissingMedia` `{ silent }` option - `showPendingWarnings` `{ silent }` option for missing nodes overlay - `changeTracker.ts`: pass `silentAssetErrors: true` on undo/redo ### 7. Error tab node filtering - Selected node filters missing model/media card contents (not just group visibility) - `isAssetErrorInSelection`: resolve execution ID → graph node for selection matching - Missing nodes intentionally unfiltered (pack-level scope) - `hasMissingMediaSelected` added to `RightSidePanel.vue` error tab visibility - Download All button: show only when 2+ downloadable models exist ### 8. New store functions - `missingModelStore`: `addMissingModels`, `removeMissingModelsByNodeId` - `missingMediaStore`: `addMissingMedia`, `removeMissingMediaByNodeId` - `missingNodesErrorStore`: `removeMissingNodesByNodeId` - `missingModelScan`: `scanNodeModelCandidates` (extracted single-node scan) - `missingMediaScan`: `scanNodeMediaCandidates` (extracted single-node scan) ### 9. Test infrastructure improvements - `data-testid` on `RightSidePanel.vue` tabs (`panel-tab-{value}`) - Error-related TestIds moved from `dialogs` to `errorsTab` namespace in `selectors.ts` - Removed unused `TestIdValue` type - Extracted `cleanupFakeModel` to shared `ErrorsTabHelper.ts` - Renamed `openErrorsTabViaSeeErrors` → `loadWorkflowAndOpenErrorsTab` - Added `aria-label` to pencil edit button and subgraph toggle button ## Test plan ### Unit tests (41 new) - Store functions: `addMissing*`, `removeMissing*ByNodeId` - `executionErrorStore`: `surfaceMissing*` silent option - Scan functions: muted/bypassed filtering, `scanNodeModelCandidates`, `scanNodeMediaCandidates` - `workflowService`: `showPendingWarnings` silent, `beforeLoadNewGraph` caching ### E2E tests (17 new in `errorsTabModeAware.spec.ts`) **Missing nodes** - [x] Deleting a missing node removes its error from the errors tab - [x] Undo after bypass restores error without showing overlay **Missing models** - [x] Loading a workflow with all nodes bypassed shows no errors - [x] Bypassing a node hides its error, un-bypassing restores it - [x] Deleting a node with missing model removes its error - [x] Undo after bypass restores error without showing overlay - [x] Pasting a node with missing model increases referencing node count - [x] Pasting a bypassed node does not add a new error - [x] Selecting a node filters errors tab to only that node **Missing media** - [x] Loading a workflow with all nodes bypassed shows no errors - [x] Bypassing a node hides its error, un-bypassing restores it - [x] Pasting a bypassed node does not add a new error - [x] Selecting a node filters errors tab to only that node **Subgraph** - [x] Bypassing a subgraph hides interior errors, un-bypassing restores them - [x] Bypassing a node inside a subgraph hides its error, un-bypassing restores it **Workflow switching** - [x] Does not resurface error overlay when switching back to workflow with missing nodes - [x] Restores missing nodes in errors tab when switching back to workflow # Screenshots https://github.com/user-attachments/assets/e0a5bcb8-69ba-4120-ab7f-5c83e4cfc3c5 ## Follow-up work - Extract error-detection computed properties from `RightSidePanel.vue` into a composable (e.g. `useErrorsTabVisibility`) --------- Co-authored-by: github-actions <github-actions@github.com> Co-authored-by: GitHub Action <action@github.com> |
||
|
Kelly Yang
|
5b7ef3fe21 |
test: Painter Widget E2E Test Plan (#10846)
### Summary of Improvements
* **Custom Test Coverage Extension**: Enhanced the Painter widget E2E
test suite by refactoring logic for better maintainability and
robustness.
* **Stable Component Targeting**: Introduced
`data-testid="painter-dimension-text"` to `WidgetPainter.vue`, providing
a reliable, non-CSS-dependent locator for canvas size verification.
* **Improved Test Organization**: Reorganized existing test scenarios
into logical categories using `test.describe` blocks (Drawing, Brush
Settings, Canvas Size Controls, etc.).
* **Asynchronous Helper Integration**: Converted `hasCanvasContent` to
an asynchronous helper and unified its usage across multiple test cases
to eliminate redundant pixel-checking logic.
* **Locator Resilience**: Updated Reka UI slider interaction logic to
use more precise targeting (`:not([data-slot])`), preventing ambiguity
and improving test stability.
* **Scenario Refinement**: Updated the `pointerup` test logic to
accurately reflect pointer capture behavior when interactions occur
outside the canvas boundaries.
* **Enhanced Verification Feedback**: Added descriptive error messages
to `expect.poll` assertions to provide clearer context on potential
failure points.
* **Standardized Tagging**: Restored the original tagging strategy
(including `@smoke` and `@screenshot` tags) to ensure tests are
categorized correctly for CI environments.
### Red-Green Verification
| Commit | CI Status | Purpose |
| :--- | :--- | :--- |
| `test: refactor painter widget e2e tests and address review findings`
| 🟢 Green | Addresses all E2E test quality and stability issues from
review findings. |
### Test Plan
- [x] **Quality Checks**: `pnpm format`, `pnpm lint`, and `pnpm
typecheck` verified as passing.
- [x] **Component Integration**: `WidgetPainter.vue` `data-testid`
correctly applied and used in tests.
- [x] **Helper Reliability**: `hasCanvasContent` correctly identifies
colored pixels and returns a promise for `expect.poll`.
- [x] **Locator Robustness**: Verified Reka slider locators correctly
exclude internal thumb spans.
- [x] **Boundary Interaction**: Verified `pointerup` correctly ends
strokes when triggered outside the viewport.
- [x] **Tagging Consistency**: Verified `@smoke` and `@screenshot` tags
are present in the final test suite.
┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-10846-test-Painter-Widget-E2E-Test-Plan-3386d73d365081deb70fe4afbd417efb)
by [Unito](https://www.unito.io)
<!-- CURSOR_SUMMARY -->
---
> [!NOTE]
> **Low Risk**
> Primarily adds/refactors Playwright E2E tests and stable `data-testid`
hooks, with no changes to Painter drawing logic. Risk is limited to
potential test brittleness or minor UI attribute changes.
>
> **Overview**
> Expands the Painter widget Playwright suite with new grouped scenarios
covering drawing/erasing behavior, tool switching, brush inputs, canvas
resizing (including preserving drawings), clear behavior, and
serialization/upload flows (including failure toast).
>
> Refactors the tests to use a shared `@e2e/helpers/painter` module
(`drawStroke`, `hasCanvasContent`, `triggerSerialization`), improves
stability via role/testid-based locators and clearer `expect.poll`
messaging, and adds `data-testid` attributes (e.g.,
`painter-clear-button`, `painter-*-row`, `painter-dimension-text`) to
`WidgetPainter.vue` to avoid CSS-dependent selectors.
>
> <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit
|
||
|
Kelly Yang
|
cab46567c0 |
test: add E2E tests for ImageCropV2 widget (#10737)
## Summary
Adds Playwright E2E tests for the ImageCropV2 widget covering
1. the empty state (no source image)
2. default control rendering
3. source image display with crop overlay
4. drag-to-reposition behavior.
┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-10737-test-add-E2E-tests-for-ImageCropV2-widget-3336d73d365081b28ed9db63e5df383e)
by [Unito](https://www.unito.io)
<!-- CURSOR_SUMMARY -->
---
> [!NOTE]
> **Low Risk**
> Low risk: primarily adds Playwright E2E coverage and introduces
`data-testid` attributes for more stable selectors, with no changes to
core crop behavior.
>
> **Overview**
> Adds new Playwright E2E coverage for the `ImageCropV2` Vue-node
widget, including workflows/fixtures for a disconnected input and a
`LoadImage -> ImageCropV2 -> PreviewImage` pipeline.
>
> Tests validate the empty state and default controls, verify the crop
overlay renders after execution with screenshot assertions, and exercise
drag-to-reposition by dispatching pointer events and asserting the
widget’s crop value updates.
>
> Updates `WidgetImageCrop.vue` to add `data-testid` hooks (empty
state/icon and crop overlay) to make the E2E selectors stable.
>
> <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit
|
