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

Admonition Customization tutorial #32

Merged
merged 11 commits into from
Jul 23, 2024
Merged

Conversation

dillp1
Copy link
Contributor

@dillp1 dillp1 commented Jul 9, 2024

This PR is set to add an admonition customization tutorial. The tutorial is aimed at beginners who have little or no prior CSS or HTML experience. The tutorial guides the user through the HTML using Inspect, and teaches them how to edit the simple CSS of Docusaurus to edit any html element.

Constructive feedback is appreciated for the clarity and correctness of the tutorial. I would be more than happy to add or change any content!

Copy link

github-actions bot commented Jul 9, 2024

⚠️ Deployments require the '🚀request-deploy' label

This repository is a forked repository. For security reasons, deployments from forked repositories are not automatic.

To request a deployment, add the '🚀request-deploy' label to this pull request. (Only some members can add labels).

Copy link
Contributor

@bohyunjung bohyunjung left a comment

Choose a reason for hiding this comment

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

Nice work @dillp1!

If you find my review helpful, please apply it to PR-wide (e.g., Change all tables to Markdown). You might also want to run a quick spell check on the document!

docs/design/admonitions/index.mdx Outdated Show resolved Hide resolved
docs/design/admonitions/index.mdx Outdated Show resolved Hide resolved
docs/design/admonitions/index.mdx Outdated Show resolved Hide resolved
docs/design/admonitions/index.mdx Outdated Show resolved Hide resolved
docs/design/admonitions/index.mdx Show resolved Hide resolved
@homotechsual homotechsual added the 🚀request-deploy Adding this label will request a preview build of your PR label Jul 10, 2024
Copy link

⚡ Successfully deployed to Cloudflare Pages!

| 🔨 Latest commit | 9b2e29e |
| 🔍 Latest deploy log | https://github.com/DocusaurusCommunity/website/actions/runs/9870800139 |
| 😎 Deploy preview URL | https://eba80249.docusauruscommunitywebsite.pages.dev |
| 🌳 Environment | preview |

@github-actions github-actions bot removed the 🚀request-deploy Adding this label will request a preview build of your PR label Jul 10, 2024
@dillp1
Copy link
Contributor Author

dillp1 commented Jul 11, 2024

I made some great changes and finally added that diagram (don't call me a graphic designer... I tried my best haha)

@dillp1 dillp1 requested a review from bohyunjung July 11, 2024 00:04
Copy link
Contributor

@bohyunjung bohyunjung left a comment

Choose a reason for hiding this comment

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

Looks good to me! Thanks for applying the suggestions.

Before the merge, I noticed a mix of comma-separated and comma-less syntax for HTML rgb / rgba representations. Both work in modern browsers, but it'd be better if we stick to one style;

  • I see another in-project page has comma-separated expressions.
  • However, surprisingly, according to the MDN document, comma-less expression is standard, while comma-separated is legacy.

@dillp1 @homotechsual What's your opinion on which one to adopt?

@dillp1
Copy link
Contributor Author

dillp1 commented Jul 11, 2024

Looks good to me! Thanks for applying the suggestions.

Before the merge, I noticed a mix of comma-separated and comma-less syntax for HTML rgb / rgba representations. Both work in modern browsers, but it'd be better if we stick to one style;

  • I see another in-project page has comma-separated expressions.
  • However, surprisingly, according to the MDN document, comma-less expression is standard, while comma-separated is legacy.

@dillp1 @homotechsual What's your opinion on which one to adopt?

I've always used comma-separated expressions in my experience coding with Java, C, and Python. Ultimately, I think @homotechsual has a bit more experience than me and could give a more technical answer, but I would use comma-separated based on my experience.

@homotechsual
Copy link
Collaborator

Looks good to me! Thanks for applying the suggestions.
Before the merge, I noticed a mix of comma-separated and comma-less syntax for HTML rgb / rgba representations. Both work in modern browsers, but it'd be better if we stick to one style;

  • I see another in-project page has comma-separated expressions.
  • However, surprisingly, according to the MDN document, comma-less expression is standard, while comma-separated is legacy.

@dillp1 @homotechsual What's your opinion on which one to adopt?

I've always used comma-separated expressions in my experience coding with Java, C, and Python. Ultimately, I think @homotechsual has a bit more experience than me and could give a more technical answer, but I would use comma-separated based on my experience.

Definitely comma separated imo.

@homotechsual
Copy link
Collaborator

So basically if you can standardise the rgb/rgba definitions - either with our without commas just standardised - I think this is good to merge.

@bohyunjung
Copy link
Contributor

bohyunjung commented Jul 21, 2024

@dillp1 Can you make another commit to make them all comma-separated? Let's get these merged!

@dillp1
Copy link
Contributor Author

dillp1 commented Jul 21, 2024

@bohyunjung will do when I’m free tomorrow! 😁

@dillp1
Copy link
Contributor Author

dillp1 commented Jul 23, 2024

Updated content to be more consistent with color values!

@dillp1 dillp1 requested a review from bohyunjung July 23, 2024 14:35
Copy link
Contributor

@bohyunjung bohyunjung left a comment

Choose a reason for hiding this comment

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

👏 LGTM!

@bohyunjung
Copy link
Contributor

@homotechsual Waiting for your approval and merge :)

@homotechsual homotechsual merged commit 10e1357 into DocusaurusCommunity:main Jul 23, 2024
2 checks passed
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.

4 participants