Skip to content
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

Avoid edge overlap #569

Merged
merged 5 commits into from
Nov 24, 2024
Merged

Avoid edge overlap #569

merged 5 commits into from
Nov 24, 2024

Conversation

keyserj
Copy link
Collaborator

@keyserj keyserj commented Nov 24, 2024

Closes #559

Unfortunately I spent a ton of time trying to get edges and their labels to avoid overlapping, in order to reduce the feeling of clutter from edges. I couldn't get through all of the issues, and I didn't think it was worth spending more time on this right now (spent more than I should have), so I've added two separate config options to enable the functionality that might benefit some situations but not all situations:
image

The first config is "Draw simple edge paths". It defaults to true (which is the current-prod behavior), but when set to false, edge paths are drawn based on the output from the layout algorithm, so they do a better job of avoiding each other.

  • The main problem with this (and the reason the config exists) is that the layout output does not result in vertical slopes at the start and end points of each path. These vertical slopes often seem desirable, because they go with the top-down flow of the diagram, and without them, the edges can look a little haphazard. Created Make spline edges end in vertical slopes #568 to address this.

The second config is "Avoid edge label overlap". It defaults to false (which is the current-prod behavior), but when set to true, edge labels are included in the layout algorithm, which results in them trying to avoid overlapping with each other.

  • The main problem with this (and the reason the config exists) is that the ELK layout algorithm treats edge labels as nodes, which results in inconsistent spacing between node layers, as well as more node layers than otherwise. These both can result in a more chaotic layout (though sometimes it's fine). See Edge labels create additional node layers and inconsistent spacing between node layers eclipse/elk#1092 for more info - if that is resolved, we may be able to always include edge labels in the layout, without this config.

Copy link

netlify bot commented Nov 24, 2024

Deploy Preview for velvety-vacherin-4193fb ready!

Name Link
🔨 Latest commit 0f773c4
🔍 Latest deploy log https://app.netlify.com/sites/velvety-vacherin-4193fb/deploys/6743b71a36c34600087e9c72
😎 Deploy Preview https://deploy-preview-569--velvety-vacherin-4193fb.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.
Lighthouse
Lighthouse
1 paths audited
Performance: 70
Accessibility: 86
Best Practices: 92
SEO: 100
PWA: -
View the detailed breakdown and full score reports

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link

netlify bot commented Nov 24, 2024

Deploy Preview for ameliorate-docs canceled.

Name Link
🔨 Latest commit 0f773c4
🔍 Latest deploy log https://app.netlify.com/sites/ameliorate-docs/deploys/6743b71a4b66620008ec7db1

seems to make slightly more sense to go positive, neutral, negative
than neutral, positive, negative. I guess?
so that edges can be drawn via layout calc without having some points
being too high/above the bottom of a node
and sort edges by node type in addition to nodes.

because the `mergeEdges` option (which will be needed for
drawing edges that avoid each other) results in the
PREFER_NODES strategy no longer being respected.

see eclipse/elk#1091
edges overlap less with nodes and other edges when using layout calc.

includes change edge routing to SPLINES because that seems to
avoid overlap better.

wanted to make this default/not behind an option, but unfortunately
the edges drawn via layout calc do not have vertical slopes at their
start and end points. I think this can be ok in some situations, but
others look bad and would much prefer the vertical slopes (which the
simple edges guarantee).

also added a comment TODO for modifying the edge drawing via layout
calc to have vertical slopes - if this were done, we should be able
to remove this config option and just _always_ draw via layout calc.
so that they avoid each other.

wanted to always do this (i.e. without an option), but ELK treats labels
as nodes, creating inconsistent spacing between node layers, and
creating more node layers total.

this sometimes results in more-chaotic layouts than otherwise, so an
option is added for it.

see eclipse/elk#1092 for more information.
@keyserj
Copy link
Collaborator Author

keyserj commented Nov 24, 2024

One example of before:
image

vs after:
image

@keyserj keyserj merged commit e90da88 into main Nov 24, 2024
13 checks passed
@keyserj keyserj deleted the avoid-edge-overlap branch November 24, 2024 23:50
@prototyperspective
Copy link

Great, much better now!

  1. Couldn't one simply add a few pixels or a padding to the labels / label-widths?
  2. I think it would be best to make this option enabled by default.
  3. Nodes can still overlap labels: e.g. here without the option enabled "One can stop incentivizing and facilitating car" in bottom left overlaps a label and "This requires collective investment" as well as "One could develop a system that checks travel times", "Climate change has a lots of current" and "This would save many years " overlaps edges but with the option enabled "Climate change has a lots of current and future negative effects on health" still overlaps the "subproblem of" edge (on my end at least) and "Electric vehicles only pollute during[…]" overlaps "addresses".

@keyserj
Copy link
Collaborator Author

keyserj commented Nov 25, 2024

Couldn't one simply add a few pixels or a padding to the labels / label-widths?

Yeah I'll probably just do that. I originally was thinking that an average of all the label widths made more sense, to balance between using too much space vs using too little space, but I think that's not the case. It doesn't seem like a big deal to use too much space, whereas labels still overlapping basically forfeits a lot of the value of trying to avoid overlap in the first place!

I think it would be best to make this option enabled by default

I'll keep an eye on how it looks as I continue using more diagrams, but my impression is currently that the creation of inconsistent spacing between node layers, as well as more node layers than before, forfeits a lot of the value that the node layers create in the first place: of being able to quickly look at an organized-feeling diagram and know where you need to look to find section of node types you want to see. Here's the cars-going-too-fast example that hopefully illustrates what I mean:

image

image

Nodes can still overlap labels

I think in your example you need to turn off the simple edge config I added in the More Actions Drawer:
image

It is pretty annoying to have these as separate configs but I explain my reasoning for it in the PR description.

With that config set, edge labels don't overlap, as far as I can see:
image

@prototyperspective
Copy link

Makes sense, thanks for explaining with the screenshots that makes it clearer. I think ultimately it probably needs to improved so that setting can be a default, e.g. configuring things so that parallel nodes are always next to each other not like in the 2nd screenshot where Detriment "pedestrians might get hit" is not parallel to its other nodes. For the sharp angles, I think it would be best anyway if the lines connected to different locations of the node so that one can display the edge rating directly above/underneath the node.

@keyserj
Copy link
Collaborator Author

keyserj commented Nov 25, 2024

ultimately it probably needs to improved so that setting can be a default

Yeah I agree, it's not ideal when it has to be manually configured to get the benefits

For the sharp angles, I think it would be best anyway if the lines connected to different locations of the node so that one can display the edge rating directly above/underneath the node.

That's a good point. I hadn't considered that separate connections (node "ports" is the term the layout library uses) would be desirable - if we want to do something like that edge-ratings-closer-to-nodes idea, we'd probably want the ports to be separate. One consideration though is that if there are 8+ ports on one side of a node, that would probably get super hectic (though such a case might want to be avoided/refactored by the topic author anyway).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Investigate options for reducing clutter of edges
2 participants