Skip to content

update to simplify scroll sync behavior #358

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 2 commits into from
Nov 9, 2024

Conversation

tscanlin
Copy link
Owner

@tscanlin tscanlin commented Nov 9, 2024

update to simplify scroll sync behavior to be simpler and easier to follow. This should result in things feeling like they are jumping around less in long TOC's. I think because this only affects very long TOCs and since it seems like an improvement that this should be a pretty low risk change.

new way
tocbot-scroll-sync-new

previous way
tocbot-scroll-sync-previous

Comment on lines -17 to -22
if (eTop < cTop + options.tocScrollOffset) {
toc.scrollTop -= (cTop - eTop) + options.tocScrollOffset
// Below scroll view
} else if (eBottom > cBottom - options.tocScrollOffset - SCROLL_LEEWAY) {
toc.scrollTop += (eBottom - cBottom) + options.tocScrollOffset + (2 * SCROLL_LEEWAY)
}
Copy link
Owner Author

Choose a reason for hiding this comment

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

This was not always ideal and felt buggy since it would just barely keep the items in view.

Comment on lines +12 to +13
const scrollAmount = eTop - options.tocScrollOffset
toc.scrollTop = scrollAmount > 0 ? scrollAmount : 0
Copy link
Owner Author

Choose a reason for hiding this comment

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

This new way is more similar to how Stripe does it:
https://docs.stripe.com/api

@tscanlin tscanlin merged commit 4e7d6c6 into master Nov 9, 2024
2 checks passed
@tscanlin tscanlin deleted the adjust-scroll-sync-behavior branch November 9, 2024 22:11
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.

1 participant