-
Notifications
You must be signed in to change notification settings - Fork 198
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
📚 DOCS: Full update #717
📚 DOCS: Full update #717
Conversation
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## master #717 +/- ##
==========================================
+ Coverage 89.93% 89.95% +0.01%
==========================================
Files 23 23
Lines 2892 2967 +75
==========================================
+ Hits 2601 2669 +68
- Misses 291 298 +7
Flags with carried forward coverage won't be shown. Click here to find out more.
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
98463fb
to
a92be0d
Compare
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.
Took a look through all the docs, block attrs and inline attributes look great. The hover-tool tips are great, as you know I am a big fan of showing contextual information. Also was funny to see some of the Shakespeare quote and poems I chose making their way back here.
Only concern in this PR is around the mixing of directive names and class names. I think that is fine if we adopt a widely used existing syntax (e.g. support quarto-style callouts). However, I don't think that we should suggest another way to do admonitions without addressing some of the existing confusions. e.g. in jupyter-book/myst-spec#49. @mmcky @choldgraf and I have been chatting about this recently, and some of our thoughts are going to be clarified in the next few weeks!
Can we please either (1) put a warning around this saying we might deprecate it; or (2) remove the functionality.
I would love to discuss how directives and class-names interact in a specific MEP, but I think that needs to come after we decide as a team on the block and inline syntax!
Just to link, as we discussed on slack; I have tried to kick start some conversation here, as to a shorthand to essentially specify a role/directive in attribute syntax: jgm/djot#146 |
28cad78
to
2c15fb6
Compare
This is a caption in **Markdown** | ||
::: | ||
``` | ||
::::{myst-example} |
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 think the docs should probably use sth like:

For an example. Hopefully most people use the md image syntax, not the HTML one :)
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.
Done thanks!
The live preview now renders only the body HTML, and inherits the outer theme
2c15fb6
to
b8b460a
Compare
Admonitions removed, can take another pass when this is closer to merge, please ping me!
4b1bc2c
to
d6d1cd0
Compare
d6d1cd0
to
b4fa8a8
Compare
Just about ready for a release!