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

subgraph direction not applying #2509

Open
MathieuJC opened this issue Nov 24, 2021 · 36 comments · Fixed by #4740
Open

subgraph direction not applying #2509

MathieuJC opened this issue Nov 24, 2021 · 36 comments · Fixed by #4740
Assignees
Labels
Contributor needed Graph: Flow Status: Approved Is ready to be worked on Type: Bug / Error Something isn't working or is incorrect

Comments

@MathieuJC
Copy link

Describe the bug
The subgraph direction features doesn't work anymore

To Reproduce
Steps to reproduce the behavior:

  1. Create a graph
  2. Add subgraphs with different directions

Expected behavior
subgraph should have different directions then the main graph

Screenshots
image

Code Sample
live editor.

Desktop (please complete the following information):

  • OS: Windows 10
  • Browser: Chrome
  • Version: 96.0.4664.45

Additional context
Feature merge with following PR: #2271

@MathieuJC MathieuJC added Status: Triage Needs to be verified, categorized, etc Type: Bug / Error Something isn't working or is incorrect labels Nov 24, 2021
@MathieuJC
Copy link
Author

MathieuJC commented Nov 24, 2021

After further investigation, I realize that subgraph directions are not working correctly when the subgraphs interacts with each other.

removing the arrow from C to D "solves" the problem

@knsv
Copy link
Collaborator

knsv commented Dec 7, 2021

You are right, the direction only works when the subgraph is "isolated".

@Albert-King
Copy link

Albert-King commented Dec 21, 2021

I'm willing to send more codes to show this bug:

flowchart TB

subgraph master
direction TB
etcd<-->kube-APIServer<-->kube-controller-manager
kube-APIServer<-->kube-scheduler
end

subgraph nodeCluser
	subgraph node1
	kubelet
	kube-proxy
	docker
	end
	style node1 fill:red
	
	subgraph node2
	end
	
	subgraph node3
	end
end
Loading

in this case, two subgraphs are side by side, while i want to place master at the top of nodeCluser, but the code 'flowchart TB' is not work.
And, in subgraph node1, three boxes occlude the text 'node1'

windows10
mermaid in typora v8.11.0

@nikmd23
Copy link

nikmd23 commented Feb 8, 2022

I ran into this bug today as well.

I'd love to have it be fixed - of course. 😁

Thanks for the wonderful project. I'm very excited to see it being integrated into GitHub.

@iamkarlson
Copy link

This is really annoying bug for big diagrams, please prioritize.

@sjoerd-vogel
Copy link

I also confirm this and find it rather annoying.

@olmigs
Copy link

olmigs commented May 2, 2022

@MathieuJC Have you tried removing the line C --> D{Something ?} and replacing it with a line after your subgraph definitions like Flow --> API?

live preview

@jdrusso
Copy link

jdrusso commented May 18, 2022

@MathieuJC Have you tried removing the line C --> D{Something ?} and replacing it with a line after your subgraph definitions like Flow --> API?

live preview

I can confirm this works for me, but it would still be great to be able to preserve relations between elements in different subgraphs.

@MathieuJC
Copy link
Author

@olmigs

Changing from targeting a node to targeting a subgraph doesn't fix the issue as the intent is different.

I would use "graph" targeting to represent something like "System A uses System B"
The intent here is to represent "This element of System A is going to use this element of System B"

here's another live preview that might show the issue in a more concrete way (mermaid v. 9.1.1)

Result:
image

Expected
Screen Shot 2022-05-18 at 12 21 16 PM

@olmigs
Copy link

olmigs commented May 18, 2022

So like this preview ? Looks like either option works.

@MathieuJC
Copy link
Author

@olmigs

You're right, putting the "links" inside the subgraph fixes it for that case.

But reusing the exemple I gave when I reported the issue 6 months ago, you can see that the "Flow" chart is declared as being Left to Right, but displayed as Top to Bottom, and the "API" graph is declared as Bottom to Top, but displayed as Top to Bottom

I think I expected a result like this:
Screen Shot 2022-05-18 at 3 44 55 PM

In other words, mermaid disregards the defined direction to display things it thinks is the best.

slightly modified preview used for the image above

@olmigs
Copy link

olmigs commented May 18, 2022

The key here is the declaration of the diagram type. See the first line of this preview
I believe we need to declare like flowchart LR or else mermaid defaults to TD/TB

@jdrusso
Copy link

jdrusso commented May 18, 2022

That still doesn't give the expected behavior described by @MathieuJC . The API subgraph in your preview is display LR, despite being specified as BT.

@nikhiltitus
Copy link

nikhiltitus commented Jun 7, 2022

I have a use case similar to the one @MathieuJC has. Can we have this fixed. Can the direction in subgraph be maintained even when we connect nodes between two subgraphs.

@taranlu-houzz
Copy link

I am also running into this issue.

@ferrieux
Copy link

As this group of issues is still unassigned and keeps eliciting zero reaction from the devs, despite a direct link to user adoption (or is the phrase "show stopper" ambiguous ?), my gut reaction is to run away from this abandonware.

