Bundle mermaid locally#2677
Conversation
- Updated mermaid to use local version (V10.6.0) if a specific version is not specified. - Updated tests.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #2677 +/- ##
========================================
Coverage 51.90% 51.90%
========================================
Files 130 130
Lines 7104 7104
Branches 1547 1572 +25
========================================
Hits 3687 3687
+ Misses 3263 3118 -145
- Partials 154 299 +145 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Tim-Siu
left a comment
Tim-Siu
left a comment
There was a problem hiding this comment.
An effective and straightforward fix. LGTM.
| import { PluginContext, FrontMatter } from './Plugin'; | ||
|
|
||
| const DEFAULT_CDN_ADDRESS = 'https://unpkg.com/mermaid@10/dist/mermaid.esm.min.mjs'; | ||
| const DEFAULT_MERMAID_PATH = './mermaidAssets/mermaid.min.js'; |
There was a problem hiding this comment.
Can be simplified const DEFAULT_MERMAID_PATH = 'mermaidAssets/mermaid.min.js'; as well. I used this in other plugins, but not sure which is the best practice.
There was a problem hiding this comment.
Ill update it so that it stays consistent with the convention that we are already using. I don't think there is much difference either, except maybe using ./ is a bit more explicit in defining the path.
lhw-1
left a comment
lhw-1
left a comment
There was a problem hiding this comment.
Thanks for the fix @Incogdino! Apart from the issues brought up by other reviewers, LGTM
1. Relative path now follows our convention to be more implicit. 2. Updated test cases.
…arkbind into branch-bundleMermaid
|
Regressions Found:
Mermaid rendering does not work in panels (dynamic rendered content)
Some further work to refactor the logic would be good |
5 participants
What is the purpose of this pull request?
Closes #2667
Part of #2548
Overview of changes:
is not specified.
Anything you'd like to highlight/discuss:
We could explore upgrading mermaid to a later version in a future issue.
Testing instructions:
Proposed commit message: (wrap lines at 72 characters)
Bundle mermaid locally
is not specified.
Checklist: ☑️
Reviewer checklist:
Indicate the SEMVER impact of the PR:
At the end of the review, please label the PR with the appropriate label:
r.Major,r.Minor,r.Patch.Breaking change release note preparation (if applicable):