Skip to content

Conversation

@christian-byrne
Copy link
Contributor

@christian-byrne christian-byrne commented Jul 31, 2025

Summary

Refactored the selection filtering logic from SelectionOverlay.vue into a reusable composable useSelectedLiteGraphItems. This improves code organization and makes the selection filtering logic available for other components. It also makes the functions easier to test.

Changes

  • Created new useSelectedLiteGraphItems composable that filters out Reroute nodes from selections
  • Refactored SelectionOverlay.vue to use the new composable
  • Added comprehensive unit tests for the composable

Resolves #4623.

┆Issue is synchronized with this Notion page by Unito

…tems composable

- Create new composable to handle filtering of selected LiteGraph items
- Move Reroute filtering logic from SelectionOverlay to the composable
- Add comprehensive unit tests for the new composable
- Refactor SelectionOverlay to use the new composable

This improves code organization by extracting reusable selection logic
that can be shared across components.
@christian-byrne christian-byrne requested review from a team as code owners July 31, 2025 23:16
@github-actions
Copy link

github-actions bot commented Jul 31, 2025

⚠️ Warnings

⚠️ Warning: E2E Test Coverage Missing

If this PR modifies behavior that can be covered by browser-based E2E tests, those tests are required. PRs lacking applicable test coverage may not be reviewed until added. Please add or update browser tests to ensure code quality and prevent regressions.

⚠️ Warning: Visual Documentation Missing

If this PR changes user-facing behavior, visual proof (screen recording or screenshot) is required. PRs without applicable visual documentation may not be reviewed until provided.
You can add it by:

  • GitHub: Drag & drop media directly into the PR description

  • YouTube: Include a link to a short demo

The original implementation used computed properties that accessed the store's
selectedItems array, but the canvas maintains its own selectedItems Set that
needs to be accessed dynamically. Changed to use methods that directly access
the canvas selectedItems each time they're called.

This fixes the selection overlay to properly update when items are selected.
This composable deals with canvas rendering/selection visualization,
not the graph data model, so it belongs in composables/canvas/ instead
of composables/graph/.
@christian-byrne christian-byrne changed the title [refactor] Extract selection filtering logic to useSelectedLiteGraphItems composable Extract selection filtering logic to useSelectedLiteGraphItems composable and don't show toolbox when selecting Reroutes Aug 1, 2025
@christian-byrne christian-byrne merged commit baea47c into main Aug 1, 2025
14 checks passed
@christian-byrne christian-byrne deleted the feat/ignore-reroute-selection branch August 1, 2025 01:02
@christian-byrne christian-byrne added the needs-backport Fix/change that needs to be cherry-picked to the current feature freeze branch label Aug 1, 2025
@christian-byrne christian-byrne mentioned this pull request Aug 3, 2025
christian-byrne added a commit that referenced this pull request Aug 4, 2025
…able and don't show toolbox when selecting Reroutes (#4634)
christian-byrne added a commit that referenced this pull request Aug 4, 2025
…able and don't show toolbox when selecting Reroutes (#4634)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:reroutes needs-backport Fix/change that needs to be cherry-picked to the current feature freeze branch

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug] The selection toolbox in the rerouter node seems unnecessary.

2 participants