🎨 [Frontend] Less noisy Node moving #8152
Conversation
odeimaiz
added
t:enhancement
There was a problem hiding this comment.
Pull Request Overview
This PR reduces the frequency of node position updates sent to the backend during node movement by consolidating position changes only when the node stops moving, rather than continuously during the drag operation. This optimization decreases the number of patches sent to the backend and subsequently broadcasted to other clients.
- Moves shared functionality from BaseNodeUI into NodeUI and removes the BaseNodeUI class entirely
- Changes the "nodeMoving" event to pass coordinate data instead of just triggering an event
- Consolidates node position updates to occur only when movement stops rather than continuously during drag
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| WorkbenchUI.js | Updates event handling to use coordinate data from nodeMoving event and fixes references from BaseNodeUI to NodeUI |
| NodeUI.js | Inherits directly from qx.ui.window.Window, incorporates all BaseNodeUI functionality, and modifies movement logic to consolidate position updates |
| BaseNodeUI.js | Completely removed as its functionality has been moved to NodeUI |
| AvatarGroup.js | Adds missing __avatars array initialization |
Comments suppressed due to low confidence (2)
services/static-webserver/client/source/class/osparc/workbench/NodeUI.js:880
- The method name __setPositionFromEvent is misleading as it doesn't actually set the node's position in the data model, it only updates the DOM position. Consider renaming to __updateDomPositionFromEvent or __calculatePositionFromEvent.
__setPositionFromEvent: function(e) {
services/static-webserver/client/source/class/osparc/workbench/NodeUI.js:652
- Method name changed from _createPort to __createPort but the call here wasn't updated consistently. This should be this.__createPort(isInput, true);
this.__createPort(isInput, true);
services/static-webserver/client/source/class/osparc/workbench/NodeUI.js
Show resolved
Hide resolved
odeimaiz
requested review from
GitHK,
giancarloromeo,
matusdrobuliak66,
pcrespov and
sanderegg
|
wvangeit
left a comment
wvangeit
left a comment
There was a problem hiding this comment.
Less noise is always good. Thanks.
GitHK
left a comment
GitHK
left a comment
There was a problem hiding this comment.
🙏 the backend is thankful for this change
|
@mergify queue |
✅ The pull request has been merged automaticallyThe pull request has been merged automatically at 233031b |
4 participants
What do these changes do?
In order to reduce the number of patches sent to the backend and these broadcasted, this PR makes the Node Moving action less noisy: the node position is consolidated when it stops moving. It also moves shared functionality from BaseNodeUI into NodeUI and removes the BaseNodeUI class entirely.
Before:
After:
Related issue/s
How to test
Dev-ops