Fix custom components markdown parsing #1548
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ang-zeyu
reviewed
Apr 17, 2021
packages/core/src/lib/markdown-it/plugins/custom-component/customComponentPlugin.js
Outdated
Show resolved
Hide resolved
packages/core/src/lib/markdown-it/plugins/custom-component/inlineTags.js
Outdated
Show resolved
Hide resolved
packages/core/src/lib/markdown-it/plugins/custom-component/htmlBlockRule.js
Outdated
Show resolved
Hide resolved
ang-zeyu
reviewed
Apr 21, 2021
packages/core/src/lib/markdown-it/patches/custom-component/htmlBlockRule.js
Outdated
Show resolved
Hide resolved
ang-zeyu
reviewed
Apr 21, 2021
packages/core/src/lib/markdown-it/patches/custom-component/htmlBlockRule.js
Outdated
Show resolved
Hide resolved
ang-zeyu
reviewed
Apr 21, 2021
packages/core/src/lib/markdown-it/patches/custom-component/htmlBlockRule.js
Outdated
Show resolved
Hide resolved
packages/core/src/lib/markdown-it/patches/custom-component/htmlBlockRule.js
Show resolved
Hide resolved
packages/core/src/lib/markdown-it/patches/custom-component/htmlBlockRule.js
Outdated
Show resolved
Hide resolved
ang-zeyu
reviewed
Apr 21, 2021
...cli/test/functional/test_site/expected/requirements/NonFunctionalRequirements._include_.html
Outdated
Show resolved
Hide resolved
ang-zeyu
reviewed
Apr 21, 2021
| requirements capture are some of the terms commonly <strong>and</strong> interchangeably used to represent the activity | ||
| of understanding what a software product should do.</seg> | ||
| </p> | ||
| <seg id="preview">Requirements gathering, requirements elicitation, requirements analysis, |
There was a problem hiding this comment.
Hmm yep, that would be good. On another note, should we update to something else in the SSR PR since we don't "allow" unknown components?
sounds good, over here might be better in fact since you've changed the behaviour of unknown tags
packages/cli/test/functional/test_site/expected/testIncludeMultipleModals.html
Outdated
Show resolved
Hide resolved
packages/cli/test/functional/test_site/testPanelMarkdownParsing.md
Outdated
Show resolved
Hide resolved
ang-zeyu
approved these changes
Apr 21, 2021
wxwxwxwx9
added a commit
to wxwxwxwx9/markbind
that referenced
this pull request
Apr 22, 2021
wxwxwxwx9
added a commit
to wxwxwxwx9/markbind
that referenced
this pull request
Apr 22, 2021
wxwxwxwx9
added a commit
to wxwxwxwx9/markbind
that referenced
this pull request
Apr 22, 2021
wxwxwxwx9
added a commit
to wxwxwxwx9/markbind
that referenced
this pull request
Apr 22, 2021
wxwxwxwx9
added a commit
to wxwxwxwx9/markbind
that referenced
this pull request
Apr 22, 2021
…tion issues but resolved by MarkBind#1548)
wxwxwxwx9
added a commit
to wxwxwxwx9/markbind
that referenced
this pull request
Apr 22, 2021
…ation issues but resolved by MarkBind#1548)
|
Remark: this PR introduces a lot of breaking changes in the form of properly defined block/inline behaviour. (should be a step in the right direction irregardless of SSR or not) |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What is the purpose of this pull request?
Resolves #958.
This issue is also related to #1534. We were prompted to re-visit #958 as it needs to be fixed before we can proceed to implement #1534.
As @ang-zeyu mentioned, we have never defined how our components should behave as html tags (block / inline (default)) regarding markdown.
Thus, we decided to adapt VuePress implementation to fit our own needs. VuePress treats every custom component (or unknown tags) as block-level element. This applies mostly in our use-case as well, except for
<panel minimized>component (panels which are minimized), which is treated as inline element.Overview of changes:
Adapted VuePress
markdown-itpatch for custom components.Anything you'd like to highlight / discuss:
The patch used by VuePress replaces the
html_blockrule inmarkdown-it. However, on our side, we are also replacinghtml_blockrule withspecial-escape-tagsalready.Thus, instead of modifying / merging
special-escape-tagsrule and VuePress'scustom-componentrule, I decided to just add VuePress's custom component patch rule underspecial-escape-tagsrule.Should I look into combining the rules together somehow? Seeing that there is a bit of code duplication here and time-wise it is a little more expensive as
markdown-ithas to run through an additional rule (which can be combined into one).But I think I am more comfortable with not combining them for separation of concern reasons. It would be easier to read the code with them being separate in my opinion.
Testing instructions:
npm run testTry out the examples as raised by @ang-zeyu in #958. The bugs should be fixed.
However, when you use the minimized option for panel, then the "bug" (not actually a bug since it is expected behaviour) should still be present, as minimized panel is treated as an inline element instead of block-level element.
Proposed commit message: (wrap lines at 72 characters)
Fix custom components markdown parsing.
We have never defined the behavior of our custom components as html tags
(whether block-level or inline) in markdown parsing. This causes some
undesirable edge cases during markdown parsing, as raised up in #958.
Let's adopt the way VuePress define the behaviour for custom components
and adapt it to fit our needs.
Checklist: ☑️