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

Adjusting prose content to max 70ch wide #623

Merged
merged 4 commits into from Mar 21, 2023
Merged

Adjusting prose content to max 70ch wide #623

merged 4 commits into from Mar 21, 2023

Conversation

tony-sull
Copy link
Contributor

@tony-sull tony-sull commented Mar 21, 2023

Updating the prose styling used in blog posts, theme detail pages, and generic pages like T&Cs and Privacy Policy

Note: I also shrunk the width of blog post cover images slightly, it felt a bit too big to me with the narrower blog post content

@netlify
Copy link

netlify bot commented Mar 21, 2023

Deploy Preview for astro-www-2 ready!

Name Link
🔨 Latest commit 637c3ed
🔍 Latest deploy log https://app.netlify.com/sites/astro-www-2/deploys/6419d431b20b7e000830806a
😎 Deploy Preview https://deploy-preview-623--astro-www-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.

@@ -2,8 +2,6 @@
export interface Props {
src: string
}

const { src } = Astro.props as Props
Copy link
Contributor Author

Choose a reason for hiding this comment

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

linter fix: unused variable

@@ -85,6 +85,11 @@
@apply mx-0.5 inline-block rounded-lg bg-astro-gray-500 px-2 align-baseline text-sm leading-6 text-astro-gray-100;
}

.prose :is(h1, h2, h3, h4, h5) > code {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

noticed a styling bug in blog posts with <code> in a heading, font-size and line-height need to be inherited in this case

@tony-sull tony-sull marked this pull request as draft March 21, 2023 15:39
@tony-sull tony-sull marked this pull request as ready for review March 21, 2023 16:45
Copy link
Member

@Yan-Thomas Yan-Thomas left a comment

Choose a reason for hiding this comment

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

LGTM @tony-sull!

I just have this small nit: the Twitter card isn't filling the 70ch width (or being centered). Considering it's an iframe it might not be seamless to change, and not something I'd say is blocking, so I'm fine with leaving this way.

image

@tony-sull
Copy link
Contributor Author

@Yan-Thomas Those Twitter embeds are the worst 😅 Their iframe actually breaks responsive layouts in weird ways by reading the rendered width once when it loads and inlining a width: 500px that can overflow when the window is resized...

Consider it a high-priority TODO to replace those any Tweet in our blog with a static representation of the tweet, we don't need all that JS in a blog post!

@tony-sull tony-sull merged commit f63ce7f into main Mar 21, 2023
6 checks passed
@tony-sull tony-sull deleted the nit/prose-width branch March 21, 2023 20:06
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.

None yet

2 participants