Skip to content

Passing root to isScrollbarClick #9

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

Merged
merged 5 commits into from
Sep 11, 2024

Conversation

patrick-hurley
Copy link
Contributor

Hello, thanks for the recent release and for making this repo - it’s been incredibly useful.

I hope you don't mind the PR but I noticed that clicking and dragging the scrollbar still causes the active link state to stop updating correctly in some cases. I’ve created a small update to address this problem and hope it’s helpful.

Reproduction of issue being solved:

  • On the 'Window' example of the demo, When clicking on link ‘9’ and then dragging the scrollbar, the active link no longer updates correctly.
  • On the 'Container' example of the demo, clicking any menu item, followed by dragging the scrollbar, results in the active link not updating as expected.

Solution:

  • Modified the isScrollbarClick method to accept root as a parameter and update the condition for identifying scrollbar clicks.
  • Removed the isTop and isBottom checks within isScrollbarClicked

Once again, I hope you don’t mind me opening this PR, and I really appreciate all the work you’ve done in making this project public.

Copy link

netlify bot commented Sep 10, 2024

Deploy Preview for vue-use-active-scroll ready!

Name Link
🔨 Latest commit 4cb1c91
🔍 Latest deploy log https://app.netlify.com/sites/vue-use-active-scroll/deploys/66e0de39b9d6e90008ffddc5
😎 Deploy Preview https://deploy-preview-9--vue-use-active-scroll.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.

@smastrom smastrom merged commit 90f3731 into smastrom:main Sep 11, 2024
5 checks passed
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