Give markers unique id's per graph #4825

chadfawcett · 2023-09-08T22:45:10Z

📑 Summary

This PR makes markers have unique IDs based on the graph they are within.

Resolves #1871
Supersedes #3546 - This PR seemed abandoned

📏 Design Decisions

While #3546 seemed to resolve #1871, it appeared to be abandoned and also had a lot of syntax changes. I decided to keep the fix simple so hopefully it's easier to review and quicker to merge.

This implementation gives markers unique IDs based on the graph they are within. By using the ID, rather than some random string, we are able to reference the markers based on graph container ID when re-positioning.

📋 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.~~
🔖 targeted develop branch

netlify · 2023-09-08T22:45:14Z

✅ Deploy Preview for mermaid-js ready!

Name	Link
🔨 Latest commit	`983cf45`
🔍 Latest deploy log	https://app.netlify.com/sites/mermaid-js/deploys/6502b816cefe510008e4a755
😎 Deploy Preview	https://deploy-preview-4825--mermaid-js.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

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

codecov · 2023-09-08T22:49:24Z

Codecov Report

Merging #4825 (983cf45) into develop (ce78ff1) will increase coverage by 6.59%.
The diff coverage is 90.32%.

Additional details and impacted files

@@             Coverage Diff             @@
##           develop    #4825      +/-   ##
===========================================
+ Coverage    73.11%   79.70%   +6.59%     
===========================================
  Files          138      148      +10     
  Lines        12544    13024     +480     
  Branches       603      612       +9     
===========================================
+ Hits          9171    10381    +1210     
+ Misses        3225     2510     -715     
+ Partials       148      133      -15

Flag	Coverage Δ
e2e	`84.75% <90.32%> (+7.94%)`	⬆️
unit	`43.67% <ø> (ø)`

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

Files Changed	Coverage Δ
packages/mermaid/src/dagre-wrapper/edges.js	`83.97% <84.21%> (+7.66%)`	⬆️
packages/mermaid/src/dagre-wrapper/index.js	`98.82% <100.00%> (ø)`
packages/mermaid/src/dagre-wrapper/markers.js	`100.00% <100.00%> (ø)`

... and 43 files with indirect coverage changes

sidharthv96

@chadfawcett the changes looks great!

Can you please add an integration test to verify that the issue is solved?

The content from #1871 can be in the html file.
You can refer https://github.com/mermaid-js/mermaid/blob/bf18f25c820fb9264fe70ce5113a85f676cfd306/cypress/integration/other/ghsa.spec.js on how it should be done.

chadfawcett · 2023-09-11T19:03:03Z

@sidharthv96 I didn't see any documentation around contributing to the tests, so I did my best guess at what you might want. It'd be nice to see some documentation about what you expect in the CONTRIBUTING.md doc with best practices around naming and file location etc.

Here's a screenshot of what I see in Cypress. Let me know if you need me to make changes to what I've done.

sidharthv96

The test is great.
I've added 2 more cases.

cypress/platform/marker_unique_id.html

Co-authored-by: Sidharth Vinod <sidharthv96@gmail.com>

* develop: chore: Fix type in 'getLineFunctionsWithOffset' refactor: Add getLineFunctionsWithOffset function refactor: Move EdgeData to types fix: PointStart marker refX chore(deps): update all patch dependencies refactor: Fix typings in utils.ts chore: Add @internal to createCSSStyles chore: Bump version refactor: Remove unused variables fix: mermaid-js#4818 support `getClasses` in external diagrams.

mermaid-bot · 2023-09-14T08:08:35Z

@chadfawcett, 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.

chadfawcett · 2023-09-14T18:37:08Z

Thanks @sidharthv96 and @knsv for getting this merged in. Any idea when I could expect this fix to be released in a new version on npm?

sidharthv96 · 2023-09-14T19:33:48Z

Our plan was to release today, but we discovered some bugs while testing. So that has blocked the release.

