-
Notifications
You must be signed in to change notification settings - Fork 6
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
Adding sticky header js and styling #244
Adding sticky header js and styling #244
Conversation
5087f8a
to
f817ac4
Compare
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.
Very like. Much approve!
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.
👍 for implementing this in the design system.
I have some comments for you to consider. The most important one what I think you need to address before merging this is up
/down
header classes.
<p className="my-16"> | ||
<strong> | ||
Because of limitations regarding determing y position and scroll in an | ||
iframe open the frame to see the sticky header in action | ||
</strong> |
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.
👍 excellent.
Co-authored-by: Kasper Garnæs <[email protected]>
Since we manipulate with the style attribute of the header directly
068095b
to
6c56c89
Compare
We need to handle the z-index layers in a controlled manner so we use the same concept as we use for spacing with fixed values.
6c56c89
to
bb11fc1
Compare
Link to issue
https://reload.atlassian.net/browse/DDFDPDEL-215
Description
Adds sticky header feature to the header.
See it in action
https://616ffdab9acbf5003ad5fd2b-kemehffrpc.chromatic.com/iframe.html?args=&id=blocks-header--sticky-header-with-content&viewMode=story
Checklist