Closed Emuohwo closed 4 years ago
@AdrianBZG @Nikhil-Vats Kindly review Pull Request for issue #90
Nice work @Emuohwo. Everything works as expected. 💯 👍 🎉
I think this class is not used anywhere and can be removed. I tried removing it and everything works just fine. @AdrianBZG what do you think?
One more thing, for smaller screens the button overlaps footer content -
I think we can increase bottom to 40px and it will be better that way, see -
Hi @Emuohwo ,
Nice work, if you could address the issue pointed by @Nikhil-Vats (increase bottom to 40px), we'd happy to accept this PR 😄
@Nikhil-Vats and @AdrianBZG consider it done.
@Nikhil-Vats and @AdrianBZG the scroll button still covered part of the footer content even after I have increase bottom to 40px
I now increase the bottom to 50px
and added a media query of max-width: 600px
and set it to 75px
. here's what I got
Should I continue with your recommendation or I should use the 50px and media query. What do you advice?
@Emuohwo Nice work on the media query! 🎉 💯
Can you change the media-query from max-width:600px
to max-width:785px
, it would look better that way?
@Nikhil-Vats I have increased the width to 785px for the media query.
@Emuohwo Good work! 💯
Description
Describe the changes made and why they were made. Added a container in footer.pug for the scroll to top button: this is the container where the scroll button is located. Added js code for the button to common.js: I want to button to trigger when a user scroll to a certain level downward I did the styling of the button in _global.scss: This is to give the button a good look
Related issues and discussion
90
Screenshots, if any
This is the look on small screen On a bigger screen
Checklist
Please make sure these boxes are checked before submitting your pull request - thanks!