fix for clicking UX for last few headings when they cannot be scrolled to

[tscanlin]

fix for clicking UX for last few headings when they cannot be scrolled to. This is hoping to address this comment from storybook:
storybookjs/storybook#26470 (comment)

[Sidnioulz]

Hey Tim, thanks for taking the time to build this! ❤️

I tested your branch and it's overall a great improvement! This will definitely satisfy most users.

The only issue I see is if I navigate up from the bottom anchor (http://localhost:3001/tocbot#license), as soon as it's off the screen, I get sent straight to 'Troubleshooting' as per the original algorithm (the first heading to be above the top of the viewport), but I'd expect to go to 'Contributing / Steps to publish' instead. That's why I was suggesting finding the threshold element to detect 'bottom mode' in my original comment, and using that element as a trigger for either mode.

I'm curious how const isBottomMode = activeHeading.offsetTop > scrollEl.offsetHeight - (scrollEl.clientHeight * 1.4) - options.bottomModeThreshold is obtained. As a user, I would struggle to understand what use to make of bottomModeThreshold because the documentation doesn't explain how it's used and what unit to pass, and looking at the source code I am still confused. Am I supposed to tweak this value based on my content length?

If you're able to make a NPM pre-release, we could test this in situation in Storybook to see how it behaves with different content lengths.

[tscanlin]

Hey, thanks for the feedback, and I'm glad you think this is an improvement. I appreciate you working with me on this and I think that our efforts will help to ensure that users have a good experience that makes sense.

I'd like to understand better your idea of "finding the threshold element to detect 'bottom mode'", my understanding was that I could just find any headings that are at a y-offset greater than total scroll height available - scroll viewport height and those would essentially be unable to scroll to and would trigger bottomMode.

As far as the comment about jumping back up to "Troubleshooting" instead of gradually triggering other headings that cannot be scrolled to on the way back up that seems like it could be a bit tricky and doesn't seem intuitive to me. The fix in this PR is mainly meant to fix the issue for if you click a link and it is not able to be scrolled to then it should make that link show as highlighted in the right. bottomModeThreshold is meant for how many px from the bottom of the page that behavior should last for. After you scroll past 20-30 px it reverts to the previous behavior and highlights as normal (default should be fine for most folks). This is not meant to make all the links something you can scroll to or otherwise highlighted via scroll if they are out of the range. I'm not sure how that could really be done in an intuitive way but if you have ideas for that logic I'm open to it.

I'm happy to make a pre release though to help with your testing. Let me do that and once you have a chance to test it more we can figure out if any other changes are needed here. Thanks!

[tscanlin]

You can try out this pre-release version, let me know how it works for you:
[email protected]

Thanks!

[Sidnioulz]

I'd like to understand better your idea of "finding the threshold element to detect 'bottom mode'", my understanding was that I could just find any headings that are at a y-offset greater than total scroll height available - scroll viewport height and those would essentially be unable to scroll to and would trigger bottomMode.

Agree! These should all be concerned by the bottomMode logic.

As far as the comment about jumping back up to "Troubleshooting" instead of gradually triggering other headings that cannot be scrolled to on the way back up that seems like it could be a bit tricky and doesn't seem intuitive to me.

If you have multiple headers that can trigger bottom mode, you can display whichever one is visible and closest to the bottom of the screen. So in your repo's example, once License isn't visible any more, Contribution is the "bottom mode" header that's closest to the bottom and visible.

This will make the behaviour of the tocbot highlight more organic instead of leaving gaps when scrolling up and down.

The fix in this PR is mainly meant to fix the issue for if you click a link and it is not able to be scrolled to then it should make that link show as highlighted in the right. bottomModeThreshold is meant for how many px from the bottom of the page that behavior should last for. After you scroll past 20-30 px it reverts to the previous behavior and highlights as normal (default should be fine for most folks).

I feel like you've paid most of the price (in terms of software maintenance) for fixing all highlighting bugs by introducing headers that highlight differently from the existing algorithm, but you're not fully reaping the benefits by trying to keep the bugfix as small as possible.

There are now two logics for headers to be highlighted, which introduces a bunch of user behaviour edge cases. By addressing the edge cases, you can not only fix the initial bug report but prevent new ones, since there are still behaviours that feel inconsistent. And much of the price is already paid, the edge cases revolve around scrolling within the new logic.

I'm happy to make a pre release though to help with your testing. Let me do that and once you have a chance to test it more we can figure out if any other changes are needed here. Thanks!

I'll be a bit busy in the coming weeks, will likely not test integration in Storybook immediately, sadly.