feat(Page): update glass spacers by kmcfaul · Pull Request #8427 · patternfly/patternfly

kmcfaul · 2026-05-20T16:09:00Z

Closes: #8393

Updates sidebar spacer from lg to md token.
Updates the gap between masthead, sidebar, and content to use gutter token instead of page chrome. Outer spacers around the page should still be using page chrome token.

Summary by CodeRabbit

Style
- Refined layout and spacing across themes: adjusted docked-nav, sidebar and main-container paddings/margins, recalculated main-container max-height, and updated XL breakpoint gutter behavior for more consistent display across breakpoints; root masthead horizontal padding now aligns with the inset page chrome spacing.
Chores
- Regenerated token files, refreshed generation timestamps, and introduced a new global inset page-chrome spacer token used by glass/felt themes.

coderabbitai · 2026-05-20T16:13:07Z

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

@coderabbitai resume to resume automatic reviews.
@coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

▶️ Resume reviews
🔍 Trigger review

Walkthrough

Page component docked and glass-theme spacing tokens were adjusted: docked container uses the inset page-chrome spacer; glass sidebar and sidebar-main margins/padding recalculated to use gutter and smaller spacers; generated token files refreshed and a new --pf-t--global--spacer--inset--page-chrome variable added and used by Masthead.

Changes

Glass Theme Spacing Updates

Layer / File(s)	Summary
Docked container spacing updates `src/patternfly/components/Page/page.scss`	Docked nav main container MaxHeight and desktop MarginBlockStart now use `--pf-t--global--spacer--inset--page-chrome` instead of `spacer--lg`.
Glass theme sidebar and sidebar-main token updates `src/patternfly/components/Page/page.scss`	Sidebar glass-mode `--MarginBlockStart--glass` and sidebar-main margins/inline offsets recalculated (use `spacer--xs`, `spacer--md`, `gutter--default`, and `inset--page-chrome` where applicable); glass-specific dock tokens added and wired.
XL breakpoint glass overrides `src/patternfly/components/Page/page.scss`	At `xl` breakpoint, sidebar-main `MarginInlineEnd--glass` and main container `MarginBlockStart--glass` switch to `gutter--default`; main container `MaxHeight--glass` updated to subtract `inset--page-chrome + gutter--default`.
Masthead padding switch `src/patternfly/components/Masthead/masthead.scss`	Masthead root `--#{$masthead}--PaddingInline` changed from `spacer--lg` to `--pf-t--global--spacer--inset--page-chrome`.
Generated token headers & new inset spacer `src/patternfly/base/tokens/*`	Refreshed “Generated on” timestamps across token files and added `--pf-t--global--spacer--inset--page-chrome` to glass/felt token mixins.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

patternfly/patternfly#8149: Overlapping edits to src/patternfly/components/Page/page.scss docked layout tokens.
patternfly/patternfly#8274: Related glass/sidebar-main variable recalculations in page.scss.
patternfly/patternfly#8388: Modifies Page glass-theme main container token and height logic related to inset spacing.

Suggested labels

Needs design review

Suggested reviewers

lboehling
jcmill
srambach

🚥 Pre-merge checks | ✅ 4

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title 'feat(Page): update glass spacers' follows conventional commit guidelines with a feature scope and clear subject.
Linked Issues check	✅ Passed	The PR successfully addresses all requirements from issue `#8393`: sidebar spacer changed from lg to md (24px to 16px), gap between masthead/sidebar/content reduced to use gutter token instead of page chrome token.
Out of Scope Changes check	✅ Passed	All changes are directly related to updating glass theme spacers. Token file timestamp updates are automatically regenerated and not out-of-scope changes.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

patternfly-build · 2026-05-20T16:14:36Z

Preview: https://pf-pr-8427.surge.sh

A11y report: https://pf-pr-8427-a11y.surge.sh

mcoker

Need to confirm with design:

whether page-chrome token stays the same or should change to 16px.
the mobile layout of the sidebar looks correct with the inset/offset

mcoker

Full backstop report

Spotted these two issues:

the masthead uses the "lg" spacer for it's left/right padding and that should be updated to use the page chrome spacer.
the page content in the docked nav layout uses a "lg" top margin when it should use the gutter spacer on mobile (when there is a masthead visible) and the page chrome spacer on desktop (when the content is the first element on the page)

coderabbitai · 2026-05-28T13:55:26Z

Actionable comments posted: 0

mcoker

Just a few comments on the vars. I still need to run visual regressions, but need to get them updated on the main branch first. Will re-review when those are updated.

coderabbitai

♻️ Duplicate comments (1)

src/patternfly/components/Page/page.scss (1)
239-241: ⚠️ Potential issue | 🔴 Critical | ⚡ Quick win

Remove duplicate variable definitions that break non-desktop spacing.

Lines 239-241 redefine the same variables already declared at lines 13, 14-15, and 16 within the same @include pf-root($page) scope. Because CSS applies the last definition, these duplicates override the earlier declarations:

Line 239 duplicates line 13 (same value, so no functional change)

Line 240 overrides line 14, breaking the intended non-desktop behavior where MarginBlockStart should be 0 (line 14), not inset-page-chrome

Line 241 duplicates line 16 (same value, so no functional change)

The duplication makes lines 13-16 partially dead code and breaks the mobile/non-desktop spacing contract.

