-
Notifications
You must be signed in to change notification settings - Fork 712
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
Layout algorithm should consider screen dimensions #2564
Comments
It should already do this. Can you give an example/screenshot? |
The height and width of the graph are entirely dictated by the topology. So the only way to adjust to changing window sizes is to zoom. |
@bowenli Actually that's what the behaviour was like for a brief period of time, but then we made a decision that refreshing should never reset the zoom state - see #2407. So both dimensions are always taken into account by initial rendering and ignored by the refresh button. If I remember it correctly, the reasoning was that we don't wanna lose focus from a particular part of the graph if we hit refresh for layout inaccuracies (which is the typical use case). As for the unused space in the bottom-right corner, I think that's probably preferable to stretching the inactive nodes square so that the whole layout would form a perfect rectangle, as that would make inactive nodes more visualy indistinguishable from the rest of the graph. But this is just my opinion and I don't hold it strongly :) |
@fbarl Thanks for the explanation. I see that on initial load it considers the screen size. But it still seems to ignore the dimensions, ie. we will always generate a wide graph within the screen constraints, even if the screen is much taller than wide. Maybe there are some other goals the layout engine is trying to achieve. I'm saying that for a given topology there are choices to be made that affect the shape of the overall diagram. We can choose to make the graph tall or wide, etc: And given that these clusters will form strongly connected/well connected components, we can further force the general shape by abstracting one level up: |
My understanding of the scope of this is that it would take a complete re-implementation of the graphing logic, including replacing the |
The layout attempts to direct edges (it is a directed graph) downwards, to reflect information flow through the system - so you end up with front ends at the top, app/service layers in the middle and DBs / external services at the bottom. Since most applications have a small number of such layers, but a large number of components per layer, the graphs naturally become wide. |
Thanks for the sketches @bowenli. Unfortunately, I have to say @jpellizzari is right that our hands are pretty much tied regarding the main graph layout as long as we keep using https://github.com/cpettitt/dagre. Although we are already using some options to tweak the layout, I think none of them would be sufficient to affect the aspect ratio of the output graph. Because of that lack of control (and also since the library is not being maintained anymore), there is an open issue to replace On top of that, because of the points that @rade brought, I think it would be hard to retain some existing properties of the layout even if we were free to customize its shape. The only part we can fully control is the grid of single nodes in the bottom-left corner, but I gave my arguments above why I think their current layouting is not that bad. :) |
If I force re-layout, it should consider the screen dimensions (ie. tall, wide, etc.) to provide the best possible fit.
The text was updated successfully, but these errors were encountered: