fix(drawer): prevent pill inline drawer from extending beyond viewport - #8386
Conversation
…usted main padding for pill shadow
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI (base), Organization UI (inherited) Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
WalkthroughSet drawer pill-inline main gap default to 0 and add an override in ChangesPill Inline Drawer Gap
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes 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-8386.surge.sh A11y report: https://pf-pr-8386-a11y.surge.sh |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/patternfly/components/Drawer/drawer.scss (1)
9-11: 💤 Low valueRename
--PaddingBlockto--PaddingBlockEndfor accuracy.The variable
--#{$drawer}--m-pill--m-inline__main--PaddingBlockimplies a shorthand covering bothpadding-block-startandpadding-block-end, but it is only ever consumed bypadding-block-end(line 368). The name will mislead future maintainers into thinking block-start is also covered.🔧 Suggested rename
- --#{$drawer}--m-pill--m-inline__main--PaddingBlock: 0; + --#{$drawer}--m-pill--m-inline__main--PaddingBlockEnd: 0;- --#{$drawer}--m-pill--m-inline__main--PaddingBlock: var(--pf-t--global--spacer--inset--page-chrome); + --#{$drawer}--m-pill--m-inline__main--PaddingBlockEnd: var(--pf-t--global--spacer--inset--page-chrome);- padding-block-end: var(--#{$drawer}--m-pill--m-inline__main--PaddingBlock); + padding-block-end: var(--#{$drawer}--m-pill--m-inline__main--PaddingBlockEnd);Also applies to: 298-299, 368-368
🤖 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/Drawer/drawer.scss` around lines 9 - 11, The CSS custom property --#{$drawer}--m-pill--m-inline__main--PaddingBlock is misleading because it's only used for padding-block-end; rename the declaration to --#{$drawer}--m-pill--m-inline__main--PaddingBlockEnd and update all usages (notably the consumer in the padding-block-end rule and the other declaration/uses in the same SCSS around the other occurrences) to the new name so that intent is accurate and no references remain to the old --PaddingBlock variable.
🤖 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.
Nitpick comments:
In `@src/patternfly/components/Drawer/drawer.scss`:
- Around line 9-11: The CSS custom property
--#{$drawer}--m-pill--m-inline__main--PaddingBlock is misleading because it's
only used for padding-block-end; rename the declaration to
--#{$drawer}--m-pill--m-inline__main--PaddingBlockEnd and update all usages
(notably the consumer in the padding-block-end rule and the other
declaration/uses in the same SCSS around the other occurrences) to the new name
so that intent is accurate and no references remain to the old --PaddingBlock
variable.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro
Run ID: cfa3971b-49c2-4c2a-bd5a-662eb1af8d39
📒 Files selected for processing (1)
src/patternfly/components/Drawer/drawer.scss
|
🎉 This PR is included in version 6.5.0-prerelease.90 🎉 The release is available on: Your semantic-release bot 📦🚀 |
Fixes #8351
The pill inline drawer was extending below the browser window due to gap spacing on __main being added to the 100% container height. Defaulted gap to 0 and managed it in expanded. Use padding-block-end to allow for pill shadows instead of gap for pill + inline variants, keeping the inset spacing internal to the container height.
Summary by CodeRabbit