[![Mend Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com) This PR contains the following updates: | Package | Change | Age | Adoption | Passing | Confidence | |---|---|---|---|---|---| | [mermaid](https://togithub.com/mermaid-js/mermaid) | [`10.4.0` -> `10.5.0`](https://renovatebot.com/diffs/npm/mermaid/10.4.0/10.5.0) | [![age](https://developer.mend.io/api/mc/badges/age/npm/mermaid/10.5.0?slim=true)](https://docs.renovatebot.com/merge-confidence/) | [![adoption](https://developer.mend.io/api/mc/badges/adoption/npm/mermaid/10.5.0?slim=true)](https://docs.renovatebot.com/merge-confidence/) | [![passing](https://developer.mend.io/api/mc/badges/compatibility/npm/mermaid/10.4.0/10.5.0?slim=true)](https://docs.renovatebot.com/merge-confidence/) | [![confidence](https://developer.mend.io/api/mc/badges/confidence/npm/mermaid/10.4.0/10.5.0?slim=true)](https://docs.renovatebot.com/merge-confidence/) | --- ### Release Notes <details> <summary>mermaid-js/mermaid (mermaid)</summary> ### [`v10.5.0`](https://togithub.com/mermaid-js/mermaid/releases/tag/v10.5.0): 10.5.0 [Compare Source](https://togithub.com/mermaid-js/mermaid/compare/v10.4.0...v10.5.0) #### What's Changed ##### Features - feat(er): add entity name alias by [@tomperr](https://togithub.com/tomperr) in [mermaid-js/mermaid#4758 ##### Bugfixes - Fix Twitter fontawesome class in flowchart.md by [@GingerNinjaNicko](https://togithub.com/GingerNinjaNicko) in [mermaid-js/mermaid#4723 - fix(pie): align slices and legend orders by [@Yokozuna59](https://togithub.com/Yokozuna59) in [mermaid-js/mermaid#4774 - Update class member handling by [@jgreywolf](https://togithub.com/jgreywolf) in [mermaid-js/mermaid#4534 - fix(er): allow underscore as leading char by [@tomperr](https://togithub.com/tomperr) in [mermaid-js/mermaid#4776 - Align arrows on sequence diagram by [@sidharthv96](https://togithub.com/sidharthv96) in [mermaid-js/mermaid#4804 - fix: Allow hollow markers on edges by [@sidharthv96](https://togithub.com/sidharthv96) in [mermaid-js/mermaid#4788 - fix: Fix for vulnerability making it possible to add javascript in class names by [@knsv](https://togithub.com/knsv) ##### Documentation - Docs/2910 Remove n00b and fix some docs by [@nirname](https://togithub.com/nirname) in [mermaid-js/mermaid#4767 - fix: typos by [@omahs](https://togithub.com/omahs) in [mermaid-js/mermaid#4801 - "CSS" instead of "css" in flowchart.md by [@jakeboone02](https://togithub.com/jakeboone02) in [mermaid-js/mermaid#4797 - fix(docs): Correct repeated text in flowchart.md by [@andriy-koz](https://togithub.com/andriy-koz) in [mermaid-js/mermaid#4810 - Update link to Discourse theme component by [@gschlager](https://togithub.com/gschlager) in [mermaid-js/mermaid#4811 - New Mermaid Live Editor for Confluence Cloud by [@zhifeiyue](https://togithub.com/zhifeiyue) in [mermaid-js/mermaid#4814 - Update classDiagram.md by [@jgreywolf](https://togithub.com/jgreywolf) in [mermaid-js/mermaid#4781 - Support member definition to initialize class by [@sidharthv96](https://togithub.com/sidharthv96) in [mermaid-js/mermaid#4786 - fix: Add support for `~test Array~string~` back in Class by [@sidharthv96](https://togithub.com/sidharthv96) in [mermaid-js/mermaid#4805 - Added support for millisecond and second to gantt tickInterval by [@vertxxyz](https://togithub.com/vertxxyz) in [mermaid-js/mermaid#4778 - Add directive support to all diagrams by preprocessing by [@sidharthv96](https://togithub.com/sidharthv96) in [mermaid-js/mermaid#4759 - Update README.md by [@jgreywolf](https://togithub.com/jgreywolf) in [mermaid-js/mermaid#4780 ##### Chores - chore(deps): update all minor dependencies (minor) by [@renovate](https://togithub.com/renovate) in [mermaid-js/mermaid#4783 - chore(deps): update all patch dependencies (patch) by [@renovate](https://togithub.com/renovate) in [mermaid-js/mermaid#4782 - chore(deps): update all patch dependencies (patch) by [@renovate](https://togithub.com/renovate) in [mermaid-js/mermaid#4809 - chore: move `commonDb` into `diagrams/common/commonDb` by [@Yokozuna59](https://togithub.com/Yokozuna59) in [mermaid-js/mermaid#4802 - Use utf8 encoding in Jupyter example by [@jonashaag](https://togithub.com/jonashaag) in [mermaid-js/mermaid#4701 - Update flowchart.md by [@Ogglas](https://togithub.com/Ogglas) in [mermaid-js/mermaid#4792 - Update flowchart.md by [@dsblank](https://togithub.com/dsblank) in [mermaid-js/mermaid#4798 - Refactor `cypress/helpers/util.ts` by [@RohanHandore](https://togithub.com/RohanHandore) in [mermaid-js/mermaid#4340 - refactor: Fix typings in utils.ts by [@sidharthv96](https://togithub.com/sidharthv96) in [mermaid-js/mermaid#4826 - Support ClassDefs in external diagrams by [@sidharthv96](https://togithub.com/sidharthv96) in [mermaid-js/mermaid#4819 - Fix: flowchartElk Arrow overlap by [@sidharthv96](https://togithub.com/sidharthv96) in [mermaid-js/mermaid#4830 - Give markers unique id's per graph by [@chadfawcett](https://togithub.com/chadfawcett) in [mermaid-js/mermaid#4825 #### New Contributors - [@GingerNinjaNicko](https://togithub.com/GingerNinjaNicko) made their first contribution in [mermaid-js/mermaid#4723 - [@omahs](https://togithub.com/omahs) made their first contribution in [mermaid-js/mermaid#4801 - [@jakeboone02](https://togithub.com/jakeboone02) made their first contribution in [mermaid-js/mermaid#4797 - [@andriy-koz](https://togithub.com/andriy-koz) made their first contribution in [mermaid-js/mermaid#4810 - [@gschlager](https://togithub.com/gschlager) made their first contribution in [mermaid-js/mermaid#4811 - [@zhifeiyue](https://togithub.com/zhifeiyue) made their first contribution in [mermaid-js/mermaid#4814 - [@vertxxyz](https://togithub.com/vertxxyz) made their first contribution in [mermaid-js/mermaid#4778 - [@jonashaag](https://togithub.com/jonashaag) made their first contribution in [mermaid-js/mermaid#4701 - [@Ogglas](https://togithub.com/Ogglas) made their first contribution in [mermaid-js/mermaid#4792 - [@dsblank](https://togithub.com/dsblank) made their first contribution in [mermaid-js/mermaid#4798 - [@RohanHandore](https://togithub.com/RohanHandore) made their first contribution in [mermaid-js/mermaid#4340 - [@chadfawcett](https://togithub.com/chadfawcett) made their first contribution in [mermaid-js/mermaid#4825 **Full Changelog**: mermaid-js/mermaid@v10.4.0...v10.5.0 </details> --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] If you want to rebase/retry this PR, check this box --- This PR has been generated by [Mend Renovate](https://www.mend.io/free-developer-tools/renovate/). View repository job log [here](https://developer.mend.io/github/levaintech/contented).  Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>

Give markers unique id's per graph

552d3ec

sidharthv96 approved these changes Sep 9, 2023

View reviewed changes

Added cypress test

924c9e9

sidharthv96 approved these changes Sep 12, 2023

View reviewed changes

cypress/platform/marker_unique_id.html Show resolved Hide resolved

chadfawcett and others added 2 commits September 12, 2023 10:29

Update cypress/platform/marker_unique_id.html

bceae92

Co-authored-by: Sidharth Vinod <sidharthv96@gmail.com>

knsv enabled auto-merge September 14, 2023 07:39

knsv added this pull request to the merge queue Sep 14, 2023

sidharthv96 added Area: Development Internals: Renderer fix labels Sep 14, 2023

Merged via the queue into mermaid-js:develop with commit d539f70 Sep 14, 2023
18 checks passed

chadfawcett deleted the bug/1871_fix_duplicate_marker_ids branch September 14, 2023 18:20

aloisklink mentioned this pull request Sep 16, 2023

Wait for marker_unique_id.html E2E test to render before taking a screenshot #4847

Merged

4 tasks

sidharthv96 mentioned this pull request Sep 16, 2023

Generated SVG has static IDs which cause issues when there are two SVGs #3267

Closed

sidharthv96 linked an issue Sep 16, 2023 that may be closed by this pull request

Generated SVG has static IDs which cause issues when there are two SVGs #3267

Closed

williamthome mentioned this pull request Sep 16, 2023

Mermaid duplicating ids livebook-dev/livebook#1440

Closed

mkuo-notion mentioned this pull request Oct 16, 2023

ID of flowchart subgraph SVG element breaks other scripts #4948

Open

renovate bot mentioned this pull request Nov 6, 2023

chore(deps): update npm packages aneoconsulting/ArmoniK#1201

Open

1 task

This was referenced Nov 14, 2023

Mermaid diagrams displayed incorrect when editing nextcloud/collectives#942

Closed

Usage of duplicate IDs across diagrams has potential to break some diagrams #1318

Closed

mehmetoguzderin mentioned this pull request Dec 6, 2023

Reorganize WGSL tools gpuweb/gpuweb#4408

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Give markers unique id's per graph #4825

Give markers unique id's per graph #4825

chadfawcett commented Sep 8, 2023 •

edited

netlify bot commented Sep 8, 2023 •

edited

codecov bot commented Sep 8, 2023 •

edited

sidharthv96 left a comment

chadfawcett commented Sep 11, 2023

sidharthv96 left a comment

mermaid-bot bot commented Sep 14, 2023

chadfawcett commented Sep 14, 2023

sidharthv96 commented Sep 14, 2023

Give markers unique id's per graph #4825

Give markers unique id's per graph #4825

Conversation

chadfawcett commented Sep 8, 2023 • edited

📑 Summary

📏 Design Decisions

📋 Tasks

netlify bot commented Sep 8, 2023 • edited

✅ Deploy Preview for mermaid-js ready!

codecov bot commented Sep 8, 2023 • edited

Codecov Report

sidharthv96 left a comment

Choose a reason for hiding this comment

chadfawcett commented Sep 11, 2023

sidharthv96 left a comment

Choose a reason for hiding this comment

mermaid-bot bot commented Sep 14, 2023

chadfawcett commented Sep 14, 2023

sidharthv96 commented Sep 14, 2023

chadfawcett commented Sep 8, 2023 •

edited

netlify bot commented Sep 8, 2023 •

edited

codecov bot commented Sep 8, 2023 •

edited