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

feat(git-graph): GitGraph Bottom-to-Top Orientation #5259

Conversation

JingyuMarcelLee
Copy link
Contributor

@JingyuMarcelLee JingyuMarcelLee commented Feb 3, 2024

feat(git-graph): GitGraph BT feature:

📑 Summary

Added Bottom-to-Top orientation for the gitGraph. Added BT to gitGraph.jison under "DIR" token and tested this at gitGraphParser.spec.js. Made changes to the gitGraphRenderer.js to correctly render the bottom-to-top orientation of the gitGraph.
Resolves #5239

📏 Design Decisions

  1. For gitGraphRenderer.js, I reversed the array that stored the commit nodes so they can be rendered from bottom to top.
  2. Fixed the arrow and lines in the renderer so that they can be correctly generated.
  3. Added a gitGraph BT: example under /demos/git.html
  4. Added a new test case at gitGraphParser.spec.js to check if BT option can be correctly parsed.

📋 Tasks

Make sure you

Copy link

netlify bot commented Feb 3, 2024

Deploy Preview for mermaid-js ready!

Name Link
🔨 Latest commit a2f54f7
🔍 Latest deploy log https://app.netlify.com/sites/mermaid-js/deploys/65fec6b8250ce60008d388ce
😎 Deploy Preview https://deploy-preview-5259--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.

@github-actions github-actions bot added the Type: Enhancement New feature or request label Feb 3, 2024
Copy link

codecov bot commented Feb 3, 2024

Codecov Report

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

Project coverage is 5.72%. Comparing base (12bf139) to head (a2f54f7).

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##           develop   #5259      +/-   ##
==========================================
- Coverage     5.74%   5.72%   -0.02%     
==========================================
  Files          277     277              
  Lines        41751   41896     +145     
  Branches        27      27              
==========================================
  Hits          2400    2400              
- Misses       39350   39495     +145     
  Partials         1       1              
Flag Coverage Δ
unit 5.72% <0.00%> (-0.02%) ⬇️

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

Files Coverage Δ
...kages/mermaid/src/diagrams/git/gitGraphRenderer.js 0.00% <0.00%> (ø)

@JingyuMarcelLee
Copy link
Contributor Author

If e2e tests are needed, I will work on them. I will also work on the documentation.

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.

Looks great, works well! Nice work 🚀
Only some minor comments.

We also need

  • The documentation in mermaid/packages/mermaid/src/docs/syntax/gitgraph.md
  • Rendering tests in cypress/integration/rendering/gitGraph.spec.js

packages/mermaid/src/diagrams/git/gitGraphRenderer.js Outdated Show resolved Hide resolved
packages/mermaid/src/diagrams/git/gitGraphRenderer.js Outdated Show resolved Hide resolved
@JingyuMarcelLee
Copy link
Contributor Author

Updated Documentation. Currently working on the Cypress E2E. I found that the GitGraph: { parallelCommits: true } option in the config breaks the BT orientation. I found a way around it and will work on it

@JingyuMarcelLee
Copy link
Contributor Author

Finished adding Cypress E2E test cases and logic for handling GitGraph: { parallelCommits: true } option in the config.

cypress/integration/rendering/gitGraph.spec.js Outdated Show resolved Hide resolved
packages/mermaid/src/docs/syntax/gitgraph.md Outdated Show resolved Hide resolved
@JingyuMarcelLee
Copy link
Contributor Author

I did make the requested changes, but please let me know if there are any other mistakes/errors. Sorry about the bad test/doc conventions. I should've read Contributing.md more carefully.

@sidharthv96
Copy link
Member

Please resolve the conflicts.

@JingyuMarcelLee JingyuMarcelLee force-pushed the feature/5239_git-graph-bottom-to-top-orientation branch from 5f46276 to 10fb856 Compare March 20, 2024 05:19
@JingyuMarcelLee
Copy link
Contributor Author

JingyuMarcelLee commented Mar 20, 2024

@sidharthv96 I am really sorry about this; I badly polluted the commits with some of the merge conflicts and had to hard reset to the up-to-date dev branch and recommit my feature. Some of the changes in the renderer had conflicts with my logic and I had to rewrite them. I added more examples on git.html to verify everything is working with the merge.

@sidharthv96 sidharthv96 merged commit c00bf26 into mermaid-js:develop Mar 23, 2024
19 checks passed
Copy link

mermaid-bot bot commented Mar 23, 2024

@JingyuMarcelLee, Thank you for the contribution!
You are now eligible for a year of Premium account on MermaidChart.
Sign up with your GitHub account to activate.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

bottom-to-top oriented gitgraphs to mimic git logs
2 participants