Skip to content

Fixes for layout and pre styles - #45

Merged
rezrah merged 7 commits into
mainfrom
danielguillan/fix-styles
Jun 20, 2025
Merged

Fixes for layout and pre styles#45
rezrah merged 7 commits into
mainfrom
danielguillan/fix-styles

Conversation

@danielguillan

@danielguillan danielguillan commented Jun 19, 2025

Copy link
Copy Markdown
Contributor
  • Updates the .Article grid layout to prevent horizontal scrolling.
screenshot-3snUjdFA-000263.mp4
Before After
screenshot-4YeYvPoo-000260@2x screenshot-xCCxXqso-000259@2x
  • Makes index cards full-width for a bore balanced layout on narrow viewports
Before After
screenshot-2ktLfPyi-000265@2x screenshot-ppRrYSNH-000266@2x

Copilot AI review requested due to automatic review settings June 19, 2025 15:40
@danielguillan
danielguillan requested a review from a team as a code owner June 19, 2025 15:40

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull Request Overview

This PR addresses unintended horizontal scrolling in the article layout and prevents global pre styles from affecting other code previews.

  • Scoped global pre styles under .nextra-code to contain styling to Nextra code blocks.
  • Removed right: 0 from the Table of Contents CSS to adjust its positioning.
  • Refactored .Article to use CSS Grid with responsive grid-template-areas and columns, and added overflow control to the main content area.

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File Description
packages/theme/css/global.css Scoped pre styles under .nextra-code with an explanatory comment.
packages/theme/components/layout/table-of-contents/TableOfContents.module.css Removed right: 0; from the TOC container to change its anchoring.
packages/theme/components/layout/article/Article.module.css Switched to CSS Grid for .Article, added responsive grid areas/columns, and set overflow rules on .main.

Comment thread packages/theme/components/layout/article/Article.module.css Outdated
Comment thread packages/theme/components/layout/article/Article.module.css Outdated
@danielguillan
danielguillan requested a review from rezrah June 19, 2025 15:43
danielguillan and others added 2 commits June 19, 2025 17:44
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@danielguillan
danielguillan removed the request for review from rezrah June 19, 2025 15:47
@danielguillan
danielguillan marked this pull request as draft June 19, 2025 15:47
@danielguillan
danielguillan requested a review from rezrah June 19, 2025 15:57
@danielguillan
danielguillan marked this pull request as ready for review June 19, 2025 15:57
}

@media screen and (max-width: 48rem) {
@media screen and (max-width: 1023px) {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

In hindsight it's pretty odd that we don't have an equivalent size for this in Primer Brand 🤔

Guessing you can't use 64rem here Dani? Would convert to 1024px. Or 63.938rem to at least be consistent in using rem (converts to 1023px)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I'm happy to go with either option. We previously used 1023px a few lines below, and I found it clearer than using a float rem like 63.938rem. Let me know what you prefer, and I can make the update.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Yeah i know what you mean. Let's keep as-is for now if we're already using it. Maybe let's separately chat about introducing a breakpoint in PB for either 1023px or 1024px?

@rezrah rezrah left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Works great 🙌 just a nit about px values in the breakpoint

@rezrah
rezrah merged commit 42990bd into main Jun 20, 2025
7 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.

[Next docs] Exclude default pre styling from React code previews

3 participants