feat(scroll): smarter scroll behavior

[solymosi]

Currently Docsify has the following behavior when it comes to scrolling:

• if auto2top is enabled then:
  • any change to a URL that includes an ID will scroll to that ID once loaded
  • any change to a URL without an ID always scrolls to the top once loaded
  • therefore the browser's scroll restoration behavior is always nullified when moving back/forward in history or reloading the page
• if auto2top is disabled then:
  • any change to a URL that includes an ID will scroll to that ID once loaded
  • moving back/forward in history and reloading the page uses the browser's scroll restoration behavior, but only if the target does not have an ID
  • clicking a link to navigate to a page that does not include an ID maintains the current scroll position and usually ends up somewhere in the middle of that page (fixesUnexpected autoscroll docsify-cli#42; fixesUnexpected autoscroll #612)

Ideally, the following should happen (in my opinion):

• moving back/forward in history and reloading the current page should not invoke any kind of scrolling behavior and therefore should not override the browser's scroll restoration behavior
• clicking a link to navigate to a page should scroll to the top of that page or to an ID if specified

This PR implements this (in my opinion better) behavior for both the hash and the history router modes. Scrolling to the top only happens if auto2top is enabled.

It does not attempt to fix the inconsistent scrolling issue that's caused by images loaded asynchronously.

• Make sure you are merging your commits to master branch.
• Add some descriptions and refer relative issues for you PR.
• DO NOT include files inside lib directory.

[timaschew]

Please run these commands to trigger netlify deployment for this Pull Request:

git commit --amend --no-edit
git push --force

[solymosi]

@timaschew Done.

[timaschew]

Hey @solymosi,
Thank you for your contribution!

FYI: the DNS for the preview has changed: https://deploy-preview-744--docsifyjs.netlify.com

Ideally, the following should happen (in my opinion):

• moving back/forward in history and reloading the current page should not invoke any kind of scrolling behavior and therefore should not override the browser's scroll restoration behavior
• clicking a link to navigate to a page should scroll to the top of that page or to an ID if specified

I totally agree with you.

I've just tried some actions:

1. Open Plugins page
2. Click in the sidebar on the next page: Write a Plugin
3. Go back using browser history

Expected: Scroll position at pagination
Actual: Scroll position is between emoji and External Script

When using history mode it works fine.

Also I don't understand how auto2top: false can be useful. For me it's broken because if you open another page the scroll position is only correct if you didn't scroll on the previous page, which is mostly not the case. Or do I miss anything? (This is of course not directly related to your PR)

[solymosi]

I've just tried some actions:

1. Open Plugins page
2. Click in the sidebar on the next page: Write a Plugin
3. Go back using browser history

Hmm... that looks like a bug, and it's reliably reproducable when:

• the page being navigated away from is shorter than the previous page being navigated back to;
• and the target location on the previous page is lower than the length of the current page.

I guess the reason is that the browser's scroll restoration behavior is invoked before the old page is properly replaced with the new one by Docsify and therefore the target scroll position is capped at the height of the old page. The fact that it's only triggered when using the hash mode suggests that browsers treat popstate and hashchange events differently in this regard.

The problem is that this may require implementing our own custom scroll restoration behavior and turning off the browser's default: https://developers.google.com/web/updates/2015/09/history-api-scroll-restoration. But I'd need to do some digging first because there might be a simpler workaround as well.

In any case, I'd recommend addressing this particular issue in a different PR.

Also I don't understand how auto2top: false can be useful. For me it's broken because if you open another page the scroll position is only correct if you didn't scroll on the previous page, which is mostly not the case. Or do I miss anything? (This is of course not directly related to your PR)

I have no idea how it's useful either, but since it existed I assumed there was at least some reason for it, so I kept it in there. Totally fine with removing it, however.

[timaschew]

Merging this PR before the other one, actually let's say publishing a new version based on this PR would be a regression. Because currently it's working fine for URLs with an ID.

Regarding auto2top: false I suggest to remove this in 5.0

/cc @jhildenbiddle

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[anikethsaha]

cc @solymosi whats the status on this ?
@timaschew

[solymosi]

Let me rebase this and resolve the conflicts. After that it can be merged, unless you'd like me to make additional changes.

[solymosi]

Rebased and fixed up the whitespace style to match that of the develop branch.

[anikethsaha]

I will review it soon

cc @timaschew your thoughts ?

[solymosi]

Rebased again due to merge conflicts. Any updates on the review?

[anikethsaha]

can you please fix the CI issue.

[solymosi]

Linting issues fixed ✔️ – couldn't do anything about the "security flow", that step seems to be broken.