Skip to content
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

🖌 Add aside,margin and sidebar directives #1012

Merged
merged 3 commits into from
Mar 21, 2024

Conversation

agoose77
Copy link
Contributor

@agoose77 agoose77 commented Mar 20, 2024

This PR replaces #1010 with an aside directive, renaming margin, e.g.

:::{aside}
This content is an aside / margin note
:::

There is also the margin alias, and sidebar which has a different kind.

@agoose77
Copy link
Contributor Author

agoose77 commented Mar 20, 2024

@rowanc1 made a good point regarding the interpretation of the AST for a sidebar/margin. Really, these form "asides" which may have subtypes e.g. sidebar.

I've defined a new aside node that has an optional kind. This allows use to implement new kinds of asides, although I don't even know if we want to support sidebars right now. In this PR, margin and aside are semantically identical.

@agoose77 agoose77 force-pushed the agoose77/enh-add-aside-directive branch from b688080 to 58eae81 Compare March 20, 2024 15:28
@agoose77 agoose77 changed the title feat: add aside directive 🖌️ add aside directive Mar 20, 2024
@agoose77 agoose77 force-pushed the agoose77/enh-add-aside-directive branch from 58eae81 to 9071c59 Compare March 20, 2024 15:38
docs/asides.md Outdated
Comment on lines 10 to 21
````{myst}
The main article contains main-article text!

```{aside}
This is an aside. It is not entirely relevant to the main article.
```
````
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This one might be easier to start from the jupyter-books docs on this and demonstrate it in the page rather than in a demo component?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think an aside should be a MyST feature, but I would not be surprised if aside isn't very clear in the demo component. We can enable a grid layout on the MyST demo component if needs be.

Welcome your thoughts here if you think it would be better not to use the myst directive and instead include the aside in the main guide body. But, I think we can fix-up the demo component to make it possible to see, probably.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, I was saying to mimic the jupyter-book docs here. The demo component makes it look squished and isn't the best way to demo this I don't think.

Updating the docs a bit:

image

Going to merge, and we can come back to improving the docs.

@agoose77 agoose77 changed the title 🖌️ add aside directive 🖌️ Add aside directive Mar 20, 2024
@agoose77 agoose77 self-assigned this Mar 20, 2024
@rowanc1 rowanc1 changed the title 🖌️ Add aside directive 🖌 Add aside directive Mar 21, 2024
Comment on lines +1 to +2
title: aside directive
cases:
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the tests!

"myst-directives": minor
---

Add `aside` directive
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚀

@rowanc1 rowanc1 force-pushed the agoose77/enh-add-aside-directive branch from 41d55a8 to aa7fa2e Compare March 21, 2024 20:18
@rowanc1 rowanc1 merged commit a9250ae into main Mar 21, 2024
4 checks passed
@rowanc1 rowanc1 deleted the agoose77/enh-add-aside-directive branch March 21, 2024 20:21
@rowanc1 rowanc1 changed the title 🖌 Add aside directive 🖌 Add aside,margin and sidebar directives Mar 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants