feat(Toolbar): add dynamic sticky styles - #8321
Merged
Merged
Conversation
Contributor
|
Note Reviews pausedIt 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 Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughIntroduces dynamic sticky toolbar modifiers and sticky-specific tokens, adds a stuck-state pseudo-element visual, broadens a preview CSS selector, and adds a "Dynamic sticky toolbar" documentation example demonstrating the stuck behavior. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 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 |
Collaborator
|
Preview: https://pf-pr-8321.surge.sh A11y report: https://pf-pr-8321-a11y.surge.sh |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/patternfly/components/Toolbar/examples/Toolbar.md (1)
621-624: Document the new modifiers in the Usage table.The "Usage" table (around line 787) lists
.pf-m-stickybut not the newly introduced.pf-m-sticky-baseand.pf-m-sticky-stuck. Consider adding rows for both so consumers can discover them alongside the other toolbar modifiers.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/patternfly/components/Toolbar/examples/Toolbar.md` around lines 621 - 624, Add documentation rows for the two new toolbar modifiers to the existing "Usage" table in Toolbar.md: create separate entries describing `.pf-m-sticky-base` and `.pf-m-sticky-stuck` (similar format to the existing `.pf-m-sticky` row), briefly stating their purpose and when to use them so consumers can discover them alongside other toolbar modifiers; update any column values (example, description, and usage notes) to match the style of surrounding rows in the Usage table.
🤖 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/Toolbar/examples/Toolbar.css`:
- Around line 78-81: The selector list currently applies height/overflow to
`#ws-core-c-toolbar-sticky-toolbar` itself instead of its .ws-preview-html child
due to missing .ws-preview-html on the first selector; update the rule so both
selectors target their .ws-preview-html descendants (i.e., change the selector
that references `#ws-core-c-toolbar-sticky-toolbar` to include the
.ws-preview-html class just like `#ws-core-c-toolbar-dynamic-sticky-toolbar`
.ws-preview-html) so that the CSS rule for height: 200px; overflow: auto;
applies symmetrically to the .ws-preview-html elements in both Toolbar examples.
---
Nitpick comments:
In `@src/patternfly/components/Toolbar/examples/Toolbar.md`:
- Around line 621-624: Add documentation rows for the two new toolbar modifiers
to the existing "Usage" table in Toolbar.md: create separate entries describing
`.pf-m-sticky-base` and `.pf-m-sticky-stuck` (similar format to the existing
`.pf-m-sticky` row), briefly stating their purpose and when to use them so
consumers can discover them alongside other toolbar modifiers; update any column
values (example, description, and usage notes) to match the style of surrounding
rows in the Usage table.
🪄 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: 762a455f-e036-4585-b7f5-089bf0d043da
📒 Files selected for processing (3)
src/patternfly/components/Toolbar/examples/Toolbar.csssrc/patternfly/components/Toolbar/examples/Toolbar.mdsrc/patternfly/components/Toolbar/toolbar.scss
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/patternfly/components/Toolbar/toolbar.scss (1)
182-184: Nit: stray trailing whitespace and extra blank line.Line 183 has trailing whitespace after
border-radius: …;and is followed by another empty line (184) before the&::beforeblock.🧹 Proposed cleanup
border-radius: var(--#{$toolbar}--m-sticky--BorderRadius); - - + &::before {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/patternfly/components/Toolbar/toolbar.scss` around lines 182 - 184, Remove the stray trailing whitespace after the declaration "border-radius: var(--#{$toolbar}--m-sticky--BorderRadius);" and delete the extra blank line before the subsequent "&::before" block in toolbar.scss so the declaration line ends cleanly and the following pseudo-element block immediately follows; this fixes the nit about trailing whitespace and the unnecessary blank line.
🤖 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/Toolbar/toolbar.scss`:
- Around line 185-190: The ::before pseudo-element rule under the toolbar
currently lacks content and positioning so it never renders; either remove that
::before block and its TransitionDuration/TransitionTimingFunction tokens if you
intend the toolbar background swap to happen on the toolbar itself (see the
.pf-m-sticky-stuck background-color override), or flesh out the pseudo so it can
act as the fading layer: add content: ""; position: absolute; inset: 0 (or
explicit top/right/bottom/left), and set a background-color (driven by the
--#{$toolbar}--BackgroundColor token) so the opacity transition
(transition-property: opacity) actually affects a visible layer referenced by
.pf-m-sticky &::before and .pf-m-sticky-stuck &::before; keep reference to the
toolbar root selector (.#{$toolbar}) which already provides position: relative
so the absolute pseudo is contained.
---
Nitpick comments:
In `@src/patternfly/components/Toolbar/toolbar.scss`:
- Around line 182-184: Remove the stray trailing whitespace after the
declaration "border-radius: var(--#{$toolbar}--m-sticky--BorderRadius);" and
delete the extra blank line before the subsequent "&::before" block in
toolbar.scss so the declaration line ends cleanly and the following
pseudo-element block immediately follows; this fixes the nit about trailing
whitespace and the unnecessary blank line.
🪄 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: eadcda04-38df-425c-9f8d-dff4087687b8
📒 Files selected for processing (1)
src/patternfly/components/Toolbar/toolbar.scss
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/patternfly/components/Toolbar/toolbar.scss (1)
184-200:⚠️ Potential issue | 🟡 MinorMake the fade layer render, or remove it.
The
::beforerule still cannot render because it has nocontent, and the toolbar background is swapped directly on.pf-m-sticky-stuck, so the opacity transition is effectively dead code. Either complete the pseudo-element layer or drop the pseudo/transition tokens.Proposed fix if the fade layer is intentional
&::before { + position: absolute; + inset: 0; + z-index: -1; + content: ""; + background-color: var(--#{$toolbar}--m-sticky--BackgroundColor); + border-radius: inherit; opacity: 0; transition-timing-function: var(--#{$toolbar}--m-sticky--TransitionTimingFunction--BackgroundColor); transition-duration: var(--#{$toolbar}--m-sticky--TransitionDuration--BackgroundColor); transition-property: opacity; } @@ &.pf-m-sticky-stuck { - --#{$toolbar}--BackgroundColor: var(--#{$toolbar}--m-sticky--BackgroundColor); - border-block-end: var(--#{$toolbar}--m-sticky--BorderBlockEndWidth) solid var(--#{$toolbar}--m-sticky--BorderBlockEndColor); box-shadow: var(--#{$toolbar}--m-sticky--BoxShadow);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/patternfly/components/Toolbar/toolbar.scss` around lines 184 - 200, The ::before pseudo-element in the toolbar SCSS never renders because it lacks content and positioning, while .pf-m-sticky-stuck immediately swaps the toolbar background so the opacity transition is unused; either implement the fade layer by adding content, positioning, size, and background variables to &::before (ensure it sits under toolbar content and uses the --#{$toolbar}--m-sticky--BackgroundColor token and the existing transition properties) and keep the .pf-m-sticky-stuck background change only as a fallback, or remove the &::before rules and the related transition tokens (transition-timing-function, transition-duration, transition-property and opacity changes) from the .pf-m-sticky and .pf-m-sticky-stuck blocks to eliminate dead code.
🤖 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/Toolbar/toolbar.scss`:
- Around line 81-87: The glass-theme overrides are nested with
":where(.pf-v6-theme-glass) &" which triggers the Stylelint
nesting-selector-no-missing-scoping-root error; instead, move these variable
overrides under an explicit toolbar scope by creating a top-level .#{$toolbar}
selector and placing the :where(.pf-v6-theme-glass) variant inside it so the
same variables (e.g. --#{$toolbar}--m-sticky--BorderBlockEndWidth,
--#{$toolbar}--m-sticky--BoxShadow, --#{$toolbar}--m-sticky--BorderRadius,
--#{$toolbar}--m-sticky--PaddingInlineStart,
--#{$toolbar}--m-sticky--PaddingInlineEnd) are set but now scoped via the
.#{$toolbar} rule rather than nesting the ampersand under pf-root.
---
Duplicate comments:
In `@src/patternfly/components/Toolbar/toolbar.scss`:
- Around line 184-200: The ::before pseudo-element in the toolbar SCSS never
renders because it lacks content and positioning, while .pf-m-sticky-stuck
immediately swaps the toolbar background so the opacity transition is unused;
either implement the fade layer by adding content, positioning, size, and
background variables to &::before (ensure it sits under toolbar content and uses
the --#{$toolbar}--m-sticky--BackgroundColor token and the existing transition
properties) and keep the .pf-m-sticky-stuck background change only as a
fallback, or remove the &::before rules and the related transition tokens
(transition-timing-function, transition-duration, transition-property and
opacity changes) from the .pf-m-sticky and .pf-m-sticky-stuck blocks to
eliminate dead code.
🪄 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: 643e953a-a4f7-4796-8120-4d89a8928e2f
📒 Files selected for processing (1)
src/patternfly/components/Toolbar/toolbar.scss
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (2)
src/patternfly/components/Toolbar/toolbar.scss (2)
183-202:⚠️ Potential issue | 🟠 MajorAdd
contentand an actual background to the sticky pseudo-element.The
::beforelayer still will not render withoutcontent, and Line 184 only sets a custom property; it never paints the sticky background.Proposed fix
&::before { - --#{$toolbar}--BackgroundColor: var(--#{$toolbar}--m-sticky--BackgroundColor); - position: absolute; inset: 0; z-index: -1; + content: ""; + background-color: var(--#{$toolbar}--m-sticky--BackgroundColor); + border-radius: inherit; border-block-end: var(--#{$toolbar}--m-sticky--BorderBlockEndWidth) solid var(--#{$toolbar}--m-sticky--BorderBlockEndColor); box-shadow: var(--#{$toolbar}--m-sticky--BoxShadow); opacity: 0;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/patternfly/components/Toolbar/toolbar.scss` around lines 183 - 202, The ::before pseudo-element for the toolbar is missing content and never paints the background — update the &::before rule (inside the toolbar block) to include content: "" and set its background to the custom property (background: var(--#{$toolbar}--BackgroundColor)); keep the existing positioning/opacity/transition rules so the sticky layer can render and animate, and leave the .pf-m-sticky-stuck &::before opacity change as-is.
81-87:⚠️ Potential issue | 🟠 MajorMove the glass override under an explicit toolbar scope.
Line 81 still triggers Stylelint’s
nesting-selector-no-missing-scoping-root, so this will block the PR until the selector is scoped through.#{$toolbar}.Proposed fix
- :where(.pf-v6-theme-glass) & { - --#{$toolbar}--m-sticky--BorderBlockEndWidth: 0; - --#{$toolbar}--m-sticky--BoxShadow: var(--#{$toolbar}--m-sticky--BoxShadow--glass); - --#{$toolbar}--m-sticky--BorderRadius: var(--#{$toolbar}--m-sticky--BorderRadius--glass); - --#{$toolbar}--m-sticky--PaddingInlineStart: var(--#{$toolbar}--m-sticky--PaddingInlineStart--glass); - --#{$toolbar}--m-sticky--PaddingInlineEnd: var(--#{$toolbar}--m-sticky--PaddingInlineEnd--glass); - } } + +:where(.pf-v6-theme-glass) { + .#{$toolbar} { + --#{$toolbar}--m-sticky--BorderBlockEndWidth: 0; + --#{$toolbar}--m-sticky--BoxShadow: var(--#{$toolbar}--m-sticky--BoxShadow--glass); + --#{$toolbar}--m-sticky--BorderRadius: var(--#{$toolbar}--m-sticky--BorderRadius--glass); + --#{$toolbar}--m-sticky--PaddingInlineStart: var(--#{$toolbar}--m-sticky--PaddingInlineStart--glass); + --#{$toolbar}--m-sticky--PaddingInlineEnd: var(--#{$toolbar}--m-sticky--PaddingInlineEnd--glass); + } +} // - Toolbar content section - Toolbar group - Toolbar item🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/patternfly/components/Toolbar/toolbar.scss` around lines 81 - 87, The glass-theme override block is missing an explicit toolbar scoping root and triggers Stylelint's nesting-selector-no-missing-scoping-root; update the selector so the variables are defined under the toolbar scope (use .#{$toolbar} as the scoping root) and move or rewrite the current ":where(.pf-v6-theme-glass) & { ... }" block to be scoped like .#{$toolbar}:where(.pf-v6-theme-glass) { --#{$toolbar}--m-sticky--BorderBlockEndWidth: 0; --#{$toolbar}--m-sticky--BoxShadow: var(--#{$toolbar}--m-sticky--BoxShadow--glass); --#{$toolbar}--m-sticky--BorderRadius: var(--#{$toolbar}--m-sticky--BorderRadius--glass); --#{$toolbar}--m-sticky--PaddingInlineStart: var(--#{$toolbar}--m-sticky--PaddingInlineStart--glass); --#{$toolbar}--m-sticky--PaddingInlineEnd: var(--#{$toolbar}--m-sticky--PaddingInlineEnd--glass); } so that $toolbar is the explicit scoping root and Stylelint no longer complains.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@src/patternfly/components/Toolbar/toolbar.scss`:
- Around line 183-202: The ::before pseudo-element for the toolbar is missing
content and never paints the background — update the &::before rule (inside the
toolbar block) to include content: "" and set its background to the custom
property (background: var(--#{$toolbar}--BackgroundColor)); keep the existing
positioning/opacity/transition rules so the sticky layer can render and animate,
and leave the .pf-m-sticky-stuck &::before opacity change as-is.
- Around line 81-87: The glass-theme override block is missing an explicit
toolbar scoping root and triggers Stylelint's
nesting-selector-no-missing-scoping-root; update the selector so the variables
are defined under the toolbar scope (use .#{$toolbar} as the scoping root) and
move or rewrite the current ":where(.pf-v6-theme-glass) & { ... }" block to be
scoped like .#{$toolbar}:where(.pf-v6-theme-glass) {
--#{$toolbar}--m-sticky--BorderBlockEndWidth: 0;
--#{$toolbar}--m-sticky--BoxShadow:
var(--#{$toolbar}--m-sticky--BoxShadow--glass);
--#{$toolbar}--m-sticky--BorderRadius:
var(--#{$toolbar}--m-sticky--BorderRadius--glass);
--#{$toolbar}--m-sticky--PaddingInlineStart:
var(--#{$toolbar}--m-sticky--PaddingInlineStart--glass);
--#{$toolbar}--m-sticky--PaddingInlineEnd:
var(--#{$toolbar}--m-sticky--PaddingInlineEnd--glass); } so that $toolbar is the
explicit scoping root and Stylelint no longer complains.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro
Run ID: 08c6afc0-77e7-47f6-8a16-91eb646ae885
📒 Files selected for processing (1)
src/patternfly/components/Toolbar/toolbar.scss
Collaborator
|
🎉 This PR is included in version 6.5.0-prerelease.77 🎉 The release is available on: Your semantic-release bot 📦🚀 |
This was referenced May 5, 2026
This was referenced Jul 22, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Adds
pf-m-sticky-baseandpf-m-sticky-stuckfor Toolbar.Summary by CodeRabbit
New Features
Style
Documentation