fix(page): clean up glass/sidebar-main vars#8274
Conversation
WalkthroughMoved 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 Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ 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. Comment |
|
Preview: https://pf-pr-8274.surge.sh A11y report: https://pf-pr-8274-a11y.surge.sh |
There was a problem hiding this comment.
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
📒 Files selected for processing (2)
src/patternfly/components/Page/page.scsssrc/patternfly/patternfly.scss
|
🎉 This PR is included in version 6.5.0-prerelease.65 🎉 The release is available on: Your semantic-release bot 📦🚀 |
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-mainis a new element and is not required unless you're using glass theme.Summary by CodeRabbit
Style
Refactor