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

Document inline stylesheets #3355

Merged
merged 9 commits into from
Jun 6, 2023
Merged

Document inline stylesheets #3355

merged 9 commits into from
Jun 6, 2023

Conversation

matthewp
Copy link
Contributor

What kind of changes does this PR include?

  • New or updated content

Description

  • Adds a section to the Styles & CSS page which explains inlining.

@netlify
Copy link

netlify bot commented May 26, 2023

Deploy Preview for astro-docs-2 ready!

Name Link
🔨 Latest commit e7aa122
🔍 Latest deploy log https://app.netlify.com/sites/astro-docs-2/deploys/647f647f71e8ca000830ccaa
😎 Deploy Preview https://deploy-preview-3355--astro-docs-2.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@sarah11918
Copy link
Member

sarah11918 commented May 26, 2023

I like this section, and I like it where it's located!

One quick thing that comes to mind is whether there are any Tailwind implications with this, since so many people will be using Tailwind. Is there something relevant or useful to say about whether or how this does/doesn't include/affect your Tailwind styles?

@lilnasy
Copy link
Contributor

lilnasy commented May 26, 2023

Stylesheets created by tailwind would be treated the same as those created by any other source.

@sarah11918
Copy link
Member

Thanks @lilnasy - do you think some people might need to explicitly hear that? Is this something a Tailwind user might be wondering? Or is it obvious enough. I'm trying to think of what might be going through someone's mind. 😄

@lilnasy
Copy link
Contributor

lilnasy commented May 26, 2023

I haven't used tailwind, so I can't get in the mindset of a user. Whether a reassurance is necessary depends on whether astro+tailwind users are used to things not working without fiddling.

@lilnasy
Copy link
Contributor

lilnasy commented May 26, 2023

Although even if that's not the case, two lines couldn't hurt.

This configuration applies to styles authored in <style> tags inside .astro files, css files imported into astro or framework components, and tailwind classes used by server-side rendered framework components. Tailwind classes from client-only react, solid, and svelte components don't contribute to bundled CSS as they are compiled into css rules in the browser. As always, .css files in the public folder are not processed.

@matthewp
Copy link
Contributor Author

@sarah11918 Tailwind is a large stylesheet most of the time, so most likely it will be bundled in a chunk that is not inlined. Unless you are only using a small amount of Tailwind styles. So it would be retrieved externally and cached between pages, which is what you want.

@sarah11918 sarah11918 added add new content Document something that is not in docs. May require testing, confirmation, or affect other pages. merge-on-release Don't merge this before the feature is released! (MQ=approved but WAIT for feature release!) labels May 26, 2023
Copy link
Member

@sarah11918 sarah11918 left a comment

Choose a reason for hiding this comment

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

This looks great, @matthewp! I tried some minor text changes but of course as always, may have lost some nuance in the process. See what you think, and take what you like!

src/content/docs/en/guides/styling.mdx Outdated Show resolved Hide resolved
src/content/docs/en/guides/styling.mdx Outdated Show resolved Hide resolved
src/content/docs/en/guides/styling.mdx Outdated Show resolved Hide resolved
src/content/docs/en/guides/styling.mdx Outdated Show resolved Hide resolved
matthewp and others added 4 commits May 30, 2023 17:44
Co-authored-by: Sarah Rainsberger <sarah@rainsberger.ca>
Co-authored-by: Sarah Rainsberger <sarah@rainsberger.ca>
Co-authored-by: Sarah Rainsberger <sarah@rainsberger.ca>
Co-authored-by: Sarah Rainsberger <sarah@rainsberger.ca>
Copy link
Member

@sarah11918 sarah11918 left a comment

Choose a reason for hiding this comment

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

Good to go!

Copy link
Member

@ElianCodes ElianCodes left a comment

Choose a reason for hiding this comment

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

Oh waw! I love this!! 🚀

@sarah11918 sarah11918 merged commit aedcab5 into main Jun 6, 2023
@sarah11918 sarah11918 deleted the css-inlining-2 branch June 6, 2023 16:59
@sarah11918 sarah11918 added the minor-release For the next minor release; in the milestone, "merge queue" when approved by Sarah! label Jun 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
add new content Document something that is not in docs. May require testing, confirmation, or affect other pages. merge-on-release Don't merge this before the feature is released! (MQ=approved but WAIT for feature release!) minor-release For the next minor release; in the milestone, "merge queue" when approved by Sarah!
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants