fix(page): fix section spacing #6766
Conversation
|
Preview: https://patternfly-pr-6766.surge.sh A11y report: https://patternfly-pr-6766-a11y.surge.sh |
The update to the handlebars should be doing this. |
| // Page main body | ||
| --#{$page}__main-body--MarginBlockStart: 0; | ||
| --#{$page}__main-body--MarginBlockEnd: 0; | ||
| --#{$page}__main-body--PaddingInlineEnd: 0; | ||
| --#{$page}__main-body--PaddingInlineStart: 0; |
There was a problem hiding this comment.
Do we still need these? As far as I can tell, doesn't look like they're ever modified. Padding is on the outer __main-[section/breadcrumbs/etc] and the spacing between __main-body sections is a gap.
| display: flex; | ||
| flex-direction: column; |
There was a problem hiding this comment.
Since body is required now, wdyt about moving these styles to this block so all of the main-[whatever] sections (anything that has __main-body as children) have this style?
| @@ -247,7 +254,6 @@ $pf-page-v6--height-breakpoint-map: build-breakpoint-map("base", "sm", "md", "lg | |||
| &.pf-m-limit-width { | |||
| display: flex; | |||
| flex-direction: column; | |||
| padding: 0; | |||
|
|
|||
| > .#{$page}__main-body { | |||
| flex: 1; | |||
There was a problem hiding this comment.
I wonder if we can remove this? It's causing this issue of the parent section fills the available space, the main-body elements will all grow to fill the space
There was a problem hiding this comment.
This looks great! I'm assuming we want to include __main-body by default in all of the other types of page sections? With the possible exception of __main-group since it takes other sections as children. But I suppose we could add it there, too, to be consistent, if there are no side effects.
With v5, if a page had .pf-m-limit-width, we conditionally rendered __main-body and moved the padding from the outer section to the __main-body. WDYT about updating to just let the padding always remain on the outer section, and never move into the __main-body? That would allow us to simplify the CSS quite a bit. It would mean that within __main-body on a width limited section, there would be no padding-inline-end, but that should be fine. It would create a slight difference in the available space for content in a width limited section compared to v5 (more space in v6 since the padding-inline-end isn't on the inner __main-body with the max-width), but that seems fine to me.
| .#{$page}__main-list { | ||
| display: flex; | ||
| flex-direction: column; | ||
| flex-grow: 1; | ||
| } |
There was a problem hiding this comment.
I think we can take this out now? I only see it in the hackathon demo code.
| display: flex; | ||
| flex-direction: column; |
There was a problem hiding this comment.
I think you can remove this since you added it here
patternfly/src/patternfly/components/Page/page.scss
Lines 263 to 272 in bab3e8a
| .#{$page}__main-nav & { | ||
| padding-block-start: var(--#{$page}__main-nav--PaddingBlockStart); | ||
| padding-inline-start: var(--#{$page}__main-nav--PaddingInlineStart); | ||
| padding-inline-end: var(--#{$page}__main-nav--PaddingInlineEnd); |
There was a problem hiding this comment.
pretty sure you can remove this, it was for tertiary nav, which we removed since it was replaced with horizontal subnav, and we use .#{$page}__main-subnav for horizontal subnav.
| --#{$page}__main-section--PaddingBlockEnd: 0; | ||
| --#{$page}__main-section--PaddingInlineStart: calc(var(--pf-t--global--spacer--lg) - var(--#{$page}__main-container--BorderWidth)); // subtract the border of the main section | ||
| --#{$page}__main-section--PaddingBlockStart: var(--pf-t--global--spacer--md); | ||
| --#{$page}__main-section--PaddingInlineEnd: var(--pf-t--global--spacer--lg); // This is the intended spacing - the border of the main section will be subtracted |
There was a problem hiding this comment.
👍 love the update to hold the actual spacer here. Much better for customization.
|
🎉 This PR is included in version 6.0.0-alpha.162 🎉 The release is available on: Your semantic-release bot 📦🚀 |
|
Followup for the add'l stuff - #6801 |
Fixes #6631
Moves padding/margin from the page__main-body to the page__main-section.
This requires page__main-body to be required.
Refactors the side margin spacing that had a TODO on it.
Also decreases the height of the examples when they are on the example page so that each one isn't taking up 100vh!
NOTE: To see what it looks like, it currently has 16px (md spacer) in as the space between rather than the 24 that was spec'd.
Backstop report with 24px: https://drive.google.com/file/d/1or9X7tbWaN3TFv41rKexfmS1oV_invdi/view?usp=sharing