-
Notifications
You must be signed in to change notification settings - Fork 133
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
Fix heading padding FOUC (option 3 with modifications: sticky header) #1939
Changes from 23 commits
28c3358
2ed7491
c3fa410
597019b
12979af
a91e90a
0b44177
e24c96e
39a6eaa
98485a5
8a4cae8
5eb413f
526f273
41af9f5
3728ec3
38d5a9e
b5672db
fd74b17
a58bb21
e9f6732
8547c90
facad3a
aa9f60b
b9358fb
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -56,28 +56,28 @@ mark { | |
margin: 0 auto; | ||
min-width: 0; | ||
max-width: 1000px; | ||
overflow-x: auto; | ||
padding: 0.8rem 20px 0 20px; | ||
transition: 0.4s; | ||
-webkit-transition: 0.4s; | ||
} | ||
|
||
#site-nav, | ||
#page-nav { | ||
display: flex; | ||
flex-direction: column; | ||
position: sticky; | ||
top: 0; | ||
top: var(--sticky-header-height); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 2nd change per the docs |
||
flex: 0 0 auto; | ||
padding-top: 1rem; | ||
max-width: 300px; | ||
max-height: calc(100vh - var(--sticky-header-height)); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. last change: The other changes ( |
||
width: 300px; | ||
} | ||
|
||
#site-nav { | ||
display: flex; | ||
flex-direction: column; | ||
border-right: 1px solid lightgrey; | ||
padding-bottom: 20px; | ||
z-index: 999; | ||
max-height: 100vh; | ||
} | ||
|
||
.site-nav-top { | ||
|
@@ -90,13 +90,7 @@ mark { | |
} | ||
|
||
#page-nav { | ||
display: block; | ||
border-left: 1px solid lightgrey; | ||
max-height: 90vh; | ||
} | ||
|
||
#page-nav .nav-component { | ||
max-height: 90vh; | ||
} | ||
|
||
@media screen and (max-width: 1299.98px) { | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Documenting the layout file changes for review + future release changelog noting (should be simplified though I think =P)
1st of 3 important changes in the layout file:
This might be somewhat controversial, but is unfortunately necessary with a sticky header. At least, I haven't found another solution.
The issue (without this) occurs on pages that overflow (e.g. "formatting contents" page or "Using html javascript and css"). The nearest scrolling ancestor which is the page / root element expands to accomodate the overflown content, which in turn does 2 things:
window.innerWidth/Height
are erroneous (although there is an easy alternative for thisdocument.documentElement.clientWidth/Height
)Gif demo (open image in new tab to view in order): there are 3 settings here.
translateY
) to show the issue is really unrelated to header hiding / anything we are doing.You can try removing the
overflow-x
when testing this PR to see the issue better, its a little hard to present on a gif.The fix: I've set
overflow-x
on the content wrapper which significantly changes ux on mobile devices (but just pages that overflow).Pros:
Cons:
Not sure if there's a better solution =, but imo its a good enough compromise.