Improve admin header #959
Closed
Improve admin header #959
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This will give us a fixed header in the admin area.
Use a flexbox to align the content in the content-header. This is semantically better than using a table display for non-tabular data and is a move away from the 16 column skeleton that we will be eliminating.
|
I like the way the admin is heading to. Nice work. Nothing to complain (can you believe it) 👍 |
| @@ -65,6 +65,10 @@ body { | |||
| @include clearfix; | |||
| } | |||
|
|
|||
| .content-under-header { | |||
| padding-top: 89px; | |||
There was a problem hiding this comment.
What does this value represent? If it's the header's height we should save that as a value: $height-header and use it here and on the header.
|
After talking about it we're not sure we want to push in a sticky header quite yet but are happy with the other changes so we will leave this for debate and ponderings in #630 and make a new PR for the skeleton work. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Make the header sticky and adjust the display of the page title and actions in response to #630. I'd like to draw attention to leaving the
<div class="toolbar" data-hook="toolbar">in. I wasn't certain it would be safe to take this out since someone might be hooking into the toolbar data-hook and it makes no difference in how the page renders. Also I took out the conditional for the page title to render the<h1>element. I made this choice since the simple concise flex box setup I went with relied on having a title on the page (or at least an empty h1). I believe there are no pages in the admin that don't have content for the page_title. Further, an empty h1 element shouldn't hurt anyone either on the off chance they removed the text for the page_title.Before
(I skipped including a view of the screen scrolled down with no header in view. It seemed pointless)
After