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

Adding sticky header js and styling #244

Conversation

spaceo
Copy link
Contributor

@spaceo spaceo commented Aug 28, 2023

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

  • My complies with our coding guidelines.
  • My code passes our static analysis suite. If not then I have added a comment explaining why this change should be exempt from the code standards and process.
  • My code passes our continuous integration process. If not then I have added a comment explaining why this change should be exempt from the code standards and process.

Copy link
Contributor

@Adamik10 Adamik10 left a 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!

Copy link
Contributor

@kasperg kasperg left a 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.

Comment on lines +40 to +44
<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>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 excellent.

src/stories/Blocks/header/Header.tsx Show resolved Hide resolved
src/stories/Blocks/header/header-sticky.js Outdated Show resolved Hide resolved
src/stories/Blocks/header/header-sticky.js Show resolved Hide resolved
src/stories/Blocks/header/header-sticky.js Outdated Show resolved Hide resolved
src/stories/Blocks/header/header-sticky.js Outdated Show resolved Hide resolved
src/stories/Blocks/header/header.scss Outdated Show resolved Hide resolved
spaceo and others added 2 commits August 31, 2023 10:56
Since we manipulate with the style attribute of the header directly
@spaceo spaceo force-pushed the DDFDPDEL-215-semi-sticky-headers-pa-mobil branch from 068095b to 6c56c89 Compare August 31, 2023 10:02
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.
@spaceo spaceo force-pushed the DDFDPDEL-215-semi-sticky-headers-pa-mobil branch from 6c56c89 to bb11fc1 Compare August 31, 2023 10:03
@spaceo spaceo merged commit 1e34ea0 into danskernesdigitalebibliotek:develop Aug 31, 2023
6 checks passed
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

Successfully merging this pull request may close these issues.

6 participants