fix(compass): box-shadow cut off#8366
Conversation
|
Preview: https://pf-pr-8366.surge.sh A11y report: https://pf-pr-8366-a11y.surge.sh |
|
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:
WalkthroughAdds glass-theme CSS custom properties for Compass content margin and padding, maps those --*--glass values into element-level Compass variables when .pf-v6-theme-glass is active, updates .compass__content to consume those variables with 0 fallbacks, and enables scrollable/auto-height flags in Compass demos. ChangesCompass Glass Theme Spacing
Sequence Diagram(s)(omitted — changes are CSS and demo wiring without a multi-component sequential control flow) Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested reviewers
Suggested labels
🚥 Pre-merge checks | ✅ 3 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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.
Inline comments:
In `@src/patternfly/components/Compass/compass.scss`:
- Around line 400-407: The stylelint properties order is wrong in the Compass
component block: move block-axis properties before inline-axis properties to
satisfy order/properties-order; specifically reorder the rules that use the
--#{$compass}__content--* variables so the sequence is margin-block-start,
margin-block-end, padding-block-start, padding-block-end, then
margin-inline-start, margin-inline-end, padding-inline-start, padding-inline-end
(update the block in compass.scss where these CSS custom properties are
declared).
🪄 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: 9cfa962b-6c8f-4680-bef9-8dbf13b43abf
📒 Files selected for processing (1)
src/patternfly/components/Compass/compass.scss
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/patternfly/components/Compass/compass.scss (1)
400-407:⚠️ Potential issue | 🟠 Major | ⚡ Quick winReorder
__contentmargin/padding declarations to satisfy stylelint.This block still fails CI (
order/properties-order) at Line 402 and Line 406. Please reorder the declarations sopadding-block-startcomes beforemargin-block-end, andpadding-inline-startcomes beforemargin-inline-end.✅ Suggested reorder
margin-block-start: var(--#{$compass}__content--MarginBlockStart, 0); - margin-block-end: var(--#{$compass}__content--MarginBlockEnd, 0); padding-block-start: var(--#{$compass}__content--PaddingBlockStart, 0); + margin-block-end: var(--#{$compass}__content--MarginBlockEnd, 0); padding-block-end: var(--#{$compass}__content--PaddingBlockEnd, 0); margin-inline-start: var(--#{$compass}__content--MarginInlineStart, 0); - margin-inline-end: var(--#{$compass}__content--MarginInlineEnd, 0); padding-inline-start: var(--#{$compass}__content--PaddingInlineStart, 0); + margin-inline-end: var(--#{$compass}__content--MarginInlineEnd, 0); padding-inline-end: var(--#{$compass}__content--PaddingInlineEnd, 0);🤖 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/Compass/compass.scss` around lines 400 - 407, The stylelint ordering error in the Compass __content block is caused by padding properties placed after their corresponding margin properties; in the CSS rule that uses the --#{$compass}__content custom properties, move padding-block-start so it appears before margin-block-end, and move padding-inline-start so it appears before margin-inline-end, preserving the existing var(...) values for padding-block-start, padding-block-end, padding-inline-start, and padding-inline-end and keeping margin-block-start/margin-inline-start where they are; update the declarations order in the block that contains margin-block-start, margin-block-end, padding-block-start, padding-block-end, margin-inline-start, margin-inline-end, padding-inline-start, padding-inline-end to satisfy the order/properties-order rule.
🤖 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.
Inline comments:
In `@src/patternfly/components/Compass/compass.scss`:
- Around line 93-102: The glass theme selector currently uses
":where(.pf-v6-theme-glass) &" which triggers the
nesting-selector-no-missing-scoping-root lint; update that selector to
":root:where(.pf-v6-theme-glass) &" where the CSS custom properties for the
Compass component (the --#{$compass}__content--* variables block) are defined so
the theme scoping matches the pattern used elsewhere and satisfies the linter.
---
Duplicate comments:
In `@src/patternfly/components/Compass/compass.scss`:
- Around line 400-407: The stylelint ordering error in the Compass __content
block is caused by padding properties placed after their corresponding margin
properties; in the CSS rule that uses the --#{$compass}__content custom
properties, move padding-block-start so it appears before margin-block-end, and
move padding-inline-start so it appears before margin-inline-end, preserving the
existing var(...) values for padding-block-start, padding-block-end,
padding-inline-start, and padding-inline-end and keeping
margin-block-start/margin-inline-start where they are; update the declarations
order in the block that contains margin-block-start, margin-block-end,
padding-block-start, padding-block-end, margin-inline-start, margin-inline-end,
padding-inline-start, padding-inline-end to satisfy the order/properties-order
rule.
🪄 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: be2b23ce-2544-4686-b1b3-6765cee31491
📒 Files selected for processing (1)
src/patternfly/components/Compass/compass.scss
There was a problem hiding this comment.
♻️ Duplicate comments (1)
src/patternfly/components/Compass/compass.scss (1)
93-102:⚠️ Potential issue | 🟠 Major | ⚡ Quick win
nesting-selector-no-missing-scoping-rootlint error still present — add:rootprefix.The selector on line 93 is
:where(.pf-v6-theme-glass) &, which stylelint flags for the missing scoping root. This is inconsistent with the dark-theme selector in the same file (line 143::root:where(.#{$pf-prefix}theme-dark) &).💡 Proposed fix
- :where(.pf-v6-theme-glass) & { + :root:where(.pf-v6-theme-glass) & {🤖 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/Compass/compass.scss` around lines 93 - 102, The selector ":where(.pf-v6-theme-glass) &" triggers the nesting-selector-no-missing-scoping-root lint rule; update the rule-violating selector to include the :root scoping prefix (matching the dark-theme pattern used elsewhere) so it becomes ":root:where(.pf-v6-theme-glass) &" (ensure you modify the selector that wraps the CSS custom properties for the $compass component rather than the property names themselves).
🤖 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/Compass/compass.scss`:
- Around line 93-102: The selector ":where(.pf-v6-theme-glass) &" triggers the
nesting-selector-no-missing-scoping-root lint rule; update the rule-violating
selector to include the :root scoping prefix (matching the dark-theme pattern
used elsewhere) so it becomes ":root:where(.pf-v6-theme-glass) &" (ensure you
modify the selector that wraps the CSS custom properties for the $compass
component rather than the property names themselves).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro
Run ID: 53d85141-c783-4ff4-8618-3e0db8a0363b
📒 Files selected for processing (1)
src/patternfly/components/Compass/compass.scss
jcmill
changed the title
|
🎉 This PR is included in version 6.5.0-prerelease.91 🎉 The release is available on: Your semantic-release bot 📦🚀 |
4 participants
Closes: #8344
Fixes clipped box-shadow in Compass component when using glass theme by adding padding and compensating negative margins on __content element.
Summary by CodeRabbit
New Features
Style
Chores / UX