[6067] Add support for relative position bending points for edges#6177
[6067] Add support for relative position bending points for edges#6177
Conversation
191fb2e to
6bb90e0
Compare
6bb90e0 to
d20640e
Compare
Bug: #6067 Signed-off-by: Florian ROUËNÉ <florian.rouene@obeosoft.com>
d20640e to
8f6ba6a
Compare
|
Actually, it wasn't the right bug, it was #6085. |
mcharfadi
left a comment
There was a problem hiding this comment.
https://github.com/user-attachments/assets/db273b8f-82fc-4bf8-b51f-38fe6fc4e56b
If I move the node quickly I can change the position of the bending points.
I suspect that this behavior would be easier to reproduce with a diagram with more elements.
| minComputedWidth: gqlNodeLayoutData?.minComputedSize?.width ?? null, | ||
| minComputedHeight: gqlNodeLayoutData?.minComputedSize?.height ?? null, | ||
| isLastNodeSelected: false, | ||
| moving: false, |
There was a problem hiding this comment.
What's the use of this moving property ?
Is it only used in useDropNodes ?
How would this work if we're moving a node an we receive another payload from another user ?
I feel like this property should be local only and not change when converting the diagram.
If the current moving property does not reflect the nodes that are actually moving we might want to better update this property when receiving a NodeChange from ReactFlow.
There was a problem hiding this comment.
Yes it's only used in useDropNodes, I need this information to know if the node move even it's not an actual move, like for child node.
I used it like the other datas needed for useDropNodes: isDraggedNode, isDropNodeTarge, isDragNodeSource
There was a problem hiding this comment.
But should we not set the value to the previous value instead of false.
What would happen if a node has moving true but another user refresh the diagram, that would set moving to false and create an incorrect state, no ?
We might want to do like the selection and retrieve the current one after converting.
As a side note we should find a naming convention or something to differentiate between local attributes of a node and synchronized ones.
There was a problem hiding this comment.
In this case, we only want capture the move if it's made by the current user, it's the same thing that for the other data like isDraggedNode, if a refresh from another user is received during DnD the data is reset to false.
Bug: #6067
Pull request template
General purpose
What is the main goal of this pull request?
Project management
priority:andpr:labels been added to the pull request? (In case of doubt, start with the labelspriority: lowandpr: to review later)area:,difficulty:,type:)CHANGELOG.adocbeen updated to reference the relevant issues?CHANGELOG.adoc? (Including changes in the GraphQL API)CHANGELOG.adoc? For example indoc/screenshots/2022.5.0-my-new-feature.pngArchitectural decision records (ADR)
[doc]?CHANGELOG.adoc?Dependencies
CHANGELOG.adoc?CHANGELOG.adoc?Frontend
This section is not relevant if your contribution does not come with changes to the frontend.
General purpose
Typing
We need to improve the typing of our code, as such, we require every contribution to come with proper TypeScript typing for both changes contributing new files and those modifying existing files.
Please ensure that the following statements are true for each file created or modified (this may require you to improve code outside of your contribution).
useMutation<DATA_TYPE, VARIABLE_TYPE>(…)useQuery<DATA_TYPE, VARIABLE_TYPE>(…)useSubscription<DATA_TYPE, VARIABLE_TYPE>(…)useMachine<CONTEXT_TYPE, EVENTS_TYPE>(…)useState<STATE_TYPE>(…)?.(if the GraphQL API specifies that a field cannot benull, do not treat it has potentiallynullfor example)let diagram: Diagram | null = null;)Backend
This section is not relevant if your contribution does not come with changes to the backend.
General purpose
Architecture
Review
How to test this PR?
Please describe here the various use cases to test this pull request