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

GitGraph: line routing and colouring for branches above main #4932

Open
guypursey opened this issue Oct 9, 2023 · 6 comments
Open

GitGraph: line routing and colouring for branches above main #4932

guypursey opened this issue Oct 9, 2023 · 6 comments
Labels
Status: Triage Needs to be verified, categorized, etc Type: Bug / Error Something isn't working or is incorrect

Comments

@guypursey
Copy link
Contributor

guypursey commented Oct 9, 2023

Description

Branches render unexpectedly differently when they're positioned above main (as compared with their default position underneath main).

This was found by @ijmitch during proposed fix for #4912.

Steps to reproduce

  1. Open Mermaid Live
  2. Replace the code with the following
     gitGraph
       commit
       branch release-1
       commit
    
  3. Observe that arrow between first commit and second has colour of new branch and comes straight out of main before joining release-1 with a curve.
  4. Either
    a. Add following to config option on Config tab: "gitGraph": { "mainBranchOrder": 2}, or
    b Paste following line above graph code: %%{init: {'theme': 'base', 'gitGraph': {'mainBranchOrder': 2}} }%%
  5. Observe that arrow between first commit and second now has colour of main branch instead and curves out of main before joining release-1 straight on.

Screenshots

Here are examples with simplest possible graph for demonstration.

Example, standard with main as top branch:

  • Branch arrow goes straight out and curves into branch commit:
  • Branch arrow takes on colour of new branch
gitGraph
  commit
  branch release-1
  commit

image

Example, standard with main as bottom branch:

  • Branch arrow *curves out and goes *straight into branch commit
  • Branch arrow retains colour of main branch
%%{init: {'theme': 'base', 'gitGraph': {'mainBranchOrder': 2}} }%%
gitGraph
  commit
  branch release-1
  commit

image

Setup

  • Mermaid version: 10.5.0
  • Browser and Version: Firefox 118.0.1

Suggested Solutions

Amend conditionals around path rendering to take branch position into account, in the same way that top-to-bottom is considered separately from default left-to-right rendering.

Additional Context

Assumption: that branch curves should only apply to branches that aren't main.

@guypursey guypursey added Status: Triage Needs to be verified, categorized, etc Type: Bug / Error Something isn't working or is incorrect labels Oct 9, 2023
@ijmitch
Copy link

ijmitch commented Oct 9, 2023

+1 @guypursey

for context, I was trying to write some guidance which is largely aligned to https://learn.microsoft.com/en-us/azure/devops/repos/git/git-branching-guidance?view=azure-devops#use-release-branches and other schemes which advocate release branches and depict them above main (and obviously never merging into it), whereas feature branches are depicted below main and generally do merge into it.

@guypursey
Copy link
Contributor Author

@ijmitch Thanks for the extra context. With this in mind, do you think the point I've made about the curves still makes sense?

Guess it's a design decision about whether curves and straight lines are a visual shorthand for anything semantically. I haven't found any guidance or documentation on this in the Mermaid repo so far.

@ijmitch
Copy link

ijmitch commented Oct 9, 2023

It would be ideal in my opinion if branching 'upwards' was a mirror-image of the normal 'downwards' rendering. So, yes, I mentioned the colour and was a bit lazy not to mention the curves used as well.

@mathbraga
Copy link
Contributor

Hi @guypursey, are you working on this?

@guypursey
Copy link
Contributor Author

Hi @mathbraga, yes, I started work on this, as I was looking at those parts of the codebase already and drafting documentation. But I'm waiting to see if #4927 will be accepted and merged as I was building on top of the work in that PR.

@mathbraga
Copy link
Contributor

mathbraga commented Oct 11, 2023

Yeah I noticed your involvement with #4927, that's why I wanted to know before diving too much into this.

I did dig into this for a bit though, so I'll share some of what I found out that might help.

I believe that one of the ways to solving this would be to identify when a branch is above or below main.

getBranchesAsObjArray in /packages/mermaid/src/diagrams/git/gitGraphAst.js returns only the name of each branch. You can modify this to return also the branch order value.

The second .map inside getBranchesAsObjArray could be changed from:

.map(({ name }) => ({ name }))

to something like

.map(({ name, order }) => ({ name, order }))

Then inside draw in /packages/mermaid/src/diagrams/git/gitGraphRenderer.js you can have access to branches with ordering. Then you can modify drawArrows to also accept branches as input and work from there.

I didn't really give much thought to this so there could be better solutions, just felt like leaving this here as it could help.

@jgreywolf jgreywolf added include roadmap items to add to roadmap for auto workflow and removed include roadmap items to add to roadmap for auto workflow labels Nov 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Status: Triage Needs to be verified, categorized, etc Type: Bug / Error Something isn't working or is incorrect
Projects
None yet
Development

No branches or pull requests

4 participants