SVG element selection refactoring

[nirname]

📑 Summary

Set repetitive chunk of code aside

• c4
• class
• common
• er
• error
• flowchart
• gantt
• git
• info
• mindmap
• pie
• quadrant-chart
• requirement
• sankey
• sequence
• state
• timeline
• user-journey

📏 Design Decisions

📋 Tasks

Make sure you

• 📖 have read the contribution guidelines
• 💻 have added unit/e2e tests (if appropriate)
• 📓 have added documentation (if appropriate)
• 🔖 targeted develop branch

[codecov]

Codecov Report

Merging #4609 (ff9f1c5) into develop (194ef20) will decrease coverage by 0.02%.
The diff coverage is 56.09%.

Additional details and impacted files

@@             Coverage Diff             @@
##           develop    #4609      +/-   ##
===========================================
- Coverage    76.99%   76.97%   -0.02%     
===========================================
  Files          143      144       +1     
  Lines        14455    14465      +10     
  Branches       516      515       -1     
===========================================
+ Hits         11130    11135       +5     
- Misses        3220     3227       +7     
+ Partials       105      103       -2     

Flag	Coverage Δ
e2e	84.01% <72.22%> (+0.05%)	⬆️
unit	45.22% <32.35%> (-0.06%)	⬇️

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

Impacted Files	Coverage Δ
...maid/src/rendering-util/selectElementsForRender.ts	43.75% <43.75%> (ø)
packages/mermaid/src/diagrams/c4/c4Renderer.js	88.25% <100.00%> (+0.16%)	⬆️
packages/mermaid/src/diagrams/er/erRenderer.js	93.88% <100.00%> (+0.41%)	⬆️
...ckages/mermaid/src/diagrams/error/errorRenderer.ts	41.58% <100.00%> (+0.58%)	⬆️
...ckages/mermaid/src/diagrams/gantt/ganttRenderer.js	87.98% <100.00%> (+0.74%)	⬆️
packages/mermaid/src/diagrams/info/infoRenderer.ts	90.00% <100.00%> (+23.33%)	⬆️
...ages/mermaid/src/diagrams/sankey/sankeyRenderer.ts	92.15% <100.00%> (+6.97%)	⬆️

[Yokozuna59]

I'd really like to see them refactored, but if you were planning to refactor them all in one PR, it would be hard to review. I was planning to do the same thing in #4486 (originally it was standardizing all diagrams), but @sidharthv96 suggested splitting it into separate PRs.

So I'd suggest the same thing, refactor the info, then apply changes to other diagram in separate PRs.

Please see #4514, it would help you know to the structure we following with the issue we are facing with sandbox.

[Yokozuna59]

The file name is kinda long, the folder containing this is already rendering-util, so ForRendering part is kinda clear. The selectElements part is kinda referring to it could select and return multiple elements, whereas it it's only one.

Not sure what to suggest :).

---

For lines at the end of file, see #4514 to update because it's currently incorrect.

---

Sorry I could refer to specific lines in review but github in phone isn't as PC, I don't know how to review here :)

[nirname]

I didn't know that you had similar PR and ideas about that. This is simply a draft. There is a complete mess inside draw functions so I put it aside for now

[Yokozuna59]

tbh, I didn't thought of creating a file for selecting, although we should since it's a common lines in all diagram :), so thanks for that!

I'll be working in convert it files and delete some of the randomness there, but I don't have time rn.

[nirname]

Separate file is not an ultimate dream.
Too many side effects inside draw function is happening

draw takes svg element

To my mind draw function of each particular graph must accept the target element, may be along with parent element, document etc. They don have to call this "getElements" function directly, base class or interface must provide it to them.

render function with parameters

Even better would be a render function that:

• Takes parsed graph definition
• Takes options (dimensions and whatever is needed for the graph), extracted from element beforehand
• Returns some data, including string with svg content, title or any other external attributes for html elements, some additional properties such as callbacks for clicks

[nirname]

Besides we are selecting these elements in mermaidAPI, by the time of calling draw function we already know what they are.
And render function in mermaidAPI does actually the same

  return {
    svg: svgCode,
    bindFunctions: diag.db.bindFunctions,
  };

I don see a reason why diagram-dedicated functions would violate this interface, so that I could call something like mermaidAPI.render as well as mermaid.Info.render

[nirname]

Closing this, improvement is being continued here #4514