Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(page): fix section spacing #6766

Merged
merged 4 commits into from
Jun 17, 2024
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 5 additions & 9 deletions src/patternfly/components/Page/examples/Page.css
Original file line number Diff line number Diff line change
@@ -1,9 +1,5 @@
#ws-core-c-page-with-or-without-fill .ws-preview-html,
#ws-core-c-page-multiple-sidebar-body-elements-padding-and-fill .ws-preview-html {
height: 500px;
}

#ws-core-c-page-multiple-sidebar-body-elements .ws-preview-html,
#ws-core-c-page-using-flex-layout .ws-preview-html {
height: 100%;
}
/* Because the page component has height:100dvh, override it just for the embedded examples */
.ws-mdx-content-content [id^=ws-core-c-page-] .ws-preview-html .pf-v6-c-page {
height: min-content;
min-height: 30em;
}
15 changes: 13 additions & 2 deletions src/patternfly/components/Page/examples/Page.md
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,17 @@ import './Page.css'
{{#> page-main-section}}
This is a default <code>.pf-v6-c-page__main-section</code>.
{{/page-main-section}}
{{#> page-main-section page-main-section--modifier="pf-m-secondary" page-main-section--ExcludeMainBody="true"}}
{{#> page-main-body}}
This is a page__main-body, one of three within one page__main-section.
{{/ page-main-body}}
{{#> page-main-body}}
This is a page__main-body, one of three within one page__main-section.
{{/ page-main-body}}
{{#> page-main-body}}
This is a page__main-body, one of three within one page__main-section.
{{/ page-main-body}}
{{/page-main-section}}
{{/page-main}}
{{/page}}
```
Expand Down Expand Up @@ -238,10 +249,10 @@ This component provides the basic chrome for a page, including sidebar and main
| `.pf-v6-c-page__main-nav` | `<section>` | Creates a container to nest the (deprecated) tertiary navigation component in the main page area. |
| `.pf-v6-c-page__main-subnav` | `<section>` | Creates a container to nest the horizontal subnav navigation component in the main page area. |
| `.pf-v6-c-page__main-breadcrumb` | `<section>` | Creates a container to nest the breadcrumb component in the main page area. |
| `.pf-v6-c-page__main-section` | `<section>` | Creates a section container in the main page area. **Note: The last/only `.pf-v6-c-page__main-section` element will grow to fill the availble vertical space. You can change this behavior using `.pf-m-fill` and `.pf-m-no-fill`, which are documented below.** |
| `.pf-v6-c-page__main-section` | `<section>` | Creates a section container in the main page area. **Note: The last/only `.pf-v6-c-page__main-section` element will grow to fill the available vertical space. You can change this behavior using `.pf-m-fill` and `.pf-m-no-fill`, which are documented below.** |
| `.pf-v6-c-page__main-tabs` | `<section>` | Creates a container to nest the tabs component in the main page area. |
| `.pf-v6-c-page__main-wizard` | `<section>` | Creates a container to nest the wizard component in the main page area. |
| `.pf-v6-c-page__main-body` | `<div>` | Creates the body section for a page section. **Required when using `.pf-m-limit-width` on `.pf-v6-c-page__main-section`** |
| `.pf-v6-c-page__main-body` | `<div>` | Creates the body section for a page section. **Required** |
| `.pf-v6-c-page__main-group` | `<div>` | Creates the group of `.pf-v6-c-page__main-*` sections. Can be used in combination with `.pf-m-sticky-[top/bottom]` to make multiple sections sticky. |
| `.pf-v6-c-page__drawer` | `<div>` | Creates a container for the drawer component when placing the main page element in the drawer body. |
| `.pf-m-expanded` | `.pf-v6-c-page__sidebar` | Modifies the sidebar for the expanded state. |
Expand Down
6 changes: 3 additions & 3 deletions src/patternfly/components/Page/page-main-section.hbs
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,11 @@
{{#if page-main-section--attribute}}
{{{page-main-section--attribute}}}
{{/if}}>
{{#if page-main-section--IsLimitWidth}}
{{#if page-main-section--ExcludeMainBody}}
{{> @partial-block}}
{{else}}
{{#> page-main-body}}
{{> @partial-block}}
{{/page-main-body}}
{{else}}
{{> @partial-block}}
{{/if}}
</section>
52 changes: 28 additions & 24 deletions src/patternfly/components/Page/page.scss
Original file line number Diff line number Diff line change
Expand Up @@ -65,11 +65,11 @@ $pf-page-v6--height-breakpoint-map: build-breakpoint-map("base", "sm", "md", "lg
--#{$page}__main-container--BorderColor: var(--#{$page}__main-container--BackgroundColor); // TODO Border should match the background to blend in - change to be a page outline token

// Main section
--#{$page}__main-section--MarginBlockStart: var(--pf-t--global--spacer--md);
--#{$page}__main-section--PaddingBlockStart: var(--pf-t--global--spacer--lg);
--#{$page}__main-section--PaddingInlineEnd: calc(var(--pf-t--global--spacer--lg) - var(--#{$page}__main-container--BorderWidth)); // subtract the border of the main section - TODO, see if we can remove the need for this calc(), or possibly move it down to the property definition so a user can theme this without a calc()
--#{$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
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 love the update to hold the actual spacer here. Much better for customization.

--#{$page}__main-section--PaddingBlockEnd: var(--pf-t--global--spacer--md);
--#{$page}__main-section--PaddingInlineStart: var(--pf-t--global--spacer--lg); // This is the intended spacing - the border of the main section will be subtracted
--#{$page}__main-section--RowGap: var(--pf-t--global--spacer--lg);
--#{$page}__main-breadcrumb--main-section--PaddingBlockStart: var(--pf-t--global--spacer--md);
--#{$page}__main-section--BackgroundColor: var(--pf-t--global--background--color--primary--default);
--#{$page}__main-section--m-secondary--BackgroundColor: var(--pf-t--global--background--color--secondary--default);
Expand All @@ -79,6 +79,12 @@ $pf-page-v6--height-breakpoint-map: build-breakpoint-map("base", "sm", "md", "lg
// Limit width
--#{$page}--section--m-limit-width--MaxWidth: calc(#{pf-size-prem(2000px)} - var(--#{$page}__sidebar--Width));

// Page main body
--#{$page}__main-body--MarginBlockStart: 0;
--#{$page}__main-body--MarginBlockEnd: 0;
--#{$page}__main-body--PaddingInlineEnd: 0;
--#{$page}__main-body--PaddingInlineStart: 0;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.


// Sticky
--#{$page}--section--m-sticky-top--ZIndex: var(--pf-t--global--z-index--md);
--#{$page}--section--m-sticky-top--BoxShadow: var(--pf-t--global--box-shadow--sm--bottom);
Expand Down Expand Up @@ -118,6 +124,7 @@ $pf-page-v6--height-breakpoint-map: build-breakpoint-map("base", "sm", "md", "lg
--#{$page}__main-tabs--PaddingBlockStart: 0;
--#{$page}__main-tabs--PaddingInlineEnd: 0;
--#{$page}__main-tabs--PaddingBlockEnd: 0;
--#{$page}__main-tabs--PaddingInlineStart: 0;
--#{$page}__main-tabs--BackgroundColor: var(--pf-t--global--background--color--primary--default);

// Wizard main section
Expand Down Expand Up @@ -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;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

padding: 0;

> .#{$page}__main-body {
flex: 1;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Screenshot 2024-06-14 at 10 45 58 AM

Expand Down Expand Up @@ -345,6 +351,7 @@ $pf-page-v6--height-breakpoint-map: build-breakpoint-map("base", "sm", "md", "lg
.#{$page}__main-list {
display: flex;
flex-direction: column;
flex-grow: 1;
}
Comment on lines 343 to 347
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we can take this out now? I only see it in the hackathon demo code.


.#{$page}__main-nav {
Expand Down Expand Up @@ -429,10 +436,13 @@ $pf-page-v6--height-breakpoint-map: build-breakpoint-map("base", "sm", "md", "lg
}

.#{$page}__main-section {
display: flex;
flex-direction: column;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you can remove this since you added it here

.#{$page}__main-nav,
.#{$page}__main-breadcrumb,
.#{$page}__main-tabs,
.#{$page}__main-section,
.#{$page}__main-wizard,
.#{$page}__main-group,
.#{$page}__main-subnav {
display: flex;
flex-direction: column;
flex-shrink: 0;

gap: var(--#{$page}__main-section--RowGap);
padding-block-start: var(--#{$page}__main-section--PaddingBlockStart);
padding-block-end: var(--#{$page}__main-section--PaddingBlockEnd);
padding-inline-start: var(--#{$page}__main-section--PaddingInlineStart);
padding-inline-end: var(--#{$page}__main-section--PaddingInlineEnd);
padding-inline-start: calc(var(--#{$page}__main-section--PaddingInlineStart) - var(--#{$page}__main-container--BorderWidth));
padding-inline-end: calc(var(--#{$page}__main-section--PaddingInlineEnd) - var(--#{$page}__main-container--BorderWidth));
background-color: var(--#{$page}__main-section--BackgroundColor);

&.pf-m-secondary {
Expand All @@ -447,17 +457,17 @@ $pf-page-v6--height-breakpoint-map: build-breakpoint-map("base", "sm", "md", "lg
&.pf-m-padding#{$breakpoint-name} {
padding-block-start: var(--#{$page}__main-section--PaddingBlockStart);
padding-block-end: var(--#{$page}__main-section--PaddingBlockEnd);
padding-inline-start: var(--#{$page}__main-section--PaddingInlineStart);
padding-inline-end: var(--#{$page}__main-section--PaddingInlineEnd);
padding-inline-start: calc(var(--#{$page}__main-section--PaddingInlineStart) - var(--#{$page}__main-container--BorderWidth));
padding-inline-end: calc(var(--#{$page}__main-section--PaddingInlineEnd) - var(--#{$page}__main-container--BorderWidth));

&.pf-m-limit-width {
padding: 0;

.#{$page}__main-body {
padding-block-start: var(--#{$page}__main-section--PaddingBlockStart);
padding-block-end: var(--#{$page}__main-section--PaddingBlockEnd);
padding-inline-start: var(--#{$page}__main-section--PaddingInlineStart);
padding-inline-end: var(--#{$page}__main-section--PaddingInlineEnd);
padding-inline-start: calc(var(--#{$page}__main-section--PaddingInlineStart) - var(--#{$page}__main-container--BorderWidth));
padding-inline-end: calc(var(--#{$page}__main-section--PaddingInlineEnd) - var(--#{$page}__main-container--BorderWidth));
}
}
}
Expand Down Expand Up @@ -494,28 +504,23 @@ $pf-page-v6--height-breakpoint-map: build-breakpoint-map("base", "sm", "md", "lg

.#{$page}__main-body {
.#{$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);
Comment on lines 496 to 498
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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-breadcrumb & {
padding-block-start: var(--#{$page}__main-breadcrumb--PaddingBlockStart);
padding-block-end: var(--#{$page}__main-breadcrumb--PaddingBlockEnd);
padding-inline-start: var(--#{$page}__main-breadcrumb--PaddingInlineStart);
padding-inline-end: var(--#{$page}__main-breadcrumb--PaddingInlineEnd);
padding-inline-start: 0;
padding-inline-end: 0;
}

.#{$page}__main-section & {
padding-block-start: var(--#{$page}__main-section--PaddingBlockStart);
padding-block-end: var(--#{$page}__main-section--PaddingBlockEnd);
padding-inline-start: var(--#{$page}__main-section--PaddingInlineStart);
padding-inline-end: var(--#{$page}__main-section--PaddingInlineEnd);
padding-inline-start: var(--#{$page}__main-body--PaddingInlineStart);
padding-inline-end: var(--#{$page}__main-body--PaddingInlineEnd);
margin-block-start: var(--#{$page}__main-body--MarginBlockStart);
margin-block-end: var(--#{$page}__main-body--MarginBlockEnd);
}

.#{$page}__main-tabs & {
padding-block-start: var(--#{$page}__main-tabs--PaddingBlockStart);
padding-block-end: var(--#{$page}__main-tabs--PaddingBlockEnd);
padding-inline-start: var(--#{$page}__main-tabs--PaddingInlineStart);
padding-inline-end: var(--#{$page}__main-tabs--PaddingInlineEnd);
}
Expand All @@ -528,4 +533,3 @@ $pf-page-v6--height-breakpoint-map: build-breakpoint-map("base", "sm", "md", "lg
flex: 1 0 auto;
}
}

Loading