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

docs: add documentation for custom admonitions #5625

Closed

Conversation

essential-randomness
Copy link

Motivation

I found myself needing to add a custom admonition, and thought it would be worth adding documentation on how to do so.

I've considered adding the admonition itself to this repository to be able to preview it in the docs, but I didn't want to propose a config change unless you agree beforehand.

Have you read the Contributing Guidelines on pull requests?

Yes.

(Unfortunately, I didn't do so before committing the change, and I'm uncertain how to change the commit message to adhere to the Semantic Commit Messages.)

Test Plan

No code change.

Related PRs

None.

Hello,
I found myself needing to add a custom admonition, and thought it would be worth adding documentation on how to do so. 

I've considered adding the admonition itself to this repository to be able to preview it in the docs, but I didn't want to propose a config change unless you agree beforehand.
@facebook-github-bot
Copy link
Contributor

Hi @essential-randomness!

Thank you for your pull request and welcome to our community.

Action Required

In order to merge any pull request (code, docs, etc.), we require contributors to sign our Contributor License Agreement, and we don't seem to have one on file for you.

Process

In order for us to review and merge your suggested changes, please sign at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need to sign the corporate CLA.

Once the CLA is signed, our tooling will perform checks and validations. Afterwards, the pull request will be tagged with CLA signed. The tagging process may take up to 1 hour after signing. Please give it that time before contacting us about it.

If you have received this in error or have any questions, please contact us at [email protected]. Thanks!

@netlify
Copy link

netlify bot commented Sep 28, 2021

✔️ [V2]
Built without sensitive environment variables

🔨 Explore the source changes: e70ade3

🔍 Inspect the deploy log: https://app.netlify.com/sites/docusaurus-2/deploys/615392f7d832dc0007db04a2

😎 Browse the preview: https://deploy-preview-5625--docusaurus-2.netlify.app

@github-actions
Copy link

⚡️ Lighthouse report for the changes in this PR:

Category Score
🟢 Performance 95
🟢 Accessibility 98
🟢 Best practices 100
🟢 SEO 100
🟢 PWA 95

Lighthouse ran on https://deploy-preview-5625--docusaurus-2.netlify.app/

Copy link
Collaborator

@slorber slorber left a comment

Choose a reason for hiding this comment

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

Thanks for adding this missing doc

Some suggestions:

  • Sign CLA (required)
  • Remove refs to Infima
  • rename custom to admonitionExample
  • Add config + custom CSS + example directly in the doc page


If you're using `@docusaurus/preset-classic`, you can create custom admonitions by adding them to your website's `docusaurus.config.js`.

For example, the following configuration adds an admonition with the `custom` keyword. This admonition will be styled as an [Infima `alert-info` component](https://infima.dev/docs/components/alert):
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'd rather not couple this doc to Infima, considering we'll provide themes that are not using Infima in the future.

What you can say is that the admonition will have a .admonition-custom classname that users can target with custom CSS

// ...
admonitions: {
customTypes: {
custom: {
Copy link
Collaborator

Choose a reason for hiding this comment

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

"custom" may not be the best choice, I'd rather use "admonitionExample"

admonitions: {
customTypes: {
custom: {
ifmClass: "info",
Copy link
Collaborator

Choose a reason for hiding this comment

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

Remove infima class

Copy link
Collaborator

Choose a reason for hiding this comment

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

This is directly passed to remark-admonitions. Surprising it may sound, but remark-admonitions is built on top of Infima. I think we should build an unbiased remark plugin using Remark directive ourselves 🤷‍♂️

@Josh-Cena
Copy link
Collaborator

Hi @essential-randomness, please sign the CLA so your PR can be a merge candidate. Your PR has almost gone stale, but if you sign the CLA, I can help you complete it.

@Josh-Cena Josh-Cena added the pr: documentation This PR works on the website or other text documents in the repo. label Oct 30, 2021
@essential-randomness
Copy link
Author

Hi! I need an answer from the Facebook team before signing the CLA since I have some concerns about who has access to my information. I sent the email a month ago now, but haven't gotten a reply.

I'll ping them now. If anyone knows where to find the info, I'd love to get us all unblocked.

@Josh-Cena
Copy link
Collaborator

Okay, I see. Unfortunate that it's been so long, wonder if they would ever respond🙄 It's good to be vigilant. There are a lot of PRs far staler than this so we wouldn't mind waiting, but just pinging to check if everything's up-to-date.

@slorber
Copy link
Collaborator

slorber commented Nov 3, 2021

Hey

I'm going to close because I'm not sure you'll get a response for the CLA 😅 and I don't feel like it's a good idea to document this.

When migrating to MDX 2, we'll likely replace that admonition plugin, so doing what you suggest will be a breaking change that we probably don't want to encourage in the meantime (see #5848 (comment))

@slorber slorber closed this Nov 3, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr: documentation This PR works on the website or other text documents in the repo.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants