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

Replace all blockquote admonitions with the shortcode #7420

Merged
merged 1 commit into from
Feb 19, 2024

Conversation

jdbaldry
Copy link
Member

@jdbaldry jdbaldry commented Feb 19, 2024

The admonition shortcode renders its content in a blockquote with consistent styling across the website.

I hope that this will also encourage future contributions to avoid blockquote notes and use the shortcode since it is common throughout the documentation.

The [`admonition` shortcode](https://grafana.com/docs/writers-toolkit/write/shortcodes/#admonition) renders its content in a blockquote with consistent styling across the website.

Signed-off-by: Jack Baldry <jack.baldry@grafana.com>
@jdbaldry jdbaldry requested a review from a team as a code owner February 19, 2024 10:16
@pstibrany
Copy link
Member

pstibrany commented Feb 19, 2024

I hope that this will also encourage future contributions to avoid blockquote notes and use the shortcode since it is common throughout the documentation.

Blockquote syntax is much more memorable (TIL that admonition term exists), can we please keep using it and let the tools handle the rendering?

@jdbaldry
Copy link
Member Author

jdbaldry commented Feb 19, 2024

Blockquote syntax is much more memorable (TIL that admonition term exists), can we please keep using it and let the tools handle the rendering?

Mm, I do agree that it's easier to remember blockquote syntax.

The website squad maintains the HTML template for the admonition shortcode and ensures that it has consistent styling, and also proper semantic tags, and attributes for accessibility. Conversely, blockquotes with bold text are just blockquotes in the HTML output and are inconsistently used throughout the documentation.

Example of different usage I've seen in Mimir documentation (also present for tips, warnings, and cautions):

  • Note: Capitalized first word

  • NOTE: Capitalized first word

  • Note: Capitalized first word

  • NOTE: Capitalized first word

  • Note: lower first word

  • NOTE: lower first word

  • Note: lower first word

  • NOTE: lower first word

Unfortunately, there is no render hook in Hugo for blockquotes which means that the conversion cannot be done with Hugo alone -- gohugoio/hugo#7876.

Of course we could introduce a transpilation step in Mimir but I think that would be more awkward than using the shortcode.

Copy link
Member

@pstibrany pstibrany left a comment

Choose a reason for hiding this comment

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

Unblocking.

@jdbaldry
Copy link
Member Author

Thanks for taking another look @pstibrany.

I don't want to introduce additional complexity where it is not wanted but I do feel that the admonition shortcode is better than blockquote conventions. Of course, if there is anything I can do to make this easier to use or any feedback I pass on to the website squad, let me know and I will happily do that.

It would be great to merge this sooner rather than later because of the risk of conflicts with changes across so many files.

@pstibrany
Copy link
Member

It would be great to merge this sooner rather than later because of the risk of conflicts with changes across so many files.

Feel free to merge this. I see that there's too much complexity to make blockquotes "just work", so let's use "admonition" for now.

@jdbaldry
Copy link
Member Author

Thanks Peter, I agree.

I'm not authorized to merge PRs because I'm no longer a member of @grafana/mimir-maintainers. Would you mind doing the honors? :)

@pstibrany pstibrany merged commit a596e1a into main Feb 19, 2024
28 checks passed
@pstibrany pstibrany deleted the jdb/2024-02-admonitions branch February 19, 2024 14:36
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