Diagram enhancements #578
Open
Diagram enhancements #578
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
pau-hedgehog
force-pushed
the
pau/fix_diagram_labels
branch
2 times, most recently
from
aba7bd4 to
c4f796b
Compare
Frostman
requested changes
May 5, 2025
| var nodes []Node | ||
|
|
||
| func GenerateDrawio(workDir string, topo Topology, styleType StyleType, outputPath string) error { | ||
| slog.Debug("Initializing new diagram generation") |
There was a problem hiding this comment.
Is there any value in that log message?
pau-hedgehog
force-pushed
the
pau/fix_diagram_labels
branch
from
c4f796b to
a7628e0
Compare
pau-hedgehog
force-pushed
the
pau/fix_diagram_labels
branch
2 times, most recently
from
9db6eec to
ed23ae2
Compare
pau-hedgehog
force-pushed
the
pau/fix_diagram_labels
branch
3 times, most recently
from
5a5e832 to
0fc5e00
Compare
pau-hedgehog
force-pushed
the
pau/fix_diagram_labels
branch
3 times, most recently
from
72a648d to
8540b5c
Compare
Frostman
requested changes
Jun 3, 2025
| var nodes []Node | ||
|
|
||
| func GenerateDrawio(workDir string, topo Topology, styleType StyleType, outputPath string) error { | ||
| slog.Debug("Initializing new diagram generation") |
|
|
||
| for _, link := range links { | ||
| // First check for MCLAG links which use "mclagType" property | ||
| if mclagType, ok := link.Properties["mclagType"]; ok { |
There was a problem hiding this comment.
Could you please introduce constants for all those strings? it looks very error-prone for future edits right now. Like "mclagType", "mclag_peer", "mclag_session", etc
|
|
||
| connectionType := getConnectionType(group.Source, group.Target) | ||
|
|
||
| slog.Debug("Processing edge group", |
There was a problem hiding this comment.
I think we shouldn't have any debug messages like that. It seems to be print debug during dev.
| sx, sy := fixedConnectionPoint(sourceCell, tgtCenterX, tgtCenterY) | ||
| tx, ty := fixedConnectionPoint(targetCell, srcCenterX, srcCenterY) | ||
|
|
||
| slog.Debug("Connection centers", |
| "targetCenterY", tgtCenterY) | ||
|
|
||
| // Calculate connection points | ||
| slog.Debug("Calculating optimal connection point for source", "source", group.Source) |
| srcDefaultX := sourceCell.Geometry.X + sx*float64(sourceCell.Geometry.Width) | ||
| srcDefaultY := sourceCell.Geometry.Y + sy*float64(sourceCell.Geometry.Height) | ||
| tgtDefaultX := targetCell.Geometry.X + tx*float64(targetCell.Geometry.Width) | ||
| tgtDefaultY := targetCell.Geometry.Y + ty*float64(targetCell.Geometry.Height) | ||
|
|
||
| slog.Debug("Base connection line", |
| vx := tgtDefaultX - srcDefaultX | ||
| vy := tgtDefaultY - srcDefaultY | ||
| baseLength := math.Sqrt(vx*vx + vy*vy) | ||
| if baseLength == 0 { | ||
| baseLength = 1 | ||
| slog.Warn("Zero length edge, using default length of 1") |
There was a problem hiding this comment.
Would it mean anything to the user? If we can handle it there should be no warn log
| px := -uy | ||
| py := ux | ||
|
|
||
| slog.Debug("Edge vectors", |
| var verticalOffset float64 | ||
| if isLeafToLeaf { | ||
| verticalOffset = 12.0 | ||
| slog.Debug("Leaf-to-leaf connection detected", "verticalOffset", verticalOffset) |
| // Check for spine-to-distant-leaf connection that needs special handling | ||
| isSpineToDistantLeaf, spineLeafOffset := calculateSpineToLeafOffset(group.Source, group.Target, cellMap) | ||
| if isSpineToDistantLeaf { | ||
| slog.Debug("Spine-to-distant-leaf connection detected", "verticalOffset", spineLeafOffset) |
changes labels to text boxes instead of inline fixes vertical edge label overlap and better style Signed-off-by: Pau Capdevila <[email protected]>
- Improve edge routing with better connection point distribution - Enhance label positioning for better readability - Fix overlapping connections with offsets - Generate legend based on connections present Signed-off-by: Pau Capdevila <[email protected]>
groups parallel spine connections for clarity Signed-off-by: Pau Capdevila <[email protected]>
add legend to mermaid diagram Signed-off-by: Pau Capdevila <[email protected]>
adds output name option adds just targets for different vlab diagrams adds just target for lab-ci env Signed-off-by: Pau Capdevila <[email protected]>
Signed-off-by: Pau Capdevila <[email protected]>
Fixes #640 Signed-off-by: Pau Capdevila <[email protected]>
pau-hedgehog
force-pushed
the
pau/fix_diagram_labels
branch
from
2e43f89 to
867691d
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
fixes some cosmetic bugs (overlapping labels) and improves diagram rendering for more complex topologies, such as 3 spine ones