-
-
Notifications
You must be signed in to change notification settings - Fork 876
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
Refactor architecture diagram to use mermaid flowchart syntax #858
Conversation
This comment has been minimized.
This comment has been minimized.
Codecov ReportAll modified and coverable lines are covered by tests ✅
❗ Your organization needs to install the Codecov GitHub app to enable full functionality. Additional details and impacted files@@ Coverage Diff @@
## main #858 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 3 3
Lines 1354 1402 +48
Branches 113 114 +1
=========================================
+ Hits 1354 1402 +48 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like it. Except that the AST types are now no longer links.
Looks really tiny to me. |
Yes, I noticed that, but I couldn't find any information on how to add URLs to the links within the Mermaid diagrams. As an alternative, I thought of preserving the URLs by adding them as a footer or citation in Markdown, but I'm not entirely convinced. flowchart LR
markdown[markdown] --> remark[remark]
subgraph remarkMarkdown[react-markdown]
direction LR
remark -- mdast --> remarkPlugins[remark plugins]
remarkPlugins -- mdast --> remarkRehype[remark-rehype]
remarkRehype -- hast --> rehypePlugins[rehype plugins]
rehypePlugins -- hast --> components[components]
end
components --> react[react elements]
click markdown "https://commonmark.org" "Markdown Specification"
click remark "https://github.com/remarkjs/remark" "Remark Documentation"
click remarkPlugins "https://github.com/remarkjs/remark/blob/main/doc/plugins.md" "Remark Plugins"
click remarkRehype "https://github.com/remarkjs/remark-rehype" "Remark Rehype Documentation"
click rehypePlugins "https://github.com/rehypejs/rehype/blob/main/doc/plugins.md" "Rehype Plugins Documentation"
click components "#appendix-b-components" "Components Documentation"
|
I really have my doubts on this. I do not want to live in a world where ASCII-diagrams are unprofessional. I don’t think all those zoom controls are useful. I think this is fine. @ChristianMurphy I think you originally made this graph? Thoughts? |
I like this, but I do have my doubts considering the readme may be displayed in places that don’t support mermaid. |
@ChristianMurphy Ping :) |
I made this, and based it off the style of the unified architecture diagram https://github.com/unifiedjs/unified?tab=readme-ov-file#overview I quite like Mermaid as a diagramming tool and don't have any objections to moving this over. |
OK. I will not block mermaid per-se. I personally like ASCII more. I do think that splitting this graph up into both a graph, and then some links under it, is not an improvement. If there is a way to fix that, I am 🤷♂️ to this PR. @cychitivav Can you somehow link mdast/hast? Report upstream? |
I’m also leaning no, but mostly because the Mermaid diagram might not be rendered in places other than GitHub. |
OK, I’ll close this then. I appreciate you contributing, @cychitivav. Thanks, but no thanks! |
Initial checklist
Description of changes
Refactored the architecture diagram using Mermaid flowchart syntax to improve readability and enhance the visual representation for a clearer understanding of the project structure, improving professionalism despite losing the URLs of links between blocks.