-
Notifications
You must be signed in to change notification settings - Fork 9.4k
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
Report: header sticks to top #1121
Conversation
Don't look at this yet. Not ready. |
Ready |
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.
nice nice. a few things.
@@ -78,6 +82,11 @@ html, body { | |||
height: 100%; | |||
} | |||
|
|||
/* When deep linking to a section, bump the headind down so it's not covered by the header. */ |
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.
headind typo
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.
done
background: #FFFFFF; | ||
min-width: 185px; | ||
/*min-width: 185px;*/ |
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.
^ ^
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.
done
display: flex; | ||
flex-direction: row; | ||
align-items: center; | ||
justify-content: flex-end; | ||
padding: 0 var(--heading-line-height); | ||
position: fixed; | ||
z-index: 1; |
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.
do me a solid and add will-change: transform
on here as well? for lo-dpi chrome desktop users.
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.
done
.report-body__menu-container { | ||
height: 100%; | ||
width: 100%; | ||
min-width: 230px; | ||
max-width: 1280px; | ||
max-width: var(--report-width); | ||
position: fixed; |
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.
can u add will-change to this one as well?
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.
done.
I was curious about the layout of the app. It seems really funky to use pos fixed, transforms, and negative margins. What's up with that?
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.
the negative margins comes from @paullewis. tbh I've never used that technique and am not sure why we do it like that. :)
PTAL |
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.
😍
R: all
This sticks the report header to the top of the page as you scroll. I think this is a nice change to make the share/print button always visible (e.g. LH viewer, devtools).