-
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
💄 [#1180] Make scroll-to-top anchor appear on every page #528
Conversation
This is such an extremely tiny new 'component', I didn't think it would be necessary to create new JS folders and a new component template (it replaces a list-item that was part of the anchor-menu) - but feel free to suggest otherwise. |
Codecov Report
@@ Coverage Diff @@
## develop #528 +/- ##
========================================
Coverage 96.51% 96.51%
========================================
Files 528 530 +2
Lines 19040 19049 +9
========================================
+ Hits 18377 18386 +9
Misses 663 663 see 4 files with indirect coverage changes 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
@@ -0,0 +1,19 @@ | |||
// Get the button | |||
const mybutton = document.getElementById('scroll-anchor') |
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.
Just a general remark, we use camel-case names in js (myButton).
No need of changes here.
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.
@vaszig Thanks, it seemed weird to use camelCase here because CSS is case-insensitive, but after some investigation, it seems it's 'convention' to use camelCase with ID's.
-make scroll-to-top appear everywhere on all mobile pages
-make it disappear when user is scrolled to top completely= make it appear only when scrolling down
issue: https://taiga.maykinmedia.nl/project/open-inwoner/task/1180