Based on learnings from past review comments, these duplicate lines should be removed entirely.
🔧 Proposed fix
-  // Docked nav
-  --#{$page}--m-dock__main-container--MaxHeight: calc(100% - var(--pf-t--global--spacer--inset--page-chrome) * 2);
-  --#{$page}--m-dock__main-container--MarginBlockStart: var(--pf-t--global--spacer--inset--page-chrome);
-  --#{$page}--m-dock--ColumnGap: var(--pf-t--global--spacer--inset--page-chrome);
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/patternfly/components/Page/page.scss` around lines 239 - 241, Remove the
duplicate CSS custom-property declarations inside the same `@include`
pf-root($page) scope: delete the extra definitions of
--#{$page}--m-dock__main-container--MaxHeight,
--#{$page}--m-dock__main-container--MarginBlockStart, and
--#{$page}--m-dock--ColumnGap so the original declarations (which set
MarginBlockStart to 0 for non-desktop) are not overridden; keep the earlier
definitions intact and ensure no other code reintroduces these duplicates.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Duplicate comments:
In `@src/patternfly/components/Page/page.scss`:
- Around line 239-241: Remove the duplicate CSS custom-property declarations
inside the same `@include` pf-root($page) scope: delete the extra definitions of
--#{$page}--m-dock__main-container--MaxHeight,
--#{$page}--m-dock__main-container--MarginBlockStart, and
--#{$page}--m-dock--ColumnGap so the original declarations (which set
MarginBlockStart to 0 for non-desktop) are not overridden; keep the earlier
definitions intact and ensure no other code reintroduces these duplicates.

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository UI (base), Organization UI (inherited)

Review profile: CHILL

Plan: Pro

Run ID: 21e2b489-fb22-48fa-a520-851d6be8a8d0

📥 Commits

Reviewing files that changed from the base of the PR and between 1a3c6b7 and 1e6420a.

📒 Files selected for processing (1)

src/patternfly/components/Page/page.scss

mcoker

LGTM!

mcoker

Oops missed one spot. Looks like there are still duped vars here - https://github.com/kmcfaul/patternfly/blob/5cd7e7ef7b1fc9a170d0c5bcba6ce70eac4deab0/src/patternfly/components/Page/page.scss#L238-L241

You can delete the previously defined ones since those are not applying currently anyways. Then can you group these separate blocks so all of the vars are in a single docked nav section?

mcoker

🎃

lboehling

lgtm!

patternfly-build · 2026-06-05T18:15:45Z

🎉 This PR is included in version 6.6.0-prerelease.4 🎉

The release is available on:

Your semantic-release bot 📦🚀

mcoker reviewed May 26, 2026

View reviewed changes

mcoker requested a review from lboehling May 27, 2026 19:21

mcoker requested changes May 27, 2026

View reviewed changes

mcoker requested changes Jun 2, 2026

View reviewed changes

Comment thread src/patternfly/components/Page/page.scss Outdated

Comment thread src/patternfly/components/Page/page.scss Outdated

Comment thread src/patternfly/components/Page/page.scss Outdated

Comment thread src/patternfly/components/Page/page.scss Outdated

mcoker requested changes Jun 4, 2026

View reviewed changes

Comment thread src/patternfly/components/Page/page.scss Outdated

kmcfaul added 2 commits June 4, 2026 11:31

feat(Page): update glass spacers

6024aad

feat(page): update sidebar margins

0615bb7

coderabbitai Bot reviewed Jun 4, 2026

View reviewed changes

mcoker and others added 3 commits June 4, 2026 11:46

chore: update page chrome token

ef403a9

fix(): update docked page content margin, masthead padding tokens

779e858

fix(): move docked changes to glass only

5cd7e7e

kmcfaul force-pushed the glass-page-spacer branch from 1e6420a to 5cd7e7e Compare June 4, 2026 15:47

mcoker approved these changes Jun 4, 2026

View reviewed changes

mcoker requested changes Jun 4, 2026

View reviewed changes

fix(): dedup docked nav vars, colocate with dock vars

0688028

mcoker approved these changes Jun 4, 2026

View reviewed changes

lboehling approved these changes Jun 5, 2026

View reviewed changes

mcoker merged commit a4f5e07 into patternfly:main Jun 5, 2026
6 checks passed

patternfly-build added the released on @prerelease label Jun 5, 2026

Conversation

kmcfaul commented May 20, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented May 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Reviews paused

Walkthrough

Changes

Estimated code review effort

Possibly related PRs

Suggested labels

Suggested reviewers

Uh oh!

patternfly-build commented May 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mcoker left a comment

Choose a reason for hiding this comment

Uh oh!

mcoker left a comment

Choose a reason for hiding this comment

Uh oh!

coderabbitai Bot commented May 28, 2026

Uh oh!

mcoker left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

mcoker left a comment

Choose a reason for hiding this comment

Uh oh!

mcoker left a comment

Choose a reason for hiding this comment

Uh oh!

mcoker left a comment

Choose a reason for hiding this comment

Uh oh!

lboehling left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

patternfly-build commented Jun 5, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

kmcfaul commented May 20, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented May 20, 2026 •

edited

Loading

patternfly-build commented May 20, 2026 •

edited

Loading