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

fix: moved smooth-scroll application to useEffect #2868

Merged
merged 3 commits into from
Apr 11, 2024

Conversation

werdnanoslen
Copy link
Contributor

@werdnanoslen werdnanoslen commented Apr 4, 2024

Summary

Fixes leak of smooth-scroll, limits it just to pages where component is being used

Also changed a variable to be more descriptive

Related Issues or PRs

Resolves #2867

How To Test

Use storybook as an example, comparing the live version (where smooth-scroll is applied on all pages after loading the component) vs what you can build locally, shown below:

Apr-04-2024 14-31-50

Screenshots (optional)

also renamed a variable to be more clear
@werdnanoslen werdnanoslen requested a review from a team as a code owner April 4, 2024 21:37
Copy link
Contributor

github-actions bot commented Apr 4, 2024

Warnings
⚠️ This PR does not include changes to storybook, even though it affects component code.

Generated by 🚫 dangerJS against 62cfe93

Copy link
Contributor

@kimallen kimallen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me! I wonder if there would be value in including a test that checks for the class?

@werdnanoslen
Copy link
Contributor Author

Looks good to me! I wonder if there would be value in including a test that checks for the class?

thanks, added

Copy link
Contributor

@kimallen kimallen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for adding the test. Ship!

@werdnanoslen werdnanoslen merged commit bc87158 into main Apr 11, 2024
7 checks passed
@werdnanoslen werdnanoslen deleted the an-smoothscroll-2867 branch April 11, 2024 16:17
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.

[fix] smooth scrolling css leaks from component to whole package
2 participants