Skip to content

fix(page): clean up glass/sidebar-main vars#8274

Merged
mcoker merged 2 commits into
patternfly:mainfrom
mcoker:issue-8273
Apr 6, 2026
Merged

fix(page): clean up glass/sidebar-main vars#8274
mcoker merged 2 commits into
patternfly:mainfrom
mcoker:issue-8273

Conversation

@mcoker

@mcoker mcoker commented Apr 3, 2026

Copy link
Copy Markdown
Contributor

fixes #8273
fixes #8243

Creates proper variables for a bunch of glass styles, mostly around the sidebar/sidebar-main styles. If done properly, this PR should not introduce any changes. I ran visual regressions and looks like it produced no errors but did fix a collapsed sidebar issue as noted in #8243

Here is the backstop report: BackstopJS Report.pdf

FWIW .pf-v6-c-page__sidebar-main is a new element and is not required unless you're using glass theme.

Summary by CodeRabbit

  • Style

    • Refined sidebar spacing, padding and visual treatments for a cleaner, more consistent layout.
    • Improved "glass" theme visuals and responsive adjustments for larger screens.
    • Tweaked sidebar scrolling behavior so overflow behaves more predictably.
  • Refactor

    • Consolidated sidebar and secondary-section styling into theme-driven variables for easier maintenance.

@mcoker mcoker marked this pull request as ready for review April 3, 2026 20:31
@coderabbitai

coderabbitai Bot commented Apr 3, 2026

Copy link
Copy Markdown
Contributor

Walkthrough

Moved Page sidebar/main visual and layout rules to new CSS variables on the page root, refactored glass-theme overrides to set glass-specific variables (including xl breakpoint adjustments), removed prior inlined glass rules, and changed a global sidebar overflow selector to #page-sidebar.

Changes

Cohort / File(s) Summary
Page component SCSS
src/patternfly/components/Page/page.scss
Added __sidebar--* and __sidebar-main--* CSS variables on the page root and updated .pf-v6__sidebar / .pf-v6__sidebar-main to use them; refactored glass-theme handling to assign --...--glass variables (including xl adjustments); removed previous .pf-v6-theme-glass inlined overrides; introduced dedicated __main-section--m-secondary--* border variables and updated .pf-v6__main-section.pf-m-secondary.
Global stylesheet
src/patternfly/patternfly.scss
Replaced .pf-v6-c-page__sidebar { overflow: auto } selector with #page-sidebar { overflow: auto } and added stylelint suppression comments around the new ID selector.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

Suggested labels

released on @prerelease``

Suggested reviewers

  • lboehling
  • jcmill
  • srambach
🚥 Pre-merge checks | ✅ 4
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title follows conventional commit format with 'fix' prefix and clearly describes the main change: cleaning up glass/sidebar-main variables in the page component.
Linked Issues check ✅ Passed The PR successfully converts sidebar-main glass styles to reference dedicated sidebar-main variables and moves global token references into component variable lists, fully addressing issue #8273 objectives.
Out of Scope Changes check ✅ Passed All changes are scoped to the page component SCSS files and directly address the variable refactoring objectives; the ID selector change in patternfly.scss is a necessary technical adjustment supporting the main refactoring goals.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.

✏️ 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

patternfly-build commented Apr 3, 2026

Copy link
Copy Markdown
Collaborator

@coderabbitai coderabbitai Bot 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.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/patternfly/components/Page/page.scss`:
- Around line 253-278: The xl breakpoint margin overrides for sidebar-main (the
`@media` (min-width: $pf-v6-global--breakpoint--xl) block setting
--#{$page}__sidebar-main--MarginBlockStart / MarginBlockEnd / MarginInlineStart
/ MarginInlineEnd) are currently global and should only apply to the glass
theme; move this entire `@media` block inside the :where(.pf-v6-theme-glass) & {
... } scope (or alternatively change those four vars to their --glass variants)
so non-glass themes do not inherit the page-chrome inset margins.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

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

Review profile: CHILL

Plan: Pro

Run ID: a343b303-d32a-42a0-9d49-d5f3ffe4da73

📥 Commits

Reviewing files that changed from the base of the PR and between 483890b and 6c0be8a.

📒 Files selected for processing (2)
  • src/patternfly/components/Page/page.scss
  • src/patternfly/patternfly.scss

Comment thread src/patternfly/components/Page/page.scss
@mcoker mcoker requested a review from jcmill April 4, 2026 15:37

@jcmill jcmill 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.

🧑‍🍳 💋

@mcoker mcoker merged commit 932d43d into patternfly:main Apr 6, 2026
5 checks passed
@mcoker mcoker deleted the issue-8273 branch April 6, 2026 14:38
@patternfly-build

Copy link
Copy Markdown
Collaborator

🎉 This PR is included in version 6.5.0-prerelease.65 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Bug - Page - move token references to main list of component vars Bug - Page sidebar - collapsed state still shows sidebar

3 participants