feat(Page): remove deprecated page header #6588
Conversation
| @@ -199,119 +152,11 @@ $pf-page-v6--height-breakpoint-map: build-breakpoint-map("base", "sm", "md", "lg | |||
| } | |||
|
|
|||
| // Header | |||
| .#{$page}__header, | |||
| .#{$page} > .#{$masthead} { | |||
| z-index: var(--#{$page}__header--ZIndex); | |||
There was a problem hiding this comment.
This header variable I left in because it looks like its used by masthead
There was a problem hiding this comment.
Thanks for calling that out! At a minimum, we should probably rename that var. Following our var naming scheme, --#{$page}--c-masthead--ZIndex. Though I have a related question for anyone interested (@srambach @mattnolting @thatblindgeye @tlabaj).
As a follow-up, WDYT about having a structural element for the page masthead/header area (.pf-[v]-c-page__header) that we use as the container for the header area (the masthead would go inside of it). It would:
- Be the element for the header area grid item in page, so page would be:
- Page (parent grid layout)
- page__header (holds masthead)
- page__sidebar (holds navigation)
- page__main (holds page sections)
- Page (parent grid layout)
- Get the z-index for the header area
- Could be a generic container if a user didn't pass a masthead to it or we offer some other type of element that can go up there
There was a problem hiding this comment.
Off the top of my head I wouldn't be opposed. It would match the structure we're using for the other elements like you outlined above at the very least. If it's strictly a container wrapper for structure it may not cause any/much concern? Only thing I could think of is someone using the wrapper for their header content, but outside of Masthead; basically having content that should be in a header element but isn't.
There was a problem hiding this comment.
@thatblindgeye good point. I was thinking the masthead could just be a <div> by default, and page__header would be the <header> element, so whatever went in there would be in <header> by default.
|
Preview: https://patternfly-pr-6588.surge.sh A11y report: https://patternfly-pr-6588-a11y.surge.sh |
|
🎉 This PR is included in version 6.0.0-alpha.124 🎉 The release is available on: Your semantic-release bot 📦🚀 |
Closes #6563.
There was a single variable that looked like it was still in use for masthead, so I left that in. LMK if that block of code is also something we want to remove, or if we should keep it + rename the variable?