Commit Graph

115 Commits

Author SHA1 Message Date
Alexander Brown
52ccd9ed1a refactor: internalize nextFrame() into fixture/helper methods (#11166)
## Summary

Internalize `nextFrame()` calls into fixture/helper methods so spec
authors don't need to remember to call it after common operations.
`nextFrame()` waits for one `requestAnimationFrame` (~16ms) — an extra
call is always safe, making this a low-risk refactor.

## Changes

### Phase 1: `SettingsHelper.setSetting()`
`setSetting()` now calls `nextFrame()` internally. Removed 15 redundant
calls across 7 files.

### Phase 2: `CommandHelper.executeCommand()`
`executeCommand()` now calls `nextFrame()` internally. Removed 15
redundant calls across 7 files, including the now-redundant call in
`AppModeHelper.toggleAppMode()`.

### Phase 3: `WorkflowHelper.loadGraphData()`
New helper wraps `page.evaluate(loadGraphData)` + `nextFrame()`.
Migrated `SubgraphHelper.serializeAndReload()` and `groupNode.spec.ts`.

### Phase 4: `NodeReference` cleanup
Removed redundant `nextFrame()` from `copy()`, `convertToGroupNode()`,
`resizeNode()`, `dragTextEncodeNode2()`, and
`convertDefaultKSamplerToSubgraph()`. Removed 6 spec-level calls after
`node.click('title')`.

### Phase 5: `KeyboardHelper.press()` and `delete()`
New convenience methods that press a key and wait one frame. Converted
40 `canvas.press(key)` + `nextFrame()` pairs across 13 spec files.

### Phase 6: `ComfyPage.expectScreenshot()`
New helper combines `nextFrame()` + `toHaveScreenshot()`. Converted 45
pairs across 12 spec files.

## Total impact
- **~130 redundant `nextFrame()` calls eliminated** across ~35
spec/helper files
- **3 new helper methods** added (`loadGraphData`, `press`/`delete`,
`expectScreenshot`)
- **2 existing methods** enhanced (`setSetting`, `executeCommand`)

## What was NOT changed
- `performance.spec.ts` frame-counting loops (intentional)
- `ComfyMouse.ts` / `CanvasHelper.ts` (already internalized)
- `SubgraphHelper.packAllInteriorNodes()` (deliberate orchestration)
- Builder helpers (already internalized)

┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-11166-refactor-internalize-nextFrame-into-fixture-helper-methods-33f6d73d3650817bb5f6fb46e396085e)
by [Unito](https://www.unito.io)

---------

Co-authored-by: Amp <amp@ampcode.com>
2026-04-15 15:25:47 +00:00
Christian Byrne
033b3dad3a feat: add Slack notification workflow for coverage improvements (#10977)
## Summary

Adds a GitHub Actions workflow + TypeScript script that posts to Slack
when a merged PR improves unit or E2E test coverage.

## Changes

- **What**: New `coverage-slack-notify.yaml` workflow triggered on push
to main. Compares current coverage against previous baselines, generates
Slack Block Kit payload with progress bars and milestone celebrations,
posts to `#p-frontend-automated-testing`.
- **Script**: `scripts/coverage-slack-notify.ts` — parses lcov files,
computes deltas, detects milestone crossings (every 5%), builds Slack
payload. Pure functions exported for testability.
- **Tests**: 26 unit tests in `scripts/coverage-slack-notify.test.ts`
covering all pure functions including edge cases (malformed lcov, exact
boundaries, zero coverage).

### Security hardening
- All `${{ }}` expressions moved from `run:` blocks to `env:` variables
- `SLACK_BOT_TOKEN` passed via env var, not inline
- Unique heredoc delimiter (timestamp-based) prevents payload injection
- `parseInt` fallback (`|| 0`) guards against malformed lcov
- PR regex anchored to first line of commit message

### Robustness
- `continue-on-error: true` on Slack post step (outage does not fail the
job)
- Baseline save guarded by `steps.unit-tests.outcome == success`
(prevents corrupt baselines on test failure)
- Channel ID commented for maintainability
- Top-level `text` field added for Slack mobile push notifications
- Author linked to GitHub profile instead of bare `@username`

## Review Focus

- Workflow step ordering and conditional logic
- Security of expression handling and secret management
- Slack payload structure and Block Kit formatting

┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-10977-feat-add-Slack-notification-workflow-for-coverage-improvements-33d6d73d3650819c8950f483c83f297c)
by [Unito](https://www.unito.io)

---------

Co-authored-by: GitHub Action <action@github.com>
2026-04-14 20:58:47 -07:00
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>
2026-04-14 23:20:45 +00:00
pythongosssss
5899a9392e test: Simplify vue node/menu test setup (#11184)
## Summary
Simplifies test setup for common settings

## Changes

- **What**: 
- add vue-nodes tag to auto enable nodes 2.0
- remove UseNewMenu Top as this is default

┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-11184-test-Simplify-vue-node-menu-test-setup-3416d73d3650815487e0c357d28761fe)
by [Unito](https://www.unito.io)
2026-04-13 20:43:25 +00:00
Alexander Brown
8c9328c1b2 feat: add eslint-plugin-playwright via oxlint JS plugins (#11136)
## Summary

Add eslint-plugin-playwright as an oxlint JS plugin scoped to
browser_tests/, enforcing Playwright best practices at lint time.

## Changes

- **What**: Configure eslint-plugin-playwright@2.10.1 via oxlint's alpha
`jsPlugins` field (`.oxlintrc.json` override scoped to
`browser_tests/**/*.ts`). 18 recommended rules +
`prefer-native-locators` + `require-to-pass-timeout` at error severity.
All 173 initial violations resolved (config, auto-fix, manual fixes).
`no-force-option` set to off — 28 violations need triage (canvas overlay
workarounds vs unnecessary force) in a dedicated PR.
- **Dependencies**: `eslint-plugin-playwright@^2.10.1` (devDependency,
required by oxlint jsPlugins at runtime)

## Review Focus

- `.oxlintrc.json` override structure — this is the first use of
oxlint's JS plugins alpha feature in this repo
- Manual fixes in spec files: `waitForSelector` → `locator.waitFor`,
deprecated page methods → locator equivalents, `toPass()` timeout
additions
- Compound CSS selectors replaced with `.and()` (Playwright native
locator composition) to avoid `prefer-native-locators` suppressions
- Lint script changes in `package.json` to include `browser_tests/` in
oxlint targets

---------

Co-authored-by: Amp <amp@ampcode.com>
Co-authored-by: GitHub Action <action@github.com>
2026-04-11 01:25:14 +00:00
Alexander Brown
0353524e6f refactor: standardize Page Object locators as public readonly instead of getters (#11135)
*PR Created by the Glary-Bot Agent*

---

## Summary

- Convert ~120 getter-based locators across 18 browser test fixture
files to `public readonly` constructor-assigned properties
- Removes unnecessary indirection, makes object shape explicit, and
improves IDE auto-complete / type inference
- Keeps lazy-init getters (`??=`), computed properties, and `private get
page()` convenience accessors as getters

## Changes

**`browser_tests/fixtures/components/`** (6 files):
`ComfyNodeSearchBox`, `ContextMenu`, `SettingDialog`, `SignInDialog`,
`SidebarTab` (all 6 classes), `Topbar`

**`browser_tests/fixtures/`** (4 files): `ComfyPage` (ComfyMenu.buttons,
ComfyPage.visibleToasts), `UserSelectPage`, `ComfyMouse`,
`VueNodeHelpers`

**`browser_tests/fixtures/helpers/`** (7 files): `AppModeHelper`,
`BuilderFooterHelper`, `BuilderSaveAsHelper`, `BuilderSelectHelper`,
`BuilderStepsHelper`, `ToastHelper`, `NodeOperationsHelper`

**`browser_tests/fixtures/utils/`** (1 file): `vueNodeFixtures`

## Validation

- `pnpm typecheck` 
- `pnpm typecheck:browser` 
- `pnpm exec eslint browser_tests/fixtures/` 
- All pre-commit hooks pass (oxfmt, oxlint, eslint, typecheck,
typecheck:browser) 
- No visual/manual verification needed — changes are test fixture
locator declarations only (no UI or runtime behavior change)

Fixes #11131

┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-11135-refactor-standardize-Page-Object-locators-as-public-readonly-instead-of-getters-33e6d73d3650819690cbc639f3d30daf)
by [Unito](https://www.unito.io)

---------

Co-authored-by: Glary-Bot <glary-bot@users.noreply.github.com>
2026-04-10 21:10:17 +00:00
Alexander Brown
0132c77c7d test: harden 82 Playwright specs for deterministic CI runs (#10967)
## Summary

Harden 98 E2E spec files and 8 fixtures/helpers for deterministic CI
runs by replacing race-prone patterns with retry-safe alternatives.

No source code changes -- only `browser_tests/` is touched.

## Changes

- **E2E spec hardening** (98 spec files, 6 fixtures, 2 helpers):

  | Fix class | Sites | Examples |
  |-----------|-------|---------:|
| `expect(await ...)` -> `expect.poll()` | ~153 | interaction,
defaultKeybindings, workflows, featureFlags |
| `const x = await loc.count(); expect(x)` -> `toHaveCount()` | ~19 |
menu, linkInteraction, assets, bottomPanelShortcuts |
| `nextFrame()` -> `waitForHidden()` after menu clicks | ~22 |
contextMenu, rightClickMenu, subgraphHelper |
| Redundant `nextFrame()` removed | many | defaultKeybindings, minimap,
builderSaveFlow |
| `expect(async () => { ... }).toPass()` retry blocks | 5 | interaction
(graphdialog dismiss guard) |
| `force:true` removed from `BaseDialog.close()` | 1 | BaseDialog
fixture |
| ContextMenu `waitForHidden` simplified (check-then-act race removed) |
1 | ContextMenu fixture |
| Non-deterministic node order -> proximity-based selection | 1 |
interaction (toggle dom widget) |
  | Tight poll timeout (250ms) -> >=2000ms | 2 | templates |

- **Helper improvements**: Exposed locator getters on
`ComfyPage.domWidgets`, `ToastHelper.toastErrors`, and
`WorkflowsSidebarTab.activeWorkflowLabel` so callers can use retrying
assertions (`toHaveCount()`, `toHaveText()`) directly.

- **Flake pattern catalog**: Added section 7 table to
`browser_tests/FLAKE_PREVENTION_RULES.md` documenting 8 pattern classes
for reviewers and future authors.

- **Docs**: Fixed bad examples in `browser_tests/README.md` to use
`expect.poll()`.

- **Breaking**: None
- **Dependencies**: None

## Review Focus

- All fixes follow the rules in
`browser_tests/FLAKE_PREVENTION_RULES.md`
- No behavioral changes to tests -- only timing/retry strategy is
updated
- The `ContextMenu.waitForHidden` simplification removes a
swallowed-error anti-pattern; both locators now use direct `waitFor({
state: 'hidden' })`

---------

Co-authored-by: Amp <amp@ampcode.com>
Co-authored-by: github-actions <github-actions@github.com>
2026-04-09 20:50:56 -07:00
Christian Byrne
9b769656ac test: add ShareWorkflow dialog E2E tests (DLG-05) (#10588)
## Summary
Adds Playwright E2E tests for the ShareWorkflow dialog component and its
various states.

## Tests added
- Dialog opens and shows unsaved state for new workflows (save prompt)
- Ready state shows create link button for saved but unpublished
workflows
- Shared state shows copy URL field with share link after publishing
- Stale state shows update link button when workflow modified after
publishing
- Close button dismisses dialog
- Create link transitions dialog from ready to shared state
- Tab switching between share link and publish to hub (when
comfyHubUploadEnabled)
- Tab aria-selected states update correctly on switch

## Approach
- Share dialog is gated behind `isCloud` (compile-time constant), so
tests invoke it directly via `page.evaluate()` importing
`useShareDialog`
- Share service API calls (`/api/userdata/*/publish`,
`/api/assets/from-workflow`) mocked via `page.route()` for deterministic
state testing
- Dialog state (loading → unsaved → ready → shared → stale) controlled
by mock responses
- Feature flags set via `serverFeatureFlags.value` for tab visibility
testing

## Notes
- All pre-existing TS2307 errors are `.vue` module resolution — no new
type errors
- Tests cover the 5 dialog states: loading, unsaved, ready, shared,
stale

## Task
Part of Test Coverage Q2 Overhaul (DLG-05).

## Conventions
- Uses Vue nodes with new menu enabled
- Tests read as user stories
- No full-page screenshots
- Proper waits, no sleeps
- All API calls mocked

┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-10588-test-add-ShareWorkflow-dialog-E2E-tests-DLG-05-3306d73d365081a0ab15f333707e493b)
by [Unito](https://www.unito.io)

---------

Co-authored-by: GitHub Action <action@github.com>
2026-04-08 14:14:47 -07:00
Alexander Brown
3b78dfbe1c test: migrate browser_tests/ to @e2e/ path alias and add lint rule (#10958)
## Summary

Complete the @e2e/ path alias migration started in #10735 by converting
all 354 remaining relative imports and adding a lint rule to prevent
backsliding.

## Changes

- **What**: Migrate all relative imports in browser_tests/ to use
`@e2e/` (intra-directory) and `@/` (src/ imports) path aliases. Add
`no-restricted-imports` ESLint rule banning `./` and `../` imports in
`browser_tests/**/*.ts`. Suppress pre-existing oxlint `no-eval` and
`no-console` warnings exposed by touching those files.

## Review Focus

- ESLint flat-config merging: the `@playwright/test` ban and
relative-import ban are in two separate blocks to avoid last-match-wins
collision with the `useI18n`/`useVirtualList` blocks higher in the
config.
- The `['./**', '../**']` glob patterns (not `['./*', '../*']`) are
needed to catch multi-level relative paths like `../../../src/foo`.

Follows up on #10735

┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-10958-test-migrate-browser_tests-to-e2e-path-alias-and-add-lint-rule-33c6d73d365081649d1be771eac986fd)
by [Unito](https://www.unito.io)

Co-authored-by: Amp <amp@ampcode.com>
2026-04-08 11:28:59 -07:00
Christian Byrne
83f4e7060a test(infra): AssetHelper with builder pattern + deterministic fixtures (#10545)
## What

Adds `AssetHelper` — a builder-pattern helper for mocking asset-related
API endpoints in Playwright E2E tests, plus deterministic fixture data.

## Why

12+ asset-related API endpoints need mocking for asset browser tests
(PNL-02), cloud dialog testing (DLG-08), and other asset-dependent E2E
scenarios. Random mock data from existing `createMockAssets()` is
unsuitable for deterministic E2E assertions.

## What's included

### `AssetHelper.ts` (307 LOC)
- Fluent builder API: `assetHelper.withModels(3).withImages(5).mock()`
- Stateful mock store (Map) for upload→verify→delete flows
- Endpoint coverage: GET/POST/PUT/DELETE `/assets`, download progress
- `mockError()` for error state testing
- `clearMocks()` cleanup matching QueueHelper/FeatureFlagHelper pattern

### `assetFixtures.ts` (304 LOC)
- 11 stable named constants (checkpoints, loras, VAE, embedding, inputs,
outputs)
- Factory functions: `generateModels()`, `generateInputFiles()`,
`generateOutputAssets()`
- Fixed IDs/dates/sizes — no randomness, safe for screenshot comparisons

### ComfyPage integration
- Available as `comfyPage.assets` in all tests

## Testing
- TypeScript compiles clean
- Follows existing QueueHelper/FeatureFlagHelper conventions

## Unblocks
- PNL-02: Asset browser tests (@Jaewon Yoon)
- DLG-08: Assets modal / cloud dialog testing

Part of: Test Coverage Q2 Overhaul

┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-10545-test-infra-AssetHelper-with-builder-pattern-deterministic-fixtures-32f6d73d365081d3985ef079ff3dbede)
by [Unito](https://www.unito.io)

---------

Co-authored-by: GitHub Action <action@github.com>
2026-04-07 14:23:36 -07:00
Dante
0d535631a5 refactor(test): extract dialog page objects from inline getByRole usage (#10822)
## Summary

Extract inline `getByRole('dialog')` calls across E2E tests into
reusable page objects.

## Changes

- **What**: Extract `ConfirmDialog` class from `ComfyPage.ts` into
`browser_tests/fixtures/components/ConfirmDialog.ts` with new `save`
button locator. Add `MediaLightbox` and `TemplatesDialog` page objects.
Refactor 4 test files to use these page objects instead of raw dialog
locators.
- **Skipped**: `appModeDropdownClipping.spec.ts` uses
`getByRole('dialog')` for a PrimeVue Popover (not a true dialog), left
as-is.

## Review Focus

The `ConfirmDialog.click()` method now supports a `save` action used by
`workflowPersistence.spec.ts`, which also waits for the dialog mask to
disappear and workflow service to settle.

Fixes #10723

┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-10822-refactor-test-extract-dialog-page-objects-from-inline-getByRole-usage-3366d73d365081b3bc0ee7ef0ddce658)
by [Unito](https://www.unito.io)
2026-04-06 09:49:32 -07:00
Dante
17d2870ef4 test(modelLibrary): add E2E tests for model library sidebar tab (#10789)
## Summary
- Add `ModelLibraryHelper` mock helper for `/experiment/models` and
`/view_metadata` endpoints
- Add `ModelLibrarySidebarTab` page object fixture with search, folder,
and leaf locators
- Add 11 E2E test scenarios covering tab open/close, folder display,
folder expansion, search with debounce, refresh, load all folders, and
empty state

## Test plan
- [ ] CI passes all Playwright shards
- [ ] `pnpm typecheck:browser` passes
- [ ] `pnpm lint` passes

┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-10789-test-modelLibrary-add-E2E-tests-for-model-library-sidebar-tab-3356d73d365081b49a7ed752512164da)
by [Unito](https://www.unito.io)

---------

Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-04-03 08:04:46 +09:00
Dante
794b986954 test: add E2E tests for Node Library V2 sidebar (#10798)
## Summary

- Adds Playwright E2E tests for the Node Library V2 sidebar tab
(`Comfy.NodeLibrary.NewDesign: true`)
- Adds `NodeLibrarySidebarTabV2` fixture class with V2-specific locators
(search input, tab buttons, node cards)
- Exposes `menu.nodeLibraryTabV2` on `ComfyPage` for test access
- Tests cover: tab visibility, default tab selection, tab switching,
folder expansion, search filtering, and sort button presence

## Test plan

- [ ] Run `pnpm test:browser:local -- --grep "Node library sidebar V2"`
against a running ComfyUI server with the V2 node library
- [ ] Verify tests pass in CI

Fixes #9079

┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-10798-test-add-E2E-tests-for-Node-Library-V2-sidebar-3356d73d36508185a11feaf95e32225b)
by [Unito](https://www.unito.io)

---------

Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-04-03 07:04:56 +09:00
Dante
380fae9a0d chore(test): remove dead QueueHelper from browser tests (#10771)
## Summary
- Remove unused `QueueHelper` class and its `comfyPage.queue` property
- `QueueHelper` mocks the legacy `/api/queue` tuple format which the app
no longer uses (now `/api/jobs` via `fetchQueue()`)
- `comfyPage.queue.*` is never called in any test

Fixes #10670

┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-10771-chore-test-remove-dead-QueueHelper-from-browser-tests-3346d73d36508117bb19db9492bcbed3)
by [Unito](https://www.unito.io)
2026-03-31 19:55:52 +09:00
Benjamin Lu
86a3938d11 test: add runtime-safe browser_tests alias (#10735)
## What changed

Added a runtime-safe `#e2e/*` alias for `browser_tests`, updated the
browser test docs, and migrated a representative fixture/spec import
path to the new convention.

## Why

`@/*` only covers `src/`, so browser test imports were falling back to
deep relative paths. `#e2e/*` resolves in both Node/Playwright runtime
and TypeScript.

## Validation

- `pnpm format`
- `pnpm typecheck:browser`
- `pnpm exec playwright test browser_tests/tests/actionbar.spec.ts
--list`

┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-10735-test-add-runtime-safe-browser_tests-alias-3336d73d36508122b253cb36a4ead1c1)
by [Unito](https://www.unito.io)

---------

Co-authored-by: Alexander Brown <drjkl@comfy.org>
2026-03-30 19:24:09 +00:00
Christian Byrne
81d3ef22b0 refactor: extract comfyExpect and makeMatcher from ComfyPage (#10652)
## Summary

Extract `makeMatcher` and `comfyExpect` from `ComfyPage.ts` into the
standalone `browser_tests/fixtures/utils/customMatchers.ts` module,
reducing the page-object file by ~50 lines.

## Changes

- **What**: Removed duplicate `makeMatcher`/`comfyExpect` definitions
from `ComfyPage.ts`; the canonical implementation now lives in
`customMatchers.ts`. A backward-compatible re-export keeps all existing
imports working.

## Review Focus

- The re-export ensures `import { comfyExpect } from
'../fixtures/ComfyPage'` continues to resolve for all ~25 spec files.

┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-10652-refactor-extract-comfyExpect-and-makeMatcher-from-ComfyPage-3316d73d365081bf8e7cd7fa324bf9a6)
by [Unito](https://www.unito.io)

---------

Co-authored-by: Alexander Brown <drjkl@comfy.org>
Co-authored-by: GitHub Action <action@github.com>
2026-03-29 16:05:10 -07:00
Christian Byrne
2d99fb446c test: add QueueClearHistoryDialog E2E tests (DLG-02) (#10586)
## Summary
Adds Playwright E2E tests for the QueueClearHistoryDialog component.

## Tests added
- Dialog opens from queue panel history actions menu
- Dialog shows confirmation message with title, description, and assets
note
- Cancel button closes dialog without clearing history
- Close (X) button closes dialog without clearing history
- Confirm clear action triggers queue history clear API call
- Dialog state resets properly after close/reopen

## Task
Part of Test Coverage Q2 Overhaul (DLG-02).

## Conventions
- Uses Vue nodes with new menu enabled (`Comfy.UseNewMenu: 'Top'`)
- Tests read as user stories
- No full-page screenshots
- Proper waits, no sleeps

┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-10586-test-add-QueueClearHistoryDialog-E2E-tests-DLG-02-3306d73d36508174a07bd9782340a0f7)
by [Unito](https://www.unito.io)

---------

Co-authored-by: GitHub Action <action@github.com>
2026-03-29 16:02:19 -07:00
Christian Byrne
af0f7cb945 refactor: extract assetPath as standalone pure function (#10651)
## Summary

Extract `assetPath` from a `ComfyPage` method to a standalone pure
function, removing unnecessary coupling to the page object.

## Changes

- **What**: Moved `assetPath` to
`browser_tests/fixtures/utils/paths.ts`. `DragDropHelper` and
`WorkflowHelper` import it directly instead of receiving it via
`ComfyPage`. `ComfyPage.assetPath` kept as thin delegate for backward
compat.

## Review Focus

Structural-only refactor — no behavioral changes. The function was
already pure (no `this`/`page` usage).

┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-10651-refactor-extract-assetPath-as-standalone-pure-function-3316d73d365081c0b0e0ce6dde57ef8e)
by [Unito](https://www.unito.io)
2026-03-29 00:27:28 -07:00
Benjamin Lu
db6e5062f2 test: add assets sidebar empty-state coverage (#10595)
## Summary

Add the first user-centric Playwright coverage for the assets sidebar
empty state and introduce a small assets-specific test helper/page
object surface.

## Changes

- **What**: add `AssetsSidebarTab`, add `AssetsHelper`, and cover
generated/imported empty states in a dedicated browser spec

## Review Focus

This is intentionally a small first slice for assets-sidebar coverage.
The new helper still mocks the HTTP boundary in Playwright for now
because current OSS job history and input files are global backend
state, which makes true backend-seeded parallel coverage a separate
backend change.

Long-term recommendation: add backend-owned, user-scoped test seeding
for jobs/history and input assets so browser tests can hit the real
routes on a shared backend. Follow-up: COM-307.

Fixes COM-306

## Screenshots (if applicable)

Not applicable.

## Validation

- `pnpm typecheck:browser`
- `pnpm exec playwright test browser_tests/tests/sidebar/assets.spec.ts
--project=chromium` against an isolated preview env

┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-10595-test-add-assets-sidebar-empty-state-coverage-3306d73d365081d1b34fdd146ae6c5c6)
by [Unito](https://www.unito.io)
2026-03-26 21:19:38 -07:00
Dante
98a9facc7d tool: add CSS containment audit skill and Playwright diagnostic (#10026)
## Summary

- Add a Playwright-based diagnostic tool (`@audit` tagged) that
automatically detects DOM elements where CSS `contain: layout style`
would improve rendering performance
- Extend `ComfyPage` fixture and `playwright.config.ts` to support
`@audit` tag (excluded from CI, perf infra enabled)
- Add `/contain-audit` skill definition documenting the workflow

## How it works

1. Loads the 245-node workflow in a real browser
2. Walks the DOM tree and scores every element by subtree size and
sizing constraints
3. For each high-scoring candidate, applies `contain: layout style` via
JS
4. Measures rendering performance (style recalcs, layouts, task
duration) before and after
5. Takes before/after screenshots to detect visual breakage
6. Outputs a ranked report to console

## Test plan

- [ ] `pnpm typecheck` passes
- [ ] `pnpm typecheck:browser` passes
- [ ] `pnpm lint` passes
- [ ] Existing Playwright tests unaffected (`@audit` excluded from CI
via `grepInvert`)
- [ ] Run `pnpm exec playwright test
browser_tests/tests/containAudit.spec.ts --project=chromium` locally
with dev server

┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-10026-tool-add-CSS-containment-audit-skill-and-Playwright-diagnostic-3256d73d365081b29470df164f798f7d)
by [Unito](https://www.unito.io)

---------

Co-authored-by: GitHub Action <action@github.com>
Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-03-27 10:08:41 +09:00
Benjamin Lu
6b7691422b fix: use named dotenv config imports (#10514)
## Summary

Use named `dotenv` config imports where we were calling
`dotenv.config()` so ESLint and IDEs stop flagging
`import-x/no-named-as-default-member`.

## Changes

- **What**: Replace default `dotenv` imports plus `.config()` member
access with `import { config as dotenvConfig } from 'dotenv'` in browser
test setup/fixture files and the desktop Vite config.
- **What**: Keep behavior unchanged while aligning those files with the
cleaner import form already used elsewhere in the repo.

## Review Focus

This is a no-behavior-change cleanup. The issue was that `dotenv`
exposes `config` both as a named export and as a property on the
default-exported module object, so `import dotenv from 'dotenv';
dotenv.config()` triggers `import-x/no-named-as-default-member` even
though it works at runtime.

┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-10514-fix-use-named-dotenv-config-imports-32e6d73d36508195b346dbcab764a6b8)
by [Unito](https://www.unito.io)
2026-03-25 12:10:20 -07:00
Johnpaul Chiwetelu
8aab2889b7 test: add copy/paste node and image paste e2e tests (#10233)
## Summary
- Add e2e test covering node copy/paste (Ctrl+C/V), image paste onto a
selected LoadImage node, and image paste on empty canvas creating a new
LoadImage node
- Extract `simulateImagePaste` into reusable
`ClipboardHelper.pasteFile()` with auto-detected filename and MIME type
- Add test workflow asset `load_image_with_ksampler.json` and
`image32x32.webp` image asset

## Test plan
- [ ] `pnpm typecheck:browser` passes
- [ ] `pnpm exec eslint browser_tests/tests/copyPaste.spec.ts` passes
- [ ] New test passes in CI: `Copy paste node, image paste onto
LoadImage, image paste on empty canvas`
- [ ] Existing copy/paste tests unaffected

┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-10233-test-add-copy-paste-node-and-image-paste-e2e-tests-3276d73d365081e78bb9f3f1bf34389f)
by [Unito](https://www.unito.io)
2026-03-20 03:24:09 +01:00
Alexander Brown
6dfc7b4306 perf: eliminate double page navigation in Playwright test setup (#10313)
## Summary

Eliminate the double `goto()` in `ComfyPage.setup()` by using
`addInitScript` to seed localStorage before the first navigation.

## Changes

- **What**: Move route mocking and localStorage seeding before `goto()`,
replacing `page.evaluate` with `page.addInitScript` so values are set
before app JS executes on first load. Removes the second `goto()` call
entirely.

## Review Focus

- Verify all Playwright E2E shards pass — the `clearStorage: false` and
`mockReleases: false` paths should be unaffected.
- Note: `addInitScript` persists for the page lifetime (like
`FeatureFlagHelper.seedFlags` already does), so any subsequent
`page.reload()` or `goto()` in tests will also clear storage. This
should be fine since tests that use `clearStorage: false` skip this
block.

┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-10313-perf-eliminate-double-page-navigation-in-Playwright-test-setup-3286d73d36508158a8edeefb27fcae20)
by [Unito](https://www.unito.io)

---------

Co-authored-by: Amp <amp@ampcode.com>
2026-03-19 16:53:07 -07:00
pythongosssss
77ddda9d3c fix: App mode - renaming widgets on subgraphs (#10245)
## Summary

Fixes renaming of widgets from subgraph nodes in app builder/app mode.

## Changes

- **What**: If the widget is from a subgraph node and no parents passed,
use the node as the subgraph parent.

┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-10245-fix-App-mode-renaming-widgets-on-subgraphs-3276d73d3650815bb131c840df43cdf2)
by [Unito](https://www.unito.io)
2026-03-19 04:00:31 -07:00
Christian Byrne
eaf1f9ac77 test: add FeatureFlagHelper and QueueHelper for E2E test infrastructure (#9554)
## Summary

Add 2 reusable test helpers for Playwright E2E tests, integrated into
the ComfyPage fixture. These provide standardized patterns for mocking
feature flags and queue state across all E2E tests.

## Changes

- **`FeatureFlagHelper.ts`** — manage localStorage `ff:` prefixed
feature flags (`seedFlags` for init-time, `setFlags` for runtime) and
mock `/api/features` route
- **`QueueHelper.ts`** — mock `/api/queue` and `/api/history` routes
with configurable running/pending counts and success/error job entries
- **`ComfyPage.ts`** — integrate both helpers as
`comfyPage.featureFlags` and `comfyPage.queue`

## Review Focus

- Helper API design: are `seedFlags`/`setFlags`/`mockServerFeatures` the
right abstractions for feature flag testing?
- Queue mock fidelity: does the mock history shape match real ComfyUI
API responses closely enough?
- These are test-only infrastructure — no production code changes.

## Stack

This is the base PR for the Playwright E2E coverage stack. Waves 1-4 all
branch from this and can merge independently once this lands:
- **→ This PR**: Test infrastructure helpers
- #9555: Toasts, error overlay, selection toolbox, linear mode,
selection rectangle
- #9556: Node search, bottom panel, focus mode, job history, side panel
- #9557: Errors tab, node headers, queue notifications, settings sidebar
- #9558: Minimap, widget copy, floating menus, node library essentials

---------

Co-authored-by: GitHub Action <action@github.com>
2026-03-16 06:51:22 -07:00
jaeone94
e2ef041170 feat: surface missing models in Error Tab for OSS and remove legacy dialog (#9921)
## Summary
- Surface missing models in the Error Tab for OSS environments,
replacing the legacy modal dialog
- Add Download button per model and Download All button in group header
with file size display
- Move download business logic from `components/dialog/content` to
`platform/missingModel`
- Remove legacy missing models dialog components and composable

## Changes
- **Pipeline**: Remove `isCloud` guard from `scanAllModelCandidates` and
`surfaceMissingModels` so OSS detects missing models
- **Grouping**: Group non-asset-supported models by directory in OSS
instead of lumping into UNSUPPORTED
- **UI**: Add Download button (matching Install Node Pack design) and
Download All header button
- **Store**: Add `folderPaths`/`fileSizes` state with setter methods,
race condition guard
- **Cleanup**: Delete `MissingModelsContent`, `MissingModelsHeader`,
`MissingModelsFooter`, `useMissingModelsDialog`, `missingModelsUtils`
- **Tests**: Add OSS/Cloud grouping tests, migrate Playwright E2E to
Error Tab, improve test isolation
- **Snapshots**: Reset Playwright screenshot expectations since OSS
missing model error detection now causes red highlights on affected
nodes
- **Accessibility**: Add `aria-label` with model name, `aria-expanded`
on toggle, warning icon for unknown category

## Test plan
- [x] Unit tests pass (86 tests)
- [x] TypeScript typecheck passes
- [x] knip passes
- [x] Load workflow with missing models in OSS → Error Tab shows missing
models grouped by directory
- [x] Download button triggers browser download with correct URL
- [x] Download All button downloads all downloadable models
- [x] Cloud environment behavior unchanged
- [x] Playwright E2E: `pnpm test:browser:local -- --grep "Missing models
in Error Tab"`

## Screenshots


https://github.com/user-attachments/assets/12f15e09-215a-4c58-87ed-39bbffd1359c

 


┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-9921-feat-surface-missing-models-in-Error-Tab-for-OSS-and-remove-legacy-dialog-3236d73d365081f0a9dfc291978f5ecf)
by [Unito](https://www.unito.io)

---------

Co-authored-by: GitHub Action <action@github.com>
Co-authored-by: github-actions <github-actions@github.com>
2026-03-15 22:46:47 +09:00
Benjamin Lu
1ada6dbfc6 fix: align run controls with queue modal design (#9134)
## Summary
- move queue batch controls to the left of the run button
- align run control styling to the Figma queue modal spec using PrimeVue
PT/Tailwind (secondary background on batch + dropdown, primary run
button)
- normalize control heights to match actionbar buttons and tighten
dropdown hit area
- update run typography/spacing and replace all three chevrons (dropdown
+ batch up/down) with the requested SVG

Design:
https://www.figma.com/design/LVilZgHGk5RwWOkVN6yCEK/Queue-Progress-Modal?node-id=3845-23904&m=dev

<img width="303" height="122" alt="image"
src="https://github.com/user-attachments/assets/4ed80ee7-3ceb-4512-96ce-f55ec6da835e"
/>


┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-9134-fix-align-run-controls-with-queue-modal-design-3106d73d36508160afcedbcfe4b98291)
by [Unito](https://www.unito.io)

---------

Co-authored-by: github-actions <github-actions@github.com>
Co-authored-by: GitHub Action <action@github.com>
2026-03-06 20:10:59 -08:00
Benjamin Lu
3366079f59 test: disable missing model warnings in browser tests (#9513)
Disable missing model warnings in browser tests by default.

Browser tests run without model files on disk, so workflows that embed
model metadata can render differently in CI than the test actually
intends to cover. The viewport screenshot golden had started depending
on the missing-model popup even though the test is only about restoring
an offscreen viewport.

Set `Comfy.Workflow.ShowMissingModelsWarning` to `false` in the shared
Playwright fixture, keep the missing-model dialog coverage by explicitly
enabling the setting in the dialog tests, and update the viewport
screenshot expectation to the no-popup rendering.

┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-9513-test-disable-missing-model-warnings-in-browser-tests-31b6d73d365081d1908bfe11ec0c3bc2)
by [Unito](https://www.unito.io)
2026-03-06 17:37:50 -08:00
Christian Byrne
8e215b3174 feat: add performance testing infrastructure with CDP metrics (#9170)
## Summary

Add a permanent, non-failing performance regression detection system
using Chrome DevTools Protocol metrics, with automatic PR commenting.

## Changes

- **What**: Performance testing infrastructure — `PerformanceHelper`
fixture class using CDP `Performance.getMetrics` to collect
`RecalcStyleCount`, `LayoutCount`, `LayoutDuration`, `TaskDuration`,
`JSHeapUsedSize`. Adds `@perf` Playwright project (Chromium-only,
single-threaded, 60s timeout), 4 baseline perf tests, CI workflow with
sticky PR comment reporting, and `perf-report.js` script for generating
markdown comparison tables.

## Review Focus

- `PerformanceHelper` uses `page.context().newCDPSession(page)` — CDP is
Chromium-only, so perf metrics are not collected on Firefox. This is
intentional since CDP gives us browser-level style recalc/layout counts
that `performance.mark/measure` cannot capture.
- The CI workflow uses `continue-on-error: true` so perf tests never
block merging.
- Baseline comparison uses `dawidd6/action-download-artifact` to
download metrics from the target branch, following the same pattern as
`pr-size-report.yaml`.

## Stack

This is the foundation PR for the Firefox performance fix stack:
1. **→ This PR: perf testing infrastructure**
2. `perf/fix-cursor-cache` — cursor style caching (depends on this)
3. `perf/fix-subgraph-svg` — SVG pre-rasterization (depends on this)
4. `perf/fix-clippath-raf` — RAF batching for clip-path (depends on
this)

PRs 2-4 are independent of each other.

┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-9170-feat-add-performance-testing-infrastructure-with-CDP-metrics-3116d73d3650817cb43def6f8e9917f8)
by [Unito](https://www.unito.io)

---------

Co-authored-by: GitHub Action <action@github.com>
Co-authored-by: Alexander Brown <drjkl@comfy.org>
2026-02-25 20:09:57 -08:00
Dante
bd95150f82 test: remove unused DebugHelper from browser_tests (#9017)
## Summary

- Remove unused `DebugHelper` class and its import from browser test
fixtures

Fixes #8581

## Test plan

- [x] Verify browser tests still pass without `DebugHelper`

┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-9017-test-remove-unused-DebugHelper-from-browser_tests-30d6d73d36508158b8c9fc83670df4f3)
by [Unito](https://www.unito.io)
2026-02-22 11:52:54 +09:00
pythongosssss
6902e38e6a V2 Node Search (+ hidden Node Library changes) (#8987)
## Summary

Redesigned node search with categories

## Changes

- **What**: Adds a v2 search component, leaving the existing
implementation untouched
- It also brings onboard the incomplete node library & preview changes,
disabled and behind a hidden setting
- **Breaking**: Changes the 'default' value of the node search setting
to v2, adding v1 (legacy) as an option

## Screenshots (if applicable)




https://github.com/user-attachments/assets/2ab797df-58f0-48e8-8b20-2a1809e3735f

┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-8987-V2-Node-Search-hidden-Node-Library-changes-30c6d73d36508160902bcb92553f147c)
by [Unito](https://www.unito.io)

---------

Co-authored-by: Yourz <crazilou@vip.qq.com>
Co-authored-by: Amp <amp@ampcode.com>
Co-authored-by: github-actions <github-actions@github.com>
Co-authored-by: GitHub Action <action@github.com>
Co-authored-by: Christian Byrne <cbyrne@comfy.org>
2026-02-20 01:10:03 -08:00
Christian Byrne
278d491030 refactor: move ellipsis and punctuation into i18n translation strings (#8573)
## Summary

Move ellipsis and punctuation characters into i18n translation strings
for proper internationalization support.

## Changes

- Add 12 new translation keys with punctuation included:
- Placeholder keys with trailing ellipsis (e.g.,
`searchNodesPlaceholder: "Search Nodes..."`)
  - `downloadWithSize` with interpolation: `"Download ({size})"`
  - `completedWithCheckmark`: `"Completed ✓"`
- Prompt keys with colons (e.g., `enterNewNamePrompt: "Enter new
name:"`)
- Update 20 files to use new translation keys instead of string
concatenation

## Review Focus

This eliminates string concatenation patterns like `$t('key') + '...'`
that break proper internationalization, since different languages may
use different punctuation or may not need ellipsis/colons in the same
contexts.

Fixes #7333


<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit

* **Chores**
* Standardized localization across the app: unified search placeholders
and input hints; updated dialog prompt texts for renaming,
saving/exporting, and related prompts.
* **New Features**
  * Download buttons now show file size via localized text.
  * Completed status displays a localized label with a checkmark.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->

┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-8573-refactor-move-ellipsis-and-punctuation-into-i18n-translation-strings-2fc6d73d365081828ad3f257bcac7799)
by [Unito](https://www.unito.io)
2026-02-03 15:50:18 -08:00
Alexander Brown
f2d5bfab73 test(browser): refactor browser tests for reliability and maintainability (#8510)
## Summary

Major refactoring of browser tests to improve reliability,
maintainability, and type safety.

## Changes

### Test Infrastructure Decomposition
- Decomposed `ComfyPage.ts` (~1000 lines) into focused helpers:
- `CanvasHelper`, `DebugHelper`, `SubgraphHelper`,
`NodeOperationsHelper`
- `SettingsHelper`, `WorkflowHelper`, `ClipboardHelper`,
`KeyboardHelper`
- Created `ContextMenu` page object, `BaseDialog` base class, and
`BottomPanel` page object
- Extracted `DefaultGraphPositions` constants

### Locator Stability
- Added `data-testid` attributes to Vue components (sidebar, dialogs,
node library)
- Created centralized `selectors.ts` with test ID constants
- Replaced fragile CSS selectors (`.nth()`, `:nth-child()`) with
`getByTestId`/`getByRole`

### Performance & Reliability
- Removed `setTimeout` anti-patterns (replaced with `waitForFunction`)
- Replaced `waitForTimeout` with retrying assertions
- Replaced hardcoded coordinates with computed `NodeReference` positions
- Enforced LF line endings for all text files

### Type Safety
- Enabled `no-explicit-any` lint rule for browser_tests via oxlint
- Purged `as any` casts from browser_tests
- Added Window type augmentation for standardized window access
- Added proper type annotations throughout

### Bug Fixes
- Restored `ExtensionManager` API contract
- Removed test-only settings from production schema
- Fixed flaky selectors and missing test setup

## Testing
- All browser tests pass
- Typecheck passes


<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit

* **Tests**
* Overhauled browser E2E test infrastructure with many new
helpers/fixtures, updated test APIs, and CI test container image bumped
for consistency.

* **Chores**
* Standardized line endings and applied stricter lint rules for browser
tests; workspace dependency version updated.

* **Documentation**
* Updated Playwright and TypeScript testing guidance and test-run
commands.

* **UI**
* Added stable data-testids to multiple components to improve
testability.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->

---------

Co-authored-by: Amp <amp@ampcode.com>
Co-authored-by: GitHub Action <action@github.com>
Co-authored-by: github-actions <github-actions@github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
2026-02-03 12:29:40 -08:00
Christian Byrne
47113b117e refactor: migrate keybindings to DDD structure (#8369)
## Summary

Migrate keybindings domain to `src/platform/keybindings/` following DDD
principles.

## Changes

- **What**: Consolidate keybinding-related code (types, store, service,
defaults, reserved keys) into a single domain module with flat structure
- Extracted `KeyComboImpl` and `KeybindingImpl` classes into separate
files
- Updated all consumers to import from new location
- Colocated tests with source files
- Updated stores/README.md and services/README.md to remove migrated
entries

## Review Focus

- Verify all import paths were updated correctly
- Check that the flat structure is appropriate (vs nested core/data/ui
layers)

┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-8369-refactor-migrate-keybindings-to-DDD-structure-2f66d73d36508120b169dc737075fb45)
by [Unito](https://www.unito.io)

---------

Co-authored-by: Subagent 5 <subagent@example.com>
Co-authored-by: Amp <amp@ampcode.com>
2026-01-29 18:40:58 -08:00
ric-yu
c0a649ef43 refactor: encapsulate error extraction in TaskItemImpl getters (#7650)
## Summary
- Add `errorMessage` and `executionError` getters to `TaskItemImpl` that
extract error info from status messages
- Update `useJobErrorReporting` composable to use these getters instead
of standalone function
- Remove the standalone `extractExecutionError` function

This encapsulates error extraction within `TaskItemImpl`, preparing for
the Jobs API migration where the underlying data format will change but
the getter interface will remain stable.

## Test plan
- [x] All existing tests pass
- [x] New tests added for `TaskItemImpl.errorMessage` and
`TaskItemImpl.executionError` getters
- [x] TypeScript, lint, and knip checks pass

🤖 Generated with [Claude Code](https://claude.com/claude-code)

┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-7650-refactor-encapsulate-error-extraction-in-TaskItemImpl-getters-2ce6d73d365081caae33dcc7e1e07720)
by [Unito](https://www.unito.io)

---------

Co-authored-by: Claude <noreply@anthropic.com>
Co-authored-by: Christian Byrne <cbyrne@comfy.org>
2026-01-15 21:11:22 -07:00
pythongosssss
ebca0cb1e0 Fix vue node slot position calculation (#7877)
## Summary

Fix slot position for Vue nodes using LiteGraph calculation

Fixes #7446

## Changes
- Updated getInput/OutputPos to call getSlotPosition which handles
positions for both LiteGraph & Vue nodes correctly
- Add regression test

┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-7877-Fix-vue-node-slot-position-calculation-2e16d73d365081219afef0e4ebfb0620)
by [Unito](https://www.unito.io)

---------

Co-authored-by: github-actions <github-actions@github.com>
2026-01-13 20:40:21 -07:00
Alexander Brown
7b68b19f11 Component: The Rest of the PrimeVue buttons (#7649)
## Summary

Automated initial change, cleaned up manually.

Please check the screenshot changes.

Includes a11y updates to icon buttons.

Doesn't hit the buttons in Desktop.

┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-7649-WIP-Component-The-Rest-of-the-PrimeVue-buttons-2ce6d73d365081d68e06f200f1321267)
by [Unito](https://www.unito.io)

---------

Co-authored-by: GitHub Action <action@github.com>
Co-authored-by: github-actions <github-actions@github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
2025-12-29 15:03:34 -08:00
Christian Byrne
585d46d4fb fix: inner groups being moved double when moving outer group (in vue mode) (#7447)
## Summary

Fixes issue when dragging a group that had inner groups when in vue
mode.

When dragging the outer group in Vue mode:

1. getAllNestedItems(selected) returns ALL items: outer group + inner
groups + nodes
2. moveChildNodesInGroupVueMode loops through all items
3. For outer group G1: calls G1.move(delta, true) then
moveGroupChildren(G1, ...)
4. moveGroupChildren calls G2.move(delta) (no skipChildren) - this moves
G2 AND G2's children!
5. Then the loop reaches G2: calls G2.move(delta, true) - moves G2 again
6. Plus moveGroupChildren(G2, ...) processes G2's children again

This PR fixes it by adding `skipChildren=true` to the `move` call.

┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-7447-fix-inner-groups-being-moved-double-when-moving-outer-group-in-vue-mode-2c86d73d365081ce97abec682f2a8518)
by [Unito](https://www.unito.io)
2025-12-14 18:37:29 -08:00
Christian Byrne
97fa128999 fix: flaky e2e test for dropping assets on nodes (#7352)
Fix flaky "Can drag-and-drop animated webp image" test that was reading
the widget value before the upload completed, causing intermittent
failures where filenames appeared truncated. Added `waitForUpload`
option to `dragAndDropFile` helper that waits for the `/upload/`
response before returning. This is opt-in since not all drag-and-drop
operations trigger uploads (e.g., loading workflows from media files).
2025-12-11 11:48:02 -08:00
Johnpaul Chiwetelu
04286c033a hotfix: stabilize flaky workflow sidebar browser tests (#7280)
## Summary
- Fix flaky workflow sidebar browser tests that were failing in headless
mode
- Add retry logic for menu hover operations in Topbar
- Add proper timing/wait helpers for dialog masks and workflow service
completion
- Fix test isolation issues in setupWorkflowsDirectory and drop workflow
test

## Test plan
- [x] Run `pnpm test:browser --
browser_tests/tests/sidebar/workflows.spec.ts` multiple times
- [x] Verify the 3 previously failing tests now pass consistently:
  - "Can overwrite other workflows with save as"
  - "Can rename nested workflow from opened workflow item"  
  - "Can drop workflow from workflows sidebar"

┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-7280-hotfix-stabilize-flaky-workflow-sidebar-browser-tests-2c46d73d365081c5b3badfafe35a63dc)
by [Unito](https://www.unito.io)

---------

Co-authored-by: Terry Jia <terryjia88@gmail.com>
Co-authored-by: Alexander Brown <drjkl@comfy.org>
Co-authored-by: Luke Mino-Altherr <luke@comfy.org>
Co-authored-by: Claude <noreply@anthropic.com>
Co-authored-by: GitHub Action <action@github.com>
Co-authored-by: Benjamin Lu <benjaminlu1107@gmail.com>
2025-12-09 23:30:40 -07:00
Alexander Brown
5139e0564e Style: Font Consistency (#7220)
## Summary

Reduce lower level font definitions in most places. Default to Inter.

See #6912 

## Review Focus

Comic Sans is still an option...

## Screenshots (if applicable)

<!-- Add screenshots or video recording to help explain your changes -->

┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-7220-Style-Font-Consistency-2c26d73d365081348f2dd8909dd9bb8f)
by [Unito](https://www.unito.io)

---------

Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: github-actions <github-actions@github.com>
2025-12-08 19:48:11 -07:00
Alexander Brown
5c01861f4e Tests: Playwright test timeouts (#7231)
## Summary

See where we can use proper DOM waiting instead of waitForTimeout.

┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-7231-WIP-Playwright-test-timeouts-2c36d73d3650812b966ac3d9c338dfd4)
by [Unito](https://www.unito.io)
2025-12-08 15:50:35 -08:00
Alexander Brown
f74c176423 Cleanup: Properties Panel (#7137)
## Summary

- Code cleanup
- Copy, padding, color, alignment of components
- Subgraph Edit mode changes
- Partial fix for the Node Info location (need to do context menu still)
- Editing node title

### Still to-do

- Bi-directionality in values

┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-7137-WIP-Cleanup-Properties-Panel-2be6d73d3650813e9430f6bcb09dfb4d)
by [Unito](https://www.unito.io)

---------

Co-authored-by: github-actions <github-actions@github.com>
2025-12-05 21:33:52 -08:00
Christian Byrne
2b751200be test: add setting to ignore version compatibility toast warnings in e2e tests (#7004)
There's a warning toast shown if the frontend is considered out-of-date
(relative to the version in the requirements.txt of
comfyanonymous/ComfyUI). As a result, e2e tests run on older release
branches (e.g., when backporting or hotfixing) can sometimes trigger the
warning which obviously causes visual regression tests to fail. This PR
adds a hidden setting to disable the warning and sets it to `true` in
the e2e test fixtures.

┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-7004-test-add-setting-to-ignore-version-compatibility-toast-warnings-in-e2e-tests-2b86d73d3650812d9e07f54a0c86b996)
by [Unito](https://www.unito.io)
2025-11-29 19:56:19 -07:00
Alexander Brown
9da82f47ef Feat: Alt+Drag to clone - Vue Nodes (#6789)
## Summary

Replicate the alt+drag to clone behavior present in litegraph.

## Changes

- **What**: Simplify the interaction/drag handling, now with less state!
- **What**: Alt+Click+Drag a node to clone it

## Screenshots (if applicable)



https://github.com/user-attachments/assets/469e33c2-de0c-4e64-a344-1e9d9339d528



<!-- Add screenshots or video recording to help explain your changes -->

┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-6789-WIP-Alt-Drag-to-clone-Vue-Nodes-2b16d73d36508102a871ffe97ed2831f)
by [Unito](https://www.unito.io)

---------

Co-authored-by: github-actions <github-actions@github.com>
2025-11-21 14:16:03 -08:00
Benjamin Lu
26578981d4 Remove queue sidebar tab (#6724)
## Summary
- drop the queue sidebar entry, its component, and the supporting
composable so only the overlay-based queue UI remains
- clean up the related tests and keybindings so nothing references the
removed tab
- prune the unused queue task card components to keep the repo tidy
- remove unused queue sidebar translations and command strings across
all locales

## Testing
- pnpm typecheck
- pnpm lint:fix

┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-6724-Remove-queue-sidebar-tab-2ae6d73d3650811db0d4c5ad4c5ffc8d)
by [Unito](https://www.unito.io)

---------

Co-authored-by: pythongosssss <125205205+pythongosssss@users.noreply.github.com>
Co-authored-by: github-actions <github-actions@github.com>
Co-authored-by: Jin Yi <jin12cc@gmail.com>
Co-authored-by: Claude <noreply@anthropic.com>
Co-authored-by: GitHub Action <action@github.com>
Co-authored-by: Alexander Brown <drjkl@comfy.org>
Co-authored-by: Johnpaul Chiwetelu <49923152+Myestery@users.noreply.github.com>
Co-authored-by: Christian Byrne <cbyrne@comfy.org>
Co-authored-by: Comfy Org PR Bot <snomiao+comfy-pr@gmail.com>
Co-authored-by: christian-byrne <72887196+christian-byrne@users.noreply.github.com>
2025-11-19 19:50:24 -07:00
Simula_r
f629d325b2 Feat/vue nodes arrange alg (#6212)
## Summary

### Problem: 
The Vue nodes renderer/feature introduces new designs for each node i.e.
the equivalent Litegraph node design is smaller and the vue node design
is non uniformly larger.

### Example: 

Litegraph Ksampler node: 200w x 220h

<img width="200" height="220" alt="image"
src="https://github.com/user-attachments/assets/eef0117b-7e02-407d-98ab-c610fd1ec54c"
/>

Vue Node Ksampler node: 445w x 430h 

<img width="445" height="430" alt="image"
src="https://github.com/user-attachments/assets/e78d9d45-5b32-4e8d-bf1c-bce1c699037f"
/>

This means if users load a workflow in Litegraph and then switches to
Vue nodes renderer the nodes are using the same Litegraph positions
which would cause a visual overlap and overall look broken.

### Example:

<img width="1510" height="726" alt="image"
src="https://github.com/user-attachments/assets/3b7ae9d2-6057-49b2-968e-c531a969fac4"
/>
<img width="1475" height="850" alt="image"
src="https://github.com/user-attachments/assets/ea10f361-09bd-4daa-97f1-6b45b5dde389"
/>

### Solution:

Scale the positions of the nodes in lite graph radially from the center
of the bounds of all nodes. And then simply move the Vue nodes to those
new positions.

1. Get the `center of the bounds of all LG nodes`.
2. Get the `xy of each LG node`.
3. Get the vector from `center of the bounds of all LG nodes` `-` `xy of
each LG node`.
4. Scale it by a factor (e.g. 1.75x which is the average Vue node size
increase plus some visual padding.)
5. Move each Vue node to the scaled `xy of each LG node`. 

Result: The nodes are spaced apart removing overlaps while keeping the
spatial layout intact.

<img width="2173" height="1096" alt="image"
src="https://github.com/user-attachments/assets/7817d866-4051-47bb-a589-69ca77a0bfd3"
/>

### Further concerns.

This vector scaling algorithm needs to run once per workflow when in vue
nodes. This means when in Litegraph and switching to Vue nodes, it needs
to run before the nodes render. And then now that the entire app is in
vue nodes, we need to run it each time we load a workflow. However, once
its run, we do not need to run it again. Therefore we must persist a
flag that it has run somewhere. This PR also adds that feature by
leveraging the `extra` field in the workflow schema.

---------

Co-authored-by: GitHub Action <action@github.com>
Co-authored-by: JakeSchroeder <jake@axiom.co>
2025-10-28 14:14:39 -07:00
pythongosssss
984ebef416 Floating Menus - UI rework (#5980)
## Summary

Enhancing and further modernizing the UI, giving users more usable area
whilst keeping farmiliar positioning and feel of elements.

## Changes

- **What**: Significant restructure of the UI elements, changing
elements from large blocks to floating elements, updating:
- Side toolbar menu (floating style, supports small/normal mode,
combines to scroll on height overflow)
- Bottom tabs panel (floating style, tabs redesigned)
- Action bar (support for docking/undocking menu)
    - Added login/user menu button to top right
- Restyled breadcrumbs (still collapse when overflows)
- Add litegraph support for fps info position (so it isn't covered by
the sidebar)

- **Breaking**: 
- Removed various elements and added new ones, I have tested custom
sidebars, custom actions, etc but if scripts are inserting elements into
"other" elements they may have been (re)moved.
- Remove support for bottom menu
- Remove support for 2nd-row tabs

## Screenshots 
<img width="1116" height="907" alt="ui"
src="https://github.com/user-attachments/assets/b040a215-67d3-4c88-8c4d-f402a16a34f6"
/>


https://github.com/user-attachments/assets/571dbda5-01ec-47e8-b235-ee1b88c93dd0

┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-5980-Floating-Menus-UI-rework-2866d73d3650810aac60cc1afe979b60)
by [Unito](https://www.unito.io)

---------

Co-authored-by: GitHub Action <action@github.com>
Co-authored-by: github-actions <github-actions@github.com>
2025-10-16 18:12:09 -07:00
Christian Byrne
bb83b0107c fix Vue node border styles in different states (executing, error, selected) (#6018)
- Use exact tokens from Figma
- Fix issue in which node is stuck in `executing` state after it errors

┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-6018-fix-Vue-node-border-styles-in-different-states-executing-error-selected-2896d73d365081f39000fc3e42811f0d)
by [Unito](https://www.unito.io)
2025-10-11 12:20:06 -07:00
Benjamin Lu
bc4549244e test(e2e): align test default menu to Top; make legacy specs explicit (#5746)
## Summary
UseNewMenu has been defaulted to Top in the app for over a year;
Playwright’s test default lagged behind. This PR aligns the test default
with reality and keeps legacy specs stable.

## Changes
- tests(e2e): default to 'Top' via fixture; specs that previously relied
on the old implicit default now explicitly set 'Comfy.UseNewMenu' to
'Disabled'.
- docs(browser-tests): remove outdated README note suggesting tests set
'Top' manually.

## Review Focus
- Intentional uses of 'Top' and 'Bottom' remain unchanged.
- Confirm ComfyPage default remains 'Top' (see
browser_tests/fixtures/ComfyPage.ts).

## Screenshots (if applicable)
N/A

┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-5746-test-e2e-align-test-default-menu-to-Top-make-legacy-specs-explicit-2786d73d365081218d06c1346f3ae18e)
by [Unito](https://www.unito.io)

---------

Co-authored-by: github-actions <github-actions@github.com>
2025-09-24 18:43:22 -07:00