My primary use case being within Gitlab's Markdown, I'm starting to consider an alternative supported there, kroki.
But I would like to avoid doing the same mistake twice, investing much time in critical graphs on yet another dying platform.

Have you, ladies and gentlemen, got some insight on kroki, either in terms of usability or prognosis ?

@taranlu-houzz
Copy link

@ferrieux Yeah, I am also quite disappointed/frustrated by these issues as well. That being said, this project is pretty wide in scope, and mermaid supports a bunch of different kinds of graphs, so I can see how they are probably only able to focus effort in a particular area at any given time.

I do agree that it would be nice if it could be acknowledged that this is a very serious issue though. Maybe I am missing something, but it seems like the flowchart graph type would be pretty widely used, and as it is right now, it is not usable for anything other than very simple graphs in my opinion.

@kc-sn
Copy link

kc-sn commented Jun 22, 2022

I do agree that it would be nice if it could be acknowledged that this is a very serious issue though. Maybe I am missing something, but it seems like the flowchart graph type would be pretty widely used, and as it is right now, it is not usable for anything other than very simple graphs in my opinion.

I spent close to a day building a fairly large graph with dozens of entities and multiple subgraphs and elements within linking each other with the assumption that I could come back later and declare how subgraph elements should be ordered/rendered.

I was wrong.

My assumption is that this isn't an easy fix, but I'm guessing if someone took the time PR a solution which doesn't break anything they'd likely accept it. I'm assuming like everyone else in this thread, while it's nice if this would get fixed, I don't have the time/bandwidth to dive in and make an attempt. Meanwhile, I'll just 👍 this issue and hope some wandering soul will hear our plight.

@lems3
Copy link

lems3 commented Jul 21, 2022

I'm adding my voice here to this issue. Is there a way to add a link to this issue in the documentation with a note about the current limitation ? For us it's not a big deal, but I did lost some time over this. And it could bring some attention to this issue so hopefully someone with time can try to fix it.

@ajmeese7 ajmeese7 moved this to 🆕 New in @ajmeese7's backlog Jul 30, 2022
@knsv knsv removed the Status: Triage Needs to be verified, categorized, etc label Aug 20, 2022
@knsv knsv self-assigned this Aug 20, 2022
@knsv
Copy link
Collaborator

knsv commented Aug 20, 2022

We should fix this.

@tkrenn-hc
Copy link

tkrenn-hc commented Sep 3, 2022

I'll speak up too. Would love to see this fixed. Great tool but useless for me while this functionality is broken.

And thanks @knsv for taking it on!

@vorishirne
Copy link

So much an issue, that large diagrams just scatter like a broken piece of glass. Please fix this amazing product

@tirerocket
Copy link

Definitely a blocker for my flowcharts, would really like to see the fine-grained control over subgraph direction. Rooting for you @knsv !

@iamkarlson
Copy link

Would it be possible if any of the contributors pick this up? @knsv or @sidharthv96 I rarely call people out in opensource issues because I do realize tremendous amount of work you are doing volunteering in such projects. However, I believe that this is core functionality of this project that is a major blocker for many use cases. could you perhaps anyhow prioritize it please?

jason-curtis added a commit to jason-curtis/mermaid that referenced this issue Aug 16, 2023
jason-curtis added a commit to jason-curtis/mermaid that referenced this issue Aug 16, 2023
jason-curtis added a commit to jason-curtis/mermaid that referenced this issue Aug 16, 2023
jason-curtis added a commit to jason-curtis/mermaid that referenced this issue Aug 16, 2023
jason-curtis added a commit to jason-curtis/mermaid that referenced this issue Aug 16, 2023
jason-curtis added a commit to jason-curtis/mermaid that referenced this issue Aug 16, 2023
github-merge-queue bot pushed a commit that referenced this issue Aug 17, 2023
sidharthv96 added a commit that referenced this issue Aug 17, 2023
* develop:
  chore: Move SVG import to comment.
  build docs
  Remove whitespace on empty line
  Documentation for #2509
sidharthv96 added a commit that referenced this issue Aug 17, 2023
* next:
  chore: Move SVG import to comment.
  build docs
  Remove whitespace on empty line
  chore: Fix minify
  Documentation for #2509
  Update all minor dependencies
  Update all patch dependencies
  make more `RectData` required and remove optional assignment
  use lineBreakRegex in `svgDrawCommon`
  fix svgDrawCommon import by adding `.js`
  add types to `svgDrawCommon.ts`
  convert `svgDrawCommon` to TS
Yokozuna59 pushed a commit to Yokozuna59/mermaid that referenced this issue Aug 19, 2023
@tirerocket
Copy link

Can we reopen and classify this as a feature request please? Based on the activity in this thread, there's a lot of people who are looking for this functionality @sidharthv96 @knsv @jason-curtis

@sidharthv96 sidharthv96 reopened this Aug 21, 2023
@sidharthv96
Copy link
Member

