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

feat(sbb-sticky-bar): fade in vertically during initial load [WIP] #3073

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

jeripeierSBB
Copy link
Contributor

Closes #3072

Copy link
Contributor

@MarioCastigliano MarioCastigliano left a comment

Choose a reason for hiding this comment

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

Just a few considerations, but overall a solid job 👍

Comment on lines +72 to +74
// To decide whether the fade in should happen from bottom up,
// we check how far away the sticky bar from the intersector is. When scrolling fast, the
// difference can slightly vary. From this perspective we add a height tolerance.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
// To decide whether the fade in should happen from bottom up,
// we check how far away the sticky bar from the intersector is. When scrolling fast, the
// difference can slightly vary. From this perspective we add a height tolerance.
// To decide whether to fade in from the bottom up,
// we check how far away the sticky bar is from the intersector. When scrolling fast, the
// difference can vary slightly. To account for this, we add a height tolerance.

this.toggleAttribute(
'data-sticking',
!entry.isIntersecting && entry.boundingClientRect.top > 0,
'data-fade-vertically',
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not 100% sold on the name 'data-fade-vertically' since the animation looks a bit more like a slide-up, but I don't really know a better name and it's a trivial aspect after all 😅

@github-actions github-actions bot added pr: peer review approved Pull request has been approved by a peer review and removed pr: peer review required A peer review is required for this pull request labels Sep 11, 2024
@jeripeierSBB jeripeierSBB linked an issue Sep 16, 2024 that may be closed by this pull request
3 tasks
@jeripeierSBB jeripeierSBB changed the title feat(sbb-sticky-bar): fade in vertically during initial load feat(sbb-sticky-bar): fade in vertically during initial load [WIP] Sep 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr: peer review approved Pull request has been approved by a peer review pr: visual review required
Projects
None yet
2 participants