8000 fix(page): add container around main element by srambach · Pull Request #6410 · patternfly/patternfly · GitHub
[go: up one dir, main page]

Skip to content

fix(page): add container around main element#6410

Merged
mcoker merged 7 commits intopatternfly:v6from
srambach:6225-restructure-page-containers
Mar 21, 2024
Merged

fix(page): add container around main element#6410
mcoker merged 7 commits intopatternfly:v6from
srambach:6225-restructure-page-containers

Conversation

@srambach
Copy link
Member
@srambach srambach commented Mar 11, 2024

This adds a container around the main element so that the border-rounding and other styling can be applied to the container, while the main element handles layout of the sections.

Fixes #6225

@patternfly-build
Copy link
Collaborator
patternfly-build commented Mar 11, 2024

@srambach srambach linked an issue Mar 11, 2024 that may be closed by this pull request
@srambach srambach requested a review from mcoker March 19, 2024 13:32
Copy link
Contributor
@mcoker mcoker left a comment

Choose a reason for hiding this comment

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

Just one nit

This is a default <code>.pf-v6-c-page__main-section</code>.
{{/page-main-section}}
{{/page-main}}
{{#> page-main-container}}
Copy link
Contributor

Choose a reason for hiding this comment

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

Looks like this is only in one page component example. Could you just wrap the contents of page-main.hbs in {{#> page-main-container}} ... {{/page-main-container}}? If so I think you could also remove it from here

{{#> page-main-container}}
{{#> page-main page-main--attribute=(concat 'id="main-content-' page--id '"')}}
{{> page-template-main-content}}
{{/page-main}}
{{/page-main-container}}

@srambach srambach requested a review from mcoker March 21, 2024 18:20
Copy link
Contributor
@mcoker mcoker left a comment

Choose a reason for hiding this comment

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

One more bit - now that {{#> page-main}} includes this globally, we can remove the container from this template

{{#> page-main-container}}
{{#> page-main page-main--attribute=(concat 'id="main-content-' page--id '"')}}
{{> page-template-main-content}}
{{/page-main}}
{{/page-main-container}}

Also just to confirm, with a drawer in the page (like a notification drawer) the grid area for the main page stuff is .pf-c-page__drawer which is not currently wrapped in the main-container - should it be, or is it good the way it is?

@srambach
Copy link
Member Author
srambach commented Mar 21, 2024

One more bit - now that {{#> page-main}} includes this globally, we can remove the container from this template

if one is good, two are better, no? 🤪

{{#> page-main-container}}
{{#> page-main page-main--attribute=(concat 'id="main-content-' page--id '"')}}
{{> page-template-main-content}}
{{/page-main}}
{{/page-main-container}}

Also just to confirm, with a drawer in the page (like a notification drawer) the grid area for the main page stuff is .pf-c-page__drawer which is not currently wrapped in the main-container - should it be, or is it good the way it is?

Hmmmmm. That would fix a problem where a drawer content with a background escapes the rounded corners, but also cause the drawer panel to be contained inside the rounded corners, which is not the intention I think.

@mcoker mcoker merged commit bcae200 into patternfly:v6 Mar 21, 2024
@patternfly-build
Copy link
Collaborator

🎉 This PR is included in version 6.0.0-alpha.104 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Page - main content rounded corners not always visible

3 participants

0