feat(mdx-loader): add admonitions directive support for class/id shortcuts#11642
Conversation
✅ [V2]Built without sensitive environment variables
To edit notification comments on pull requests, go to your Netlify project configuration. |
There was a problem hiding this comment.
TIL that markdown-directives has shortcuts for class/id: https://talk.commonmark.org/t/generic-directives-plugins-syntax/444
What about also adding support for id then?
We would need to document that, yes, otherwise nobody will find out 🤪
| @@ -0,0 +1,5 @@ | |||
| Admonitions with attributes | |||
|
|
|||
| :::info[Info Title]{.bold} | |||
There was a problem hiding this comment.
Can you add more examples here, including with 2 classes, and interleaving multiple classes/ids and other non-shortcut attributes?
Looks like in case of duplicate ids, the last one wins in :::info{.c1 #id1 .c2 #id2 hello=world}
There was a problem hiding this comment.
Yes - i added more doogfood here: https://deploy-preview-11642--docusaurus-2.netlify.app/tests/docs/tests/admonitions/#admonitions-with-classes
Things to consider when we support passing attributes hello=world:
- this supports with the current remark-admonition plugin with it's hast-node approach the possibility to provide html attributes as
style="color:red"(what actually works as highlighted in the dogfood test). - this is actually a really nice feature, but...
- it brings possible hard regressions with it: when the remark admonition plugin upgrades to
mdxJsxFlowElement, props-passing gets more complicated, since react specific code, as thestyleattribute must be passed as an expression to be compatible with react. This would need some docusaurus-specific parse logic to transformstyle="color:red"tostyle={{color: 'red'}}. This can be done (i implemented something here), but i'm not sure wheter you want to maintain this in the longterm?
There was a problem hiding this comment.
Agree this is not ideal and I'd rather only support class/ids for now
Hi @slorber Thanks for your review. I think i addressed the highlighted points:
What do you think? |
| @@ -0,0 +1,5 @@ | |||
| Admonitions with attributes | |||
|
|
|||
| :::info[Info Title]{.bold} | |||
There was a problem hiding this comment.
Agree this is not ideal and I'd rather only support class/ids for now
|
Thanks for the suggestions - i cleaned up the pr and it should be ready to review again 🙂 |
Motivation
Add support for admonition directive class/id shortcuts, as specified in the directive syntax: https://talk.commonmark.org/t/generic-directives-plugins-syntax/444
Closes #11641
Test Plan
tests/docs/tests/admonitionsTest links