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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Gantt: add top xaxis #1954

Merged
merged 6 commits into from
Apr 25, 2021
Merged

Gantt: add top xaxis #1954

merged 6 commits into from
Apr 25, 2021

Conversation

nacc
Copy link
Contributor

@nacc nacc commented Mar 23, 2021

馃搼 Summary

Add a top x-axis to Gantt charts that mirrors the bottom x-axis

馃搹 Design Decisions

Straightforward copy and paste of existing code.

馃搵 Tasks

Make sure you

  • 馃摉 have read the contribution guidelines
  • 馃捇 have added unit/e2e tests (if appropriate)
  • 馃敄 targeted develop branch

Copy link
Collaborator

@knsv knsv left a comment

Choose a reason for hiding this comment

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

Thanks @nacc, this look great!

There are one thing that would make this even better. The feature need an on on/off switch similar to mirrorActors in sequence diagrams. Two things that we need to do:

  1. Add this in the configuration options so that it possible to turn is on/off on a site level when you add Mermaid
  2. Add a topAxis directive which will make it possible to turn on topAxis even if the site confiuration as topAxis as off

Is the on/off switch something you could help with?

Thanks again
/Knut

@nacc
Copy link
Contributor Author

nacc commented Mar 24, 2021

@knsv yep I can do that. I'll update the PR today!

@nacc
Copy link
Contributor Author

nacc commented Mar 24, 2021

I think my branch has the changes requested correctly. I'm not a JS person at all, so please point me in a corrected direction if I did something wrong :)

@knsv
Copy link
Collaborator

knsv commented Apr 1, 2021

Thanks will review this PR today.

@knsv
Copy link
Collaborator

knsv commented Apr 1, 2021

Hello! I am reviewing this now. I tested it in my local development environment and the top axis is not coming at the top for me:
image

Did something go wrong?

This is the html file I used:

<html>
  <head>
    <link
      href="https://fonts.googleapis.com/css?family=Montserrat&display=swap"
      rel="stylesheet"
    />
    <link href="https://unpkg.com/tailwindcss@^1.0/dist/tailwind.min.css" rel="stylesheet">
    <link rel="stylesheet" href="https://cdnjs.cloudflare.com/ajax/libs/font-awesome/4.7.0/css/font-awesome.min.css">
    <link href="https://fonts.googleapis.com/css?family=Noto+Sans+SC&display=swap" rel="stylesheet">
    <style>
      body {
        /* background: rgb(221, 208, 208); */
        /* background:#333; */
        font-family: 'Arial';
        }
      h1 { color: grey;}
      .mermaid2 {
        display: none;
      }
      .mermaid svg {
        font-size: 12px !important;
      }
    </style>
  </head>
  <body>
    <h1>info below</h1>
    <div class="flex">

      <div class="mermaid" style="width: 100%; height: 20%;">
gantt
      dateFormat  YYYY-MM-DD
      axisFormat  %d/%m
      title Adding GANTT diagram to mermaid
      excludes weekdays 2014-01-10

      section A section
      Completed task            :done,    des1, 2014-01-06,2014-01-08
      Active task               :active,  des2, 2014-01-09, 3d

      </div>
        <script src="./mermaid.js"></script>
    <script>
      mermaid.parseError = function (err, hash) {
        };
      mermaid.initialize({
        logLevel: 0,
        flowchart: { nodeSpacing: 10, curve: 'cardinal', htmlLabels: false },
        htmlLabels: false,
        sequence: { actorFontFamily: 'courier',actorMargin: 50, showSequenceNumbers: false },
        fontFamily: '"arial", sans-serif',
        fontFamily: 'courier',
        curve: 'cardinal',
        securityLevel: 'loose',
        gantt:{topAxis: true}
      });
      function callback(){alert('It worked');}
    </script>
  </body>
</html>

@nacc
Copy link
Contributor Author

nacc commented Apr 1, 2021

Urgh, I see the problem, it was my misunderstanding of what hte translate method did. Fix incoming!

@nacc
Copy link
Contributor Author

nacc commented Apr 1, 2021

Can you try with the fixlet I just pushed?

@nacc
Copy link
Contributor Author

nacc commented Apr 1, 2021

@knsv the failure seems unrelated to my changes, from something in a different diagram type?

nacc added 6 commits April 1, 2021 15:45
I am leveraging a Gantt chart to generate a years-long Roadmap from text.
This is for a large team, so it is quite large. A request I get from my
stakeholders is for a top labelling axis in parallel to the current bottom
one, so that the dates are labelled no matter where we are scrolled on
the screen.
@nacc
Copy link
Contributor Author

nacc commented Apr 1, 2021

(trying to rebase now)

@knsv
Copy link
Collaborator

knsv commented Apr 11, 2021

will give it a spin!

@knsv
Copy link
Collaborator

knsv commented Apr 22, 2021

I tried this again and it is getting closer. The axis is at the top of the diagram.
image

The problem is caused by
image

Any reason why you have that there or should I just remove it?

@nacc
Copy link
Contributor Author

nacc commented Apr 22, 2021

I tried this again and it is getting closer. The axis is at the top of the diagram.
image

The problem is caused by
image

Any reason why you have that there or should I just remove it?

Ah ha! Probably c&p failure on my part. Definitely can be removed or possibly will need adjusting to -1em? I'm treating the top axis like a mirror of the bottom, ideally.

@knsv
Copy link
Collaborator

knsv commented Apr 24, 2021

Ok, then I think this one is ready. I will remove that line and merge it! Thanks

@knsv knsv merged commit adab981 into mermaid-js:develop Apr 25, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants