Commit Graph

7 Commits

Author SHA1 Message Date
Christian Byrne
cd8b5b5d50 [refactor] reorganize Vue node browser tests into subfolders based on behaviors and states (#5823)
## Summary

Refactored Vue node browser tests by organizing them into behavioral
categories, better reflecting the nature of browser tests as behind
user-action/behavior specifications.

- **What**: Reorganized Playwright browser tests into logical behavioral
folders (`interactions/`, `nodeStates/`)
- **Structure**: Created subdirectories for canvas interactions, link
interactions, node interactions, and node states

## Review Focus

Test file path changes and associated snapshot relocations ensure all
browser tests continue to run correctly in the new structure.

┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-5823-refactor-reorganize-Vue-node-browser-tests-into-subfolder-based-on-behaviors-and-states-27b6d73d365081aaa6f9e3a388165258)
by [Unito](https://www.unito.io)
2025-09-27 15:14:51 -07:00
Christian Byrne
a25d89881b Allow Vue nodes to have their colors changed from selection toolbox (#5720)
## Summary

Fixes https://github.com/Comfy-Org/ComfyUI_frontend/issues/5680 by
allowing Vue nodes to properly synchronize color changes with LiteGraph
nodes, implementing header darkening and light theme adjustments.

<img width="2496" height="1512" alt="Screenshot from 2025-09-21
20-00-36"
src="https://github.com/user-attachments/assets/e3bdf645-1e0b-4d11-9ae5-9401f43e8e96"
/>

## Changes

- **What**: Implemented color property synchronization between LiteGraph
and Vue node rendering systems
- **Core Fix**: Added `nodeData.color` and `nodeData.bgcolor` to [v-memo
dependencies](https://vuejs.org/api/built-in-directives.html#v-memo) to
trigger re-renders on color changes
- **Color Logic**: Added header darkening using [memoized color
adjustments](https://github.com/Comfy-Org/ComfyUI_frontend/blob/main/src/utils/colorUtil.ts)
to match LiteGraph's ColorOption system
- **Event System**: Enhanced property change instrumentation in
LGraphNode.setColorOption to emit color/bgcolor events

## Review Focus

Vue component reactivity timing - the v-memo fix was critical for
immediate color updates. Verify light theme color adjustments match the
drawNode monkey patch behavior in app.ts.

## Technical Details

```mermaid
graph TD
    A[User Sets Color] --> B[LGraphNode.setColorOption]
    B --> C[Sets node.color & node.bgcolor]
    C --> D[Triggers property:changed events]
    D --> E[Vue Node Manager Updates]
    E --> F[v-memo Detects Change]
    F --> G[NodeHeader Re-renders]
    G --> H[Header Darkening Applied]

    style A fill:#f9f9f9,stroke:#333,color:#000
    style H fill:#f9f9f9,stroke:#333,color:#000
```

┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-5720-Allow-Vue-nodes-to-have-their-colors-changed-from-selection-toolbox-2766d73d36508123b441d126a74a54b2)
by [Unito](https://www.unito.io)

---------

Co-authored-by: Myestery <Myestery@users.noreply.github.com>
Co-authored-by: DrJKL <DrJKL@users.noreply.github.com>
Co-authored-by: github-actions <github-actions@github.com>
2025-09-27 15:12:13 -07:00
Christian Byrne
856eb446a5 Add node pinning functionality to Vue nodes (#5772)
## Summary

Added pinning functionality to Vue nodes with hotkey support and visual
indicators.

## Changes

- **What**: Added node pinning feature with 'p' hotkey toggle and pin
icon indicator
- **Components**: Updated `LGraphNode.vue` and `NodeHeader.vue` with pin
state tracking
- **State Management**: Extended `useGraphNodeManager` to sync pinned
flag with Vue components
- **Tests**: Added E2E tests for single and multi-node pin toggling

## Review Focus

Pin state persistence in graph serialization and visual indicator
positioning in node header layout. Verify hotkey doesn't conflict with
existing shortcuts.

## Technical Details

- Pin state tracked via `flags.pinned` property in `LGraphNode`
- Uses [Vue
memoization](https://vuejs.org/api/reactivity-advanced.html#v-memo) for
efficient header re-rendering
- Integrates with existing node property change detection system
- Visual indicator uses Lucide pin icon with theme-aware styling

## Screenshots (if applicable)

<img width="875" height="977" alt="Screenshot from 2025-09-25 13-02-21"
src="https://github.com/user-attachments/assets/51d46cea-08f0-44fb-8b07-56d1b939338f"
/>

<img width="875" height="977" alt="Screenshot from 2025-09-25 13-02-10"
src="https://github.com/user-attachments/assets/ce247426-1e39-48c0-924b-658b65c24f52"
/>

## Related 

- https://github.com/Comfy-Org/ComfyUI_frontend/pull/5715

┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-5772-Add-node-pinning-functionality-to-Vue-nodes-2796d73d36508195914bcfc986aa66b5)
by [Unito](https://www.unito.io)

---------

Co-authored-by: filtered <176114999+webfiltered@users.noreply.github.com>
2025-09-27 10:56:46 -07:00
Christian Byrne
415ebfd67b Add muted state to Vue nodes (#5770)
## Summary

Added mute state support to Vue nodes with visual feedback and keyboard
shortcut functionality.

## Changes

- **What**: Implemented mute state (mode 2) for Vue nodes with opacity
styling and `Ctrl+M` hotkey support

## Review Focus

Visual consistency between bypass and mute states, and keyboard shortcut
conflict detection with existing hotkeys.

## Test Coverage

- Single node mute/unmute with `Ctrl+M` hotkey
- Multi-selection mute/unmute operations
- Visual state verification with opacity changes

## Related

- https://github.com/Comfy-Org/ComfyUI_frontend/pull/5715

┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-5770-Add-muted-state-to-Vue-nodes-2796d73d36508143b3edfbcb782de7c1)
by [Unito](https://www.unito.io)
2025-09-25 13:07:29 -07:00
Christian Byrne
a0c06bd723 [test] add browser test for missing vue nodes error state (#5768)
## Summary

Added browser test to verify Vue nodes display error state when workflow
contains missing/unknown nodes, complementing

- https://github.com/Comfy-Org/ComfyUI_frontend/pull/5758

## Changes

- **What**: Added [Playwright
test](https://playwright.dev/docs/writing-tests) for Vue nodes error
state handling with missing nodes
- **Test Coverage**: Validates `border-error` class application on nodes
with `UNKNOWN NODE` text

## Review Focus

Test reliability when loading workflows with missing nodes and dialog
interaction timing.

┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-5768-test-add-browser-test-for-missing-vue-nodes-error-state-2796d73d365081aea187cdbc7920a643)
by [Unito](https://www.unito.io)
2025-09-25 11:16:19 -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
Christian Byrne
f951e07cea fix bypass hotkey in vue nodes and fix node data instrumentation setup issue when switching to Vue nodes after initial load (#5715)
## Summary

Fixed Vue node keybinding target element ID to enable
bypass/pin/collapse hotkeys in both LiteGraph and Vue rendering modes.

Also fixed a bug when starting in litegraph mode => switching to Vue
nodes without reloading => `graph.onTrigger` is set to `undefined` which
interferes with proper setup of node data instrumentation, among other
things.

## Changes

- **What**: Updated keybinding `targetElementId` from `graph-canvas` to
`graph-canvas-container` for node manipulation commands (parent of both
the canvas and transform pane -- vue nodes container).
- **What**: Added conditional `onTrigger` handler restoration in slot
layout sync to prevent Vue node manager conflicts

## Review Focus

Event handler precedence between Vue nodes and LiteGraph systems during
mode switching, ensuring hotkeys work consistently across rendering
modes.

┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-5715-fix-bypass-hotkey-in-vue-nodes-and-fix-node-data-instrumentation-setup-issue-when-switchi-2756d73d3650815c8ec8d5e4d06232e3)
by [Unito](https://www.unito.io)
2025-09-21 17:32:12 -07:00