As @knsv stated here, this issue seems to be a limitation of dagre. But, it is also not working in flowchart-elk, so, we think that should be fixed.

PRs are welcome!

@AndreiPashkin
Copy link

ELK engine does not support that: kieler/elkjs#26

@maikol-solis
Copy link

Hi!, I'm having the same issue. I tried using mermaid instead of graphviz for a particular diagram with subgraphs, but it's impossible due to this limitation. I will keep using graphviz until this is fixed.

Best.

@rui-costa
Copy link

Forcing direction doesn't enforce direction

Code

---
title: Test
---
flowchart LR
    subgraph API Layer 
    direction LR
    i1([Item 1])
    i2([Item 2])
        subgraph Services
            direction LR
            i3[Item 3]
            i4[Item 4]
            i5[Item 5]
            i6[Item 6]
            i7[Item 7]
        end
    end

Generated Image

---
title: Test
---
flowchart LR
    subgraph API Layer
    direction LR
    i1([Item 1])
    i2([Item 2])
        subgraph Services
            direction LR
            i3[Item 3]
            i4[Item 4]
            i5[Item 5]
            i6[Item 6]
            i7[Item 7]
        end
    end
Loading

@WilliamBonvini
Copy link

Hi everyone, I think this issue deserves attention as well. This functionality is key to many of us.
I discovered Mermaid only recently. I think it is a fantastic project, but this bug is definitely clipping my wings!

@stevedep
Copy link

stevedep commented Jun 3, 2024

Hi,
I looked into this one, specifically when using the ELKjs layout rendering engine. And I think this is indeed not possible. A less elegant workaround would be to draw lines (that do not fit within the hierarchy) manually (during post-processing after ELK returns the layout). I think that this is what is done within the ELKjs demonstrator. I implemented it the same way here.

ELK demonstrates how different directions are possible within subgraphs, but only when the children are separated from the hierarchy. It also states that hierarchical edges (edges that cross the boundary of a hierarchical node) are not possible.

image

Wrt to post-processing the results from the ELK layout engine, I took the following approach:

I modified the example a bit to see if it can indeed handle more complex scenarios.

From there I created an ELKjs example. Just as Mermaid uses. Finally, I created a page that loads a JSON structure and displays the Flowchart. The nodes that cannot be connected by ELK are manually connected afterwards via post-processing. For this to work, all the relative coordinates needed to be converted to absolute coordinates first. Webpage, here. Sourcecode here.

Whenever a link/edge can be included in the hierarchy, it will be provided with coordinates inside the element 'sections', as can be seen here:
image

Whenever it is not part of the hierarchy, the 'sections' element will be missing, as can be seen here:
image

Therefore, the results look like this when a line is drawn manually (meaning simply connecting nodes directly).

Result on the webpage:
image

After some more post-processing on the lines that need to be drawn manually, it's possible to prevent them from being straight lines. I suppose that this is the best that one can do to make this scenario work. See the webpage here.

image

Some thoughts for the post processing solution: Perhaps a Pathfinder lib can help find a path from the source to the target without touching level x subgraphs?

@stevedep
Copy link

stevedep commented Jun 9, 2024

I have made some changes to the code to ensure that ELK displays the same behaviour as the Dagre layout engine. In this video, I demonstrate the changes that I made.

I will walk through the changes.

First, I uncommented this line, to ensure that the direction is added to the dataset that creates a subgraph. When constructing the contents of the subgraph, I added a switch to set the layout options in the subgraph.
In order for the direction to work, the subgraph should contain isolated nodes. When so, the layout option 'SEPARATE_CHILDREN' can be added so the layout will be adjusted.

For this to work, I added some post-processing to the graph that has been constructed. This is because the edges are a collection on root level. So therefore, we check for subgraphs with children that don't have any children, whether they consist of any nodes where the parant is different from the subgraph id. If not, the layout option 'SEPARATE_CHILDREN' can be added.

I explain the above in this video.

Any donation to support this development and bring this to the level of a PR is much appreciated! Via Open Collective is possible as well.

image

@magmanu
Copy link

magmanu commented Jul 9, 2024

Not a workaround for everyone, but works for this use case: instead of connecting node-to-node, you can connect node-to-subgraph.

flowchart LR
subgraph Stage1
direction LR
    A --> A1
    A1 --> A2
    A2 --> A3
    A --> A4
end

subgraph Stage2
direction TB
    B1 --> B2
    B2 --> B3
end

%%here is the sauce
A2 --> Stage2 
Stage1 ~~~ Stage2
flowchart LR
subgraph Stage1
direction LR
    A --> A1
    A1 --> A2
    A2 --> A3
    A --> A4
end

subgraph Stage2
direction TB
    B1 --> B2
    B2 --> B3
end

A2 --> Stage2
Stage1 ~~~ Stage2
Loading

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Contributor needed Graph: Flow Status: Approved Is ready to be worked on Type: Bug / Error Something isn't working or is incorrect
Projects
None yet
Development

Successfully merging a pull request may close this issue.