make Vue nodes resizable #5936
make Vue nodes resizable #5936
Conversation
dosubot
bot
added
the
size:L
🎭 Playwright Test Results❌ Some tests failed ⏰ Completed at: 10/07/2025, 10:18:22 PM UTC 📈 Summary
📊 Test Reports by Browser
🎉 Click on the links above to view detailed test results for each browser configuration. |
🎨 Storybook Build Status✅ Build completed successfully! ⏰ Completed at: 10/07/2025, 10:03:23 PM UTC 🔗 Links🎉 Your Storybook is ready for review! |
| if (!transformState) { | ||
| throw new Error( | ||
| 'TransformState must be provided for node resize functionality' | ||
| ) | ||
| } |
There was a problem hiding this comment.
This might be worth keeping as a console.error to start, then letting useNodeResize degrade gracefully if it's not provided.
There was a problem hiding this comment.
Why? This should always exist.
There was a problem hiding this comment.
There was a conflict between your console.error suggestion and @christian-byrne's view that TransformState should always exist. Since this is an internal component with guaranteed injection, I kept the current error throwing approach for fail-fast debugging.
| const stopMoveListen = useEventListener('pointermove', handlePointerMove) | ||
| const stopUpListen = useEventListener('pointerup', handlePointerUp) |
There was a problem hiding this comment.
I think adding the listeners within function calls might be part of how we're leaking so many.
The handlers are already ignoring events if we're not resizing, so would it be safe to just pull them out and invert some of the start/stop logic?
There was a problem hiding this comment.
adding the listeners within function calls might be part of how we're leaking so many
What does this mean exactly?
The handlers are already ignoring events if we're not resizing, so would it be safe to just pull them out and invert some of the start/stop logic
So have the listeners active across the node lifecycle instead of just during resize? Isn't that worse?
There was a problem hiding this comment.
Analyzed this approach but the current implementation is actually optimal - listeners only exist during resize operations (seconds), avoiding unnecessary event processing. useEventListener handles cleanup properly, so no leaks occur.
christian-byrne
force-pushed
the
vue-node/node-resize-v2
branch
from
773f58e to
9445726
Compare
- Add TransformState injection mock for tests - Mock useNodeResize composable since tests don't need resize functionality - Fixes test failures after making TransformState required for resize - Rename .spec.ts to .test.ts to follow tests-ui conventions 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
…ew feedback Co-authored-by: DrJKL <[email protected]>
…dback Co-authored-by: DrJKL <[email protected]>
…eview feedback Co-authored-by: DrJKL <[email protected]>
christian-byrne
force-pushed
the
vue-node/node-resize-v2
branch
from
f6504f0 to
a1f272b
Compare
|
I'd like to merge this because I have two other features done that rely on it:
|
christian-byrne
added
the
New Browser Test Expectations
Summary
Implemented node resizing functionality for Vue nodes.
2025-10-06.11-03-35_processed_20251006_110410.mp4
Resolves #5675.
Review Focus
ResizeObserver as single source of truth pattern eliminates feedback loops between manual resize and reactive layout updates. Intrinsic content sizing calculation temporarily resets DOM styles to measure natural content dimensions.
graph TD A[User Drags Handle] --> B[Direct DOM Style Update] B --> C[ResizeObserver Detects Change] C --> D[Layout Store Update] D --> E[Slot Position Sync] style A fill:#f9f9f9,stroke:#333,color:#000 style E fill:#f9f9f9,stroke:#333,color:#000┆Issue is synchronized with this Notion page by Unito