ROX-31777: Fix PF6 spacing in compliance schedules#19215
ROX-31777: Fix PF6 spacing in compliance schedules#19215sachaudh merged 4 commits intodv/ROX-28622-pf-6from
Conversation
PF6 migration introduced several spacing inconsistencies in the
compliance schedules pages:
- Toolbar bottom padding created gaps between pagination and tables
- PageSection RowGap (24px default) added unintended space between
title rows and alerts when using hasBodyWrapper={false}
- Wizard pages didn't fill available height (isFilled)
- ReviewConfig alert lacked bottom margin before wizard footer
- NotifierConfigurationForm used semantic List for interactive form
elements; replaced with Flex layout with proper gap spacing
Added a scoped pf-v6-u-row-gap-0 utility class in trumps.css to
override the PageSection gap where needed, rather than applying
globally to avoid side effects on other pages.
Partially generated by AI.
Signed-off-by: Saif Chaudhry <schaudhr@redhat.com>
|
Skipping CI for Draft Pull Request. |
|
The number of tweaks using classes/padding/margin solidifies my stance that we really should invest in a layout component for our pages and sections. I think it would definitely help with cases like this. The PF v6 Upgrade highlights that well. |
Apply row-gap-0 to PageSection instead of removing Toolbar padding, since the extra spacing comes from PF6's PageSection row-gap between direct children. Add aria-label to delivery destinations Flex for accessibility. AI-assisted change. Signed-off-by: Saif Chaudhry <schaudhr@redhat.com>
Ensure every pf-v6-u-row-gap-0 usage has a comment explaining why the override is needed. AI-assisted change. Signed-off-by: Saif Chaudhry <schaudhr@redhat.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## dv/ROX-28622-pf-6 #19215 +/- ##
====================================================
Coverage ? 49.52%
====================================================
Files ? 2666
Lines ? 201181
Branches ? 0
====================================================
Hits ? 99635
Misses ? 94111
Partials ? 7435
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
dvail
left a comment
There was a problem hiding this comment.
The number of tweaks using classes/padding/margin solidifies my stance that we really should invest in a layout component for our pages and sections. I think it would definitely help with cases like this. The PF v6 Upgrade highlights that well.
In a lot of the pages I've worked on, I've found that the sledgehammer approach of simply removing all className overrides often gets us very close, if not exactly, to what is shown on the PatternFly docs and is similar to the OpenShift console UI.
This, in combination with removing hasBodyWrapper={false} and using <PageSection type="tabs"> and <PageSection type="breadcrumbs"> I think really reduces the need for custom layout components and CSS in the general case.
| <List isPlain> | ||
| <Flex | ||
| direction={{ default: 'column' }} | ||
| gap={{ default: 'gapMd' }} |
There was a problem hiding this comment.
Hmm I've been using the spaceItems prop for this type of behavior, and never either considered the gap prop. TIL (or re-learned? 🤔 )
ui/apps/platform/src/Containers/ComplianceEnhanced/Schedules/ScanConfigsTablePage.tsx
Outdated
Show resolved
Hide resolved
ui/apps/platform/src/Containers/ComplianceEnhanced/Schedules/ViewScanConfigDetail.tsx
Outdated
Show resolved
Hide resolved
ui/apps/platform/src/Containers/ComplianceEnhanced/Schedules/ViewScanConfigDetail.tsx
Outdated
Show resolved
Hide resolved
ui/apps/platform/src/Containers/ComplianceEnhanced/Schedules/ViewScanConfigDetail.tsx
Outdated
Show resolved
Hide resolved
ui/apps/platform/src/Containers/ComplianceEnhanced/Schedules/ViewScanConfigDetail.tsx
Outdated
Show resolved
Hide resolved
ui/apps/platform/src/Containers/ComplianceEnhanced/Schedules/ViewScanConfigDetail.tsx
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
A part of me honestly thinks we should just delete this file after the migration and clean up the fallout as a separate PR. I think these overrides are more pain than anything, and with such a small amount of Tailwind left we should deal with it as part of the effort to isolate the Tailwind styles from the pure PF parts of the app.
There was a problem hiding this comment.
Yea, that makes sense to me
…e schedules Replace row-gap-0 workaround with idiomatic PageSection usage, including type="tabs" with usePageInsets for tab sections, and remove the custom row-gap-0 CSS utility class that is no longer needed. Signed-off-by: Saif Chaudhry <schaudhr@redhat.com>
|
Oh man, lots of changes. I did try those out, and it looks like they did get the same result with fewer classes. Thanks! |
Signed-off-by: Saif Chaudhry <schaudhr@redhat.com>
Description
Fix spacing inconsistencies introduced during PF6 migration in the compliance schedules pages.
Changes:
isFilledto wizard PageSections so the wizard stretches to fill available heightList/ListItemwithFlex/FlexIteminNotifierConfigurationForm-- delivery destination cards are interactive form elements, not informational text itemspf-v6-u-row-gap-0utility class intrumps.cssto override PF6 PageSection's default 24px row gap when usinghasBodyWrapper={false}User-facing documentation
Testing and quality
Automated testing
How I validated my change
NotifierConfigurationFormchanges also apply to vulnerability reporting (shared component)Screenshots/Screen recording
Screen.Recording.2026-02-26.at.7.09.21.AM.mov