-
Notifications
You must be signed in to change notification settings - Fork 27
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Added drag and drop to Failed Message Flow Diagram nodes #1686
Conversation
…children belonging to incorrect duplicated parent
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 👍
Added a few comments, mostly just questions, not knowing how things work
src/ServicePulse.Host/vue/src/components/failedmessages/FlowDiagram.vue
Outdated
Show resolved
Hide resolved
src/ServicePulse.Host/vue/src/components/failedmessages/FlowDiagram.vue
Outdated
Show resolved
Hide resolved
} | ||
elements.value = [...constructNodes(mappedMessages), ...constructEdges(mappedMessages)]; | ||
//TODO: if doing fitView on next Vue onUpdated() then it doesn't appear the elements | ||
// are actually drawn yet. See if we can determine a better event to use this on rather than relying on setTimeout |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Still working on this one?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I couldn't work out a nice solution (the documentation for Vue-flow isn't the greatest). This was the best I could come up with, but left the TODO for future reference
src/ServicePulse.Host/vue/src/components/failedmessages/FlowDiagram.vue
Outdated
Show resolved
Hide resolved
src/ServicePulse.Host/vue/src/components/failedmessages/FlowDiagram.vue
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Early review. I need to go back over the layout stuff but so far it looks much nicer (and easier to extend)
src/ServicePulse.Host/vue/src/components/failedmessages/FlowDiagram.vue
Outdated
Show resolved
Hide resolved
src/ServicePulse.Host/vue/src/components/failedmessages/FlowDiagram.vue
Outdated
Show resolved
Hide resolved
…onflict # Conflicts: # src/ServicePulse.Host/vue/package-lock.json # src/ServicePulse.Host/vue/src/components/failedmessages/FlowDiagram.vue
Migrated flow diagram from AngularJS directly used d3 for DOM drawing of SVG elements.
This PR changes to use a Vue-specific flow diagram library, allowing for using templates rather than HTML string concatenation.
This also fixes an issue where the diagram wouldn't update the UI reactively as the node elements changed.
This PR also enables the use of drag and drop on nodes for user-managed layout.