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

Refactor positioning and spacing of page-header component #78

Open
slkennedy opened this issue Oct 12, 2017 · 6 comments
Open

Refactor positioning and spacing of page-header component #78

slkennedy opened this issue Oct 12, 2017 · 6 comments

Comments

@slkennedy
Copy link
Contributor

Currently the height of the page-header is being set by margin from the header's sub-elements. Rework this to come from padding on the page-header and come up with a way to shift the sub-elements to the top or bottom when necessary.

@townivan
Copy link

As I was playing around with this, I'm starting to think that deriving the page-header height from one of it's children (namely the logo) isn't a bad thing.

Here is a pen with that concept working to keep vertical alignment balanced for mobile and desktop: https://codepen.io/townivan/pen/pWOOjL?editors=1111

@townivan
Copy link

I've attempted some styling in this similar thinking to Willow imported from a precompiled css source: https://codepen.io/townivan/pen/YrRzEJ

@townivan
Copy link

townivan commented Oct 16, 2017

Here is some override css that fixes things in my project:
`
// header override testing
.willow-page-header {
align-items: center; // vert align branding and menu toggle (desktop nav links will be align-self:flex-end)
padding-top: 0; // top 24 not needed because top spacing will come from .willow-page-header__branding
}
.willow-page-header__branding {
padding: 24px 0; // let the horizontal padding come from willow-page-header BUT the vertical spacing (padding DOES come from here)
margin-top:0;
margin-bottom:0;
}
.willow-page-header__branding:first-child {
margin-top: 0;
}

.willow-page-header__content-controls {
padding: 0; // the vertical parts are not needed because the flex align center will handle this based on the height created from the taller branding
}
@media screen and (min-width: 828px){
.willow-page-header__content {
align-self: flex-end; // align desktop nav items to the bottom of the header
}
}
`

@slkennedy
Copy link
Contributor Author

For testing purposes:

  • with primary navigation
  • without primary navigation
  • with menu/close icon (with and without text)
  • without menu/close icon (text only)

@slkennedy slkennedy reopened this Nov 8, 2017
@slkennedy
Copy link
Contributor Author

@bollinghball pointed out that the spacing of the menu/close button is off when you add the icon in.

@slkennedy
Copy link
Contributor Author

Note for the future: to fix the menu/close button spacing add align-items: center to the .willow-page-header__controls element.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants