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

feat(theme): Content width css variable for easier customization #3767

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

elliotcourant
Copy link

@elliotcourant elliotcourant commented Dec 5, 2024

This moves the content width that was used throughout nextra max-w-[90rem] to a css variable. This way the changing the value in the future should be easier, but it also makes it possible to customize the max width of the content for people using nextra without needing to overload a tailwind class in a hacky way.

Why:

I have several screenshots in my docs that are a pretty consistent 1280x720, due to the max width of the content view being limited to 90rem these images are always scaled down quite a bit. I was hoping to make the max width a bit wider for this reason but didn't see an easy way to do so without overloading the _max-w-[90rem] tailwind CSS class; which didn't seem like a good idea.

What's being changed (if available, include any code snippets, screenshots, or gifs):

I've simply lifted the value passed to max width to a css variable similar to the other css variables already being managed by nextra. This way I can simply overwrite that css variable in my own styles.

Check off the following:

  • I have reviewed my changes in staging, available via the View
    deployment
    link in this PR's timeline (this link will be available after
    opening the PR).

This moves the content width that was used throughout nextra
`max-w-[90rem]` to a css variable. This way the changing the value in
the future should be easier, but it also makes it possible to customize
the max width of the content for people using nextra without needing to
overload a tailwind class in a hacky way.
Copy link

changeset-bot bot commented Dec 5, 2024

🦋 Changeset detected

Latest commit: 8e03721

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 3 packages
Name Type
nextra-theme-docs Major
nextra Major
nextra-theme-blog Major

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

Copy link

vercel bot commented Dec 5, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
nextra-theme-docs-dev ✅ Ready (Inspect) Visit Preview 💬 Add feedback Dec 5, 2024 0:26am
nextra-v2 ✅ Ready (Inspect) Visit Preview 💬 Add feedback Dec 5, 2024 0:26am
1 Skipped Deployment
Name Status Preview Comments Updated (UTC)
nextra ⬜️ Ignored (Inspect) Visit Preview Dec 5, 2024 0:26am

Copy link

vercel bot commented Dec 5, 2024

@elliotcourant is attempting to deploy a commit to the Vercel Team on Vercel.

A member of the Team first needs to authorize it.

Copy link
Collaborator

@dimaMachina dimaMachina left a comment

Choose a reason for hiding this comment

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

I don't think we can merge it, because there are people who rely on _max-w-[90rem] class

but in v4 we can do this feature

for now you can override this class in your CSS

.max-w-\[90rem\] {
    max-width: 90vw !important;
}

@elliotcourant
Copy link
Author

I don't think we can merge it, because there are people who rely on _max-w-[90rem] class

but in v4 we can do this feature

for now you can override this class in your CSS

.max-w-\[90rem\] {
    max-width: 90vw !important;
}

Do you want me to redo this patch against the v4-v2 branch? I'd be happy to.

@dimaMachina
Copy link
Collaborator

Do you want me to redo this patch against the v4-v2 branch? I'd be happy to.

Yes, you are welcome 🙏

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