Skip to content

fix(drawer): glass drawer placement and tokens update#8286

Merged
mcoker merged 5 commits into
patternfly:mainfrom
jcmill:fix/3748-drawer-glass
Apr 20, 2026
Merged

fix(drawer): glass drawer placement and tokens update#8286
mcoker merged 5 commits into
patternfly:mainfrom
jcmill:fix/3748-drawer-glass

Conversation

@jcmill

@jcmill jcmill commented Apr 6, 2026

Copy link
Copy Markdown
Contributor

Closes #8234

Drawer
Secondary panel: background uses background--color--floating--secondary--default instead of secondary--default.
Pill variant: border color uses border--color--subtle. Expanded pill panel gets box-shadow--md for clearer separation.

Page / masthead (glass)
Removes masthead margin-block-end in the glass theme so the band no longer owns that gap.
Adds matching top margin on the sidebar and main container via new glass tokens (MarginBlockStart = page chrome inset), so spacing stays consistent while living on the page chrome instead of the masthead.

Summary by CodeRabbit

  • Style

    • Updated Drawer secondary panel theming and pill styling with subtler borders and an enhanced shadow for expanded pill panels.
    • Removed a bottom margin in the glass-themed, non-docked Masthead variant.
    • Added glass-theme margin overrides for Page sidebar and main container spacing.
  • Documentation

    • Added a "Resizable Pill" Drawer example demonstrating a resizable pill panel variant.

@jcmill jcmill requested a review from mcoker April 6, 2026 23:53
@patternfly-build

patternfly-build commented Apr 6, 2026

Copy link
Copy Markdown
Collaborator

@jcmill jcmill requested review from kaylachumley and lboehling April 7, 2026 00:09
@kaylachumley

Copy link
Copy Markdown

The pill variant (inline and overlay) should have the option to use a splitter as well! Thinking it may be good to add that since its styled slightly different then the splitter in a standard sqaured off drawer

@kaylachumley kaylachumley left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

left a comment about the splitter with pill drawers!

@lboehling lboehling left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

lgtm!

@coderabbitai

coderabbitai Bot commented Apr 20, 2026

Copy link
Copy Markdown
Contributor

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

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

Review profile: CHILL

Plan: Pro

Run ID: 5d485726-cd91-4af2-a4ee-15a190c70d29

📥 Commits

Reviewing files that changed from the base of the PR and between e468f27 and d76120f.

📒 Files selected for processing (1)
  • src/patternfly/components/Drawer/examples/Drawer.md
✅ Files skipped from review due to trivial changes (1)
  • src/patternfly/components/Drawer/examples/Drawer.md

Walkthrough

Retargeted Drawer panel/border tokens, added an expanded-pill box-shadow variable and a resizable-pill Drawer example, removed a glass-theme masthead bottom margin, and introduced glass-specific page sidebar/main margin variables. (27 words)

Changes

Cohort / File(s) Summary
Drawer styles & example
src/patternfly/components/Drawer/drawer.scss, src/patternfly/components/Drawer/examples/Drawer.md
Retargeted panel background to --pf-t--global--background--color--floating--secondary--default; changed pill panel border to --pf-t--global--border--color--subtle; added --#{$drawer}--m-pill--m-expanded__panel--BoxShadow mapped to --pf-t--global--box-shadow--md and applied it to .pf-m-expanded. Added "Resizable Pill" example with drawer-panel--IsResizable=true.
Masthead glass-theme tweak
src/patternfly/components/Masthead/masthead.scss
Removed margin-block-end from the glass-theme non-docked masthead placeholder selector.
Page glass-theme margins
src/patternfly/components/Page/page.scss
Added --#{$page}__sidebar--MarginBlockStart--glass and --#{$page}__main-container--MarginBlockStart--glass and mapped them into the .pf-v6-theme-glass scope.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

Suggested labels

released on @prerelease``

Suggested reviewers

  • kaylachumley
  • srambach
  • mcoker
🚥 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 follows conventional commit guidelines with 'fix' type prefix and clear description of drawer component changes.
Linked Issues check ✅ Passed All code changes implement requirements from issue #8234: secondary background token update, pill border color update, expanded pill box-shadow addition, and glass theme masthead/page margin adjustments.
Out of Scope Changes check ✅ Passed All changes align with issue #8234 requirements. The Drawer token updates, Masthead margin removal, and Page glass margin additions are all directly scoped to the linked issue objectives.

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

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

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)
src/patternfly/components/Page/page.scss (1)

259-278: ⚠️ Potential issue | 🟠 Major

Fix the glass theme selector so Stylelint passes.

Stylelint is flagging Line 259 for nesting-selector-no-missing-scoping-root. Use the existing theme-scoped @at-root pattern here before wiring the new variables.

