fix(page): fix main layout with no sidebar#7455
Conversation
|
Preview: https://patternfly-pr-7455.surge.sh A11y report: https://patternfly-pr-7455-a11y.surge.sh |
|
Without the masthead, do we care that there is no space at the top of the white area? If the use case really is a whole page and no masthead I think it would look better with some. OTOH if it gets used embedded into something else, I'm not sure but also that seems like a bad idea to embed the whole page in something. |
There was a problem hiding this comment.
Code looks good - just wondering about providing spacing at the top if there is no masthead.
|
@srambach good question! My opinion - this is for no sidebar, so I'd say if that works, we could go ahead and get this through. I updated the PR title from "no masthead/sidebar" to just "no sidebar" since that's what it's about - even though it fixed the use case of no masthead/sidebar. On the top padding, I'm not sure what all use cases there are for layouts without a masthead. Looking at #7357, if I'm looking at the screenshots right, they are creating their own type of masthead, so they likely wouldn't want any sort of top padding. But looking at #7452, this is just an alert on an empty page, so they likely would want top padding. There is probably a bigger question of whether those layouts are built the way we would expect, but assuming they are, I think we would want to support both. Possibly something like We could tag design to review and give their dime, or merge this and create a follow-up to look at that. I'd probably be inclined to merge this as-is so we can go forward with the patch release. WDYT? |
|
@mcoker I'm fine to go ahead and merge since it's no worse than it was and it fixes an existing problem. |
|
🎉 This PR is included in version 6.3.0-prerelease.6 🎉 The release is available on: Your semantic-release bot 📦🚀 |
Partially addresses #7377
Fixes #7452
Fixes #7357
Adds
.pf-m-no-sidebarto.pf-v6-c-pagefor layouts without a sidebar as an opt-in so it doesn't impact existing page layouts.