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] :Adding a move to top icon #631

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

Conversation

rishabdev2997
Copy link

@rishabdev2997 rishabdev2997 commented Nov 8, 2024

[Feat] : Adding a move to top icon

#597 - Added the feature.

Feature details-

This feature reduces the time and effort needed to manually scroll up, making navigation smoother.

How to see the change

  • Scroll Button Location: The "Move to Top" button is positioned at the bottom-right corner of the homepage.
  • Visibility Condition: The button becomes visible only when the user has scrolled down from the top of the page, enhancing usability by not cluttering the view initially.
  • Functionality: Clicking on the button will instantly scroll the user back to the top of the page.

This contributes to making the site more intuitive for users, allowing them to quickly navigate long pages, especially helpful on the homepage with multiple sections.

Here is the screenshot

go.to.top.with.pointer.2.mp4

@rishabdev2997
Copy link
Author

Hi @limzykenneth,

I hope you're doing well! I’ve made a PR for the "Move to Top" button component. Previously, I had submitted a version with inline styles, which is generally considered bad practice. This time, I've refactored it to avoid inline styles and implemented the button via a component-based approach.

Could you please review the PR and let me know if any changes are required or if you have any feedback? Once reviewed, please feel free to merge it.

Looking forward to your thoughts!

Thanks,
@rishabdev2997

@limzykenneth
Copy link
Member

Thanks @rishabdev2997. As mentioned in your previous PR, there really should be some discussions in the issue to determine the implementation first. This is to serve three purposes:

  1. Discuss and determine if the feature is really needed or not.
  2. If decided that it is needed, what the proposed design and implementation should be.
  3. Understanding the existing approach and using a consistent implementation approach.

The implementation you have here does not match the general approach we take to adding and styling elements on the website. I'm hesitating to directly mention the general direction in terms of the approach here because I really need a more detail discussion to be had in the issue first before a PR should be filed.

@rishabdev2997
Copy link
Author

Hi @limzykenneth ,

Thank you for the feedback, and I understand your concerns. I apologize for jumping directly into the PR without a detailed discussion in the issue thread. I wanted to address the feedback from the previous PR about avoiding inline styles, so I went ahead with a component-based approach this time.

I agree that it’s essential to have a consistent implementation and design approach across the website. I’ll take a step back and initiate a discussion in the relevant issue to align on the need for this feature and determine the appropriate design and implementation method.

Looking forward to collaborating on this and making sure it fits well with the overall project standards.

Thanks again for your guidance!

Best,
@rishabdev2997

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