Proposed fix
-  :where(.pf-v6-theme-glass) & {
+  `@at-root` :where(.pf-v6-theme-glass) & {
     --#{$page}--BackgroundColor: var(--#{$page}--BackgroundColor--glass);
     --#{$page}__sidebar--Width--base: var(--#{$page}__sidebar--Width--base--glass);
     --#{$page}__sidebar--MarginBlockStart: var(--#{$page}__sidebar--MarginBlockStart--glass);
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/patternfly/components/Page/page.scss` around lines 259 - 278, Stylelint
complains because the glass theme block uses a nested selector without the
repository's theme-scoped `@at-root` pattern; wrap the existing
:where(.pf-v6-theme-glass) & block in the same `@at-root` theme-scoping used
elsewhere so the selector becomes theme-scoped at root before you set the
variables (the block that wires variables like --#{$page}--BackgroundColor,
--#{$page}__sidebar--Width--base, --#{$page}__sidebar-main--BackgroundColor,
--#{$page}__sidebar-main--BackdropFilter,
--#{$page}__main-container--MarginBlockStart, etc.). Locate the
:where(.pf-v6-theme-glass) & block in page.scss and apply the project’s `@at-root`
.pf-v6-theme-glass & pattern around it so Stylelint no longer flags
nesting-selector-no-missing-scoping-root.
src/patternfly/components/Drawer/drawer.scss (1)

354-359: ⚠️ Potential issue | 🟡 Minor

Route the pill shadow through the existing shadow variable.

Line 359 applies box-shadow directly, which bypasses later resets for inline/static drawers and .pf-m-no-border panels. Set --#{$drawer}--m-expanded__panel--BoxShadow instead so existing cascade rules can still override it.

🎨 Proposed fix
       > .#{$drawer}__main > .#{$drawer}__panel {
+        --#{$drawer}--m-expanded__panel--BoxShadow: var(--#{$drawer}--m-pill--m-expanded__panel--BoxShadow);
+
         border-block-start-width: var(--#{$drawer}--m-pill__panel--BorderBlockStartWidth);
         border-block-end-width: var(--#{$drawer}--m-pill__panel--BorderBlockEndWidth);
         border-inline-start-width: var(--#{$drawer}--m-pill__panel--BorderInlineStartWidth);
         border-inline-end-width: var(--#{$drawer}--m-pill__panel--BorderInlineEndWidth);
-        box-shadow: var(--#{$drawer}--m-pill--m-expanded__panel--BoxShadow);
       }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/patternfly/components/Drawer/drawer.scss` around lines 354 - 359, The
selector .#{$drawer}__main > .#{$drawer}__panel is currently setting box-shadow
directly; change it to assign the expanded-panel shadow to the cascadeable CSS
variable instead (set --#{$drawer}--m-expanded__panel--BoxShadow using the
pill-expanded value rather than writing box-shadow), so later rules
(inline/static drawers and .pf-m-no-border) can override it and the cascade
remains intact.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Outside diff comments:
In `@src/patternfly/components/Drawer/drawer.scss`:
- Around line 354-359: The selector .#{$drawer}__main > .#{$drawer}__panel is
currently setting box-shadow directly; change it to assign the expanded-panel
shadow to the cascadeable CSS variable instead (set
--#{$drawer}--m-expanded__panel--BoxShadow using the pill-expanded value rather
than writing box-shadow), so later rules (inline/static drawers and
.pf-m-no-border) can override it and the cascade remains intact.

In `@src/patternfly/components/Page/page.scss`:
- Around line 259-278: Stylelint complains because the glass theme block uses a
nested selector without the repository's theme-scoped `@at-root` pattern; wrap the
existing :where(.pf-v6-theme-glass) & block in the same `@at-root` theme-scoping
used elsewhere so the selector becomes theme-scoped at root before you set the
variables (the block that wires variables like --#{$page}--BackgroundColor,
--#{$page}__sidebar--Width--base, --#{$page}__sidebar-main--BackgroundColor,
--#{$page}__sidebar-main--BackdropFilter,
--#{$page}__main-container--MarginBlockStart, etc.). Locate the
:where(.pf-v6-theme-glass) & block in page.scss and apply the project’s `@at-root`
.pf-v6-theme-glass & pattern around it so Stylelint no longer flags
nesting-selector-no-missing-scoping-root.

ℹ️ Review info
⚙️ Run configuration

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

Review profile: CHILL

Plan: Pro

Run ID: eee80095-ce9d-423f-a2e1-1c04e67bea9e

📥 Commits

Reviewing files that changed from the base of the PR and between 4e48a9c and a91c3e6.

📒 Files selected for processing (4)
  • src/patternfly/components/Drawer/drawer.scss
  • src/patternfly/components/Drawer/examples/Drawer.md
  • src/patternfly/components/Masthead/masthead.scss
  • src/patternfly/components/Page/page.scss
💤 Files with no reviewable changes (1)
  • src/patternfly/components/Masthead/masthead.scss

@jcmill jcmill requested a review from kaylachumley April 20, 2026 13:55

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

Just a small docs nit to add a unique ID

Comment thread src/patternfly/components/Drawer/examples/Drawer.md Outdated
Co-authored-by: Michael Coker <35148959+mcoker@users.noreply.github.com>
@mcoker mcoker merged commit ea00942 into patternfly:main Apr 20, 2026
5 checks passed
@patternfly-build

Copy link
Copy Markdown
Collaborator

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

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.

Drawer - Glass PR Design Follow-ups & Fixes

7 participants