Skip to content

fix(gitGraph): honor showBranches config to hide branch lines and labels (#6535) #6554

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

Open
wants to merge 4 commits into
base: develop
Choose a base branch
from

Conversation

nour0205
Copy link

📑 Summary

When using %%{ init: { "gitGraph": { "showBranches": false } } }%%, branch lines and labels still rendered, even though they should be hidden.

before :
image
after:
image
image

Resolves #6535

📏 Design Decisions

This PR wraps both:

  • drawBranches() call
  • temporary branch label setup in the layout phase

We also ensure setBranchPosition() still runs, so branchPos is correctly calculated even when branches are hidden (important for commit layout).

📋 Tasks

Make sure you

  • 📖 have read the contribution guidelines
  • 💻 have added necessary unit/e2e tests.
  • 📓 have added documentation. Make sure MERMAID_RELEASE_VERSION is used for all new features.
  • 🦋 If your PR makes a change that should be noted in one or more packages' changelogs, generate a changeset by running pnpm changeset and following the prompts. Changesets that add features should be minor and those that fix bugs should be patch. Please prefix changeset messages with feat:, fix:, or chore:.

Copy link

changeset-bot bot commented Apr 30, 2025

⚠️ No Changeset found

Latest commit: 576878d

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@github-actions github-actions bot added the Type: Bug / Error Something isn't working or is incorrect label Apr 30, 2025
Copy link

netlify bot commented Apr 30, 2025

Deploy Preview for mermaid-js ready!

Name Link
🔨 Latest commit 576878d
🔍 Latest deploy log https://app.netlify.com/sites/mermaid-js/deploys/6813db1e4c1211000868044c
😎 Deploy Preview https://deploy-preview-6554--mermaid-js.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

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

Copy link

pkg-pr-new bot commented Apr 30, 2025

Open in StackBlitz

npm i https://pkg.pr.new/mermaid-js/mermaid@6554
npm i https://pkg.pr.new/mermaid-js/mermaid/@mermaid-js/mermaid-zenuml@6554
npm i https://pkg.pr.new/mermaid-js/mermaid/@mermaid-js/layout-elk@6554
npm i https://pkg.pr.new/mermaid-js/mermaid/@mermaid-js/parser@6554

commit: 576878d

Copy link

codecov bot commented Apr 30, 2025

Codecov Report

Attention: Patch coverage is 0% with 26 lines in your changes missing coverage. Please review.

Project coverage is 3.87%. Comparing base (560700e) to head (576878d).
Report is 6 commits behind head on develop.

Files with missing lines Patch % Lines
...kages/mermaid/src/diagrams/git/gitGraphRenderer.ts 0.00% 26 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##           develop   #6554      +/-   ##
==========================================
- Coverage     3.87%   3.87%   -0.01%     
==========================================
  Files          413     412       -1     
  Lines        43216   43220       +4     
  Branches       665     665              
==========================================
  Hits          1675    1675              
- Misses       41541   41545       +4     
Flag Coverage Δ
unit 3.87% <0.00%> (-0.01%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
...kages/mermaid/src/diagrams/git/gitGraphRenderer.ts 0.00% <0.00%> (ø)

... and 1 file with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copy link

argos-ci bot commented Apr 30, 2025

The latest updates on your projects. Learn more about Argos notifications ↗︎

Build Status Details Updated (UTC)
default (Inspect) ✅ No changes detected - May 1, 2025, 9:17 PM

Copy link
Member

@sidharthv96 sidharthv96 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if (DEFAULT_GITGRAPH_CONFIG.showBranches) {
drawBranches(diagram, branches);
}

There is already a similar check happening here, but in both cases, DEFAULT_GITGRAPH_CONFIG is used, which is a misleading name, and won't be updated based on the code in the diagram, as it is only initialised on load.

The proper fix would be to also update the config before rendering, by calling getConfig once inside draw function, after clear.

Is there anything blocking us from moving the position calculations inside the drawBranches function?

@nour0205
Copy link
Author

nour0205 commented May 1, 2025

Hello @sidharthv96
Thank you for your feedback!

I've made the requested updates. The configuration is now dynamically fetched within the draw() function, and the position calculations for branches have been moved to the drawBranches() function for more accurate rendering.

Please let me know if any other adjustments are needed

@@ -812,9 +812,11 @@ const drawArrows = (

const drawBranches = (
svg: d3.Selection<d3.BaseType, unknown, HTMLElement, any>,
branches: { name: string }[]
branches: { name: string }[],
DEFAULT_GITGRAPH_CONFIG: any
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please use variable names matching our convention.
There will be lots of changed lines, but that's okay.

Suggested change
DEFAULT_GITGRAPH_CONFIG: any
config: MermaidConfig['gitGraph']

Comment on lines +904 to 908
const DEFAULT_CONFIG = getConfig();
const DEFAULT_GITGRAPH_CONFIG = DEFAULT_CONFIG?.gitGraph;
if (!DEFAULT_GITGRAPH_CONFIG) {
throw new Error('GitGraph config not found');
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As this is not the default config, using this name is misleading. You can use config or gitGraphConfig.

Comment on lines 918 to +929
branches.forEach((branch, index) => {
const labelElement = drawText(branch.name);
const g = diagram.append('g');
const branchLabel = g.insert('g').attr('class', 'branchLabel');
const label = branchLabel.insert('g').attr('class', 'label branch-label');
label.node()?.appendChild(labelElement);
const bbox = labelElement.getBBox();
let bbox: DOMRect = {
x: 0,
y: 0,
width: 0,
height: 0,
top: 0,
right: 0,
bottom: 0,
left: 0,
toJSON: () => '',
};
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not moved inside drawBranches?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi, I’ve moved the branch label creation inside the drawBranches function as intended. Could you please clarify your suggestion? Thanks in advance

@sidharthv96 sidharthv96 self-assigned this May 1, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Bug / Error Something isn't working or is incorrect
Projects
None yet
Development

Successfully merging this pull request may close these issues.

GitGraph: showBranches: false does not hide branch names and lines
2 participants