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:add scroll-to-top #57

Closed
wants to merge 1 commit into from

Conversation

vrun1208
Copy link

@vrun1208 vrun1208 commented Oct 4, 2023

Description

Added a ScrollToTop button in order to ensure better user experience!

What type of PR is this? (check all applicable)

  • πŸ• Feature
  • πŸ› Bug Fix
  • πŸ“ Documentation Update
  • 🎨 Style
  • πŸ§‘β€πŸ’» Code Refactor
  • πŸ”₯ Performance Improvements
  • βœ… Test
  • πŸ€– Build
  • πŸ” CI
  • πŸ“¦ Chore (Release)
  • ⏩ Revert

Related Tickets & Documents

Mobile & Desktop Screenshots/Recordings

2023-10-05

Added tests?

  • πŸ‘ yes
  • πŸ™… no, because they aren't needed
  • πŸ™‹ no, because I need help

Added to documentation?

  • πŸ“œ README.md
  • πŸ““ docs.opensauced.pizza
  • πŸ• dev.to/opensauced
  • πŸ“• storybook
  • πŸ™… no documentation needed

[optional] Are there any post-deployment tasks we need to perform?

[optional] What gif best describes this PR or how it makes you feel?

@netlify
Copy link

netlify bot commented Oct 4, 2023

βœ… Deploy Preview for sauced-intro ready!

Name Link
πŸ”¨ Latest commit 2fabfda
πŸ” Latest deploy log https://app.netlify.com/sites/sauced-intro/deploys/651db237d7f4a100089dc709
😎 Deploy Preview https://deploy-preview-57--sauced-intro.netlify.app/
πŸ“± Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@CBID2
Copy link
Contributor

CBID2 commented Oct 5, 2023

Thanks for contributing @vrun1208, but I'm not sure how it fits with the project. Can you explain?

@vrun1208
Copy link
Author

vrun1208 commented Oct 5, 2023

Thanks for contributing @vrun1208, but I'm not sure how it fits with the project. Can you explain?

Hi @CBID2, actually for desktop view it does not much needed but in mobile view there are long readings and unnecessary scrolling from bottom to top on same page while reading docs.

@CBID2
Copy link
Contributor

CBID2 commented Oct 5, 2023

Thanks for contributing @vrun1208, but I'm not sure how it fits with the project. Can you explain?

Hi @CBID2, actually for desktop view it does not much needed but in mobile view there are long readings and unnecessary scrolling from bottom to top on same page while reading docs.

I'm still unsure @vrun1208.

@CBID2
Copy link
Contributor

CBID2 commented Oct 5, 2023

Hi @vrun1208. After a having discussion with the team, we decided to not accept this contribution. While this feature is nice to have, the course is not too long to scroll at the moment, even on mobile. Additionally, our hamburger menu on the bottom also makes easier to navigate between sections on mobile. In future, please raise an issue, wait until it gets approved and assigned to you before making a pull request(link the issue to your PR form too).

@CBID2 CBID2 closed this Oct 5, 2023
@vrun1208
Copy link
Author

vrun1208 commented Oct 6, 2023

Hi @vrun1208. After a having discussion with the team, we decided to not accept this contribution. While this feature is nice to have, the course is not too long to scroll at the moment, even on mobile. Additionally, our hamburger menu on the bottom also makes easier to navigate between sections on mobile. In future, please raise an issue, wait until it gets approved and assigned to you before making a pull request(link the issue to your PR form too).

Yeah, That's okay!, I'll surely come up with a new issue soonπŸš€

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.

2 participants