processing / p5.js-website

New p5.js website!
http://p5js.org
MIT License
18 stars 89 forks source link

[Feat] :Adding a move to top icon #631

Open rishabdev2997 opened 2 weeks ago

rishabdev2997 commented 2 weeks ago

[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

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

https://github.com/user-attachments/assets/55ca8a2b-671f-4811-85aa-e66c9a18d001

rishabdev2997 commented 2 weeks ago

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 commented 2 weeks ago

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 commented 2 weeks ago

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