-
-
Notifications
You must be signed in to change notification settings - Fork 79k
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
Clear scrollspy selection above first section #13563
Conversation
@tadeuzagallo Could you please remove |
Oh, sorry... Just removed it! |
return activeTarget != (i = targets[0]) && this.activate(i) | ||
if (activeTarget && scrollTop < offsets[0]) { | ||
this.activeTarget = null | ||
return this.clear() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But clear()
doesn't return anything...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It was just to stop the script... Should I change it to
this.clear()
return
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, I see, this is Jacob being idiosyncratic in his style. Nevermind then. Leave as-is for the time being.
/cc @fat for review |
@tadeuzagallo Could you write a unit test for this to prevent future regression? |
Thanks for the work! I merged a modified version with a unit test. |
Oh, I'm sorry, I hadn't seen the comment regarding the tests... |
Once scrollspy had activated any item of the navbar it wouldn't clear it
anymore, leaving the first item activated for a scrolltop above the
first section.
I don't know if this was intended, but it has been bothering me in every project that I used ScrollSpy.