TOC: Scrolling: Fixed TOC links and scrolling

[shmolf]

Description

Fixed the Scrolling when clicking a link from the TOC.

Instead of modifying the overflow for html, just needed to change the track's background.
Also, changed a reference on line 1052 to this.getWindow(), so it's consistent with a reference shortly there after.

Issue fixed

#3287

Type of changes

• 🔘 Bug fix (Change that fixed an issue)
• ⚪ Breaking change (Change that can cause existing functionality to change)
• 🔘 Improvement (Change that improves the code. Maybe performance or development improvement)
• ⚪ Feature (Change that adds new functionality)
• ⚪ Documentation change (Change that modifies documentation. Maybe typo fixes)

Checklist:

• 🔘 My code follows the project code style
• ⚪ I have written test for my code and it has been tested
• 🔘 All existing tests have been passed
• 🔘 I have attached a screenshot/video to visualize my change if possible

[shmolf]

@Flexo013 Tagging you because you mentioned some work being done here

[Flexo013]

@Flexo013 Tagging you because you mentioned some work being done here

Thanks for the tag and the PR! I think this might already be fixed by #3294. Could you compare these PRs and see if there are any differences or improvements.

[shmolf]

They are different solutions for sure. But the other solution does look like improvements. But I haven't actually reviewed the changes thoroughly.

But he corrects the usage of scrollTo in MarkdownPreview.js to be used against the window object.

[shmolf]

I recently pulled master from the upstream repository, and it doesn't seem like scrolling is working as expected from the master branch. My changes seem to work fine, however.

@Flexo013 Are you experiencing the same thing?

Unexpected behavior from master:

Expected behavior from toc-scroll:

[Flexo013]

@hikerpig @Rokt33r Can either of you confirm that the merged fix from #3294 is not quite sufficient and that this PR is a good fix on top of the previous fix?

[hikerpig]

I think #3294 is sufficient for fixing TOC-link-click-scrolling behavior.

But the ::-webkit-scrollbar-track-piece is much better than my original body-height and html-overflowY tweak for scrollbar styling (#2902). I should do some cleanup for my old solution.

[shmolf]

@hikerpig I recently pulled master into this branch (yeseterday), and it includes your commit.

[hikerpig]

@shmolf Would you mind checking this commit and merge it for your PR ? I've done testing for below scenarios.

1. While scrollPastEnd enabled , both preview and split mode should work well and are consistent. (Without my commit, this seems not working)
2. Click TOC link, scrolling to target works well.

[shmolf]

With both Allow editor to scroll past last line and Allow preview to scroll past last line enabled.

[shmolf]

@hikerpig
Yep, nevermind. It's not often I merge changes to my branch. I didn't pull into my local machine.

Looks like it works

[shmolf]

I noticed that your changes weren't in my sandbox. I'm glad it's working now though.

[shmolf]

The last screenshot was from scrolling down from the Editor side.
The screenshot below is from scrolling down on the Preview side.

[shmolf]

I'm assuming the variance has to do with offset or scrollheight differences between the two sides.

Does that sound right?

[shmolf]

@hikerpig ToC works. Did you experience this behavior from your own fork?

[hikerpig]

@shmolf I guess so, bue not quite sure, this difference exists before I initial commit about issue 2902.

From what I see in Devtools, .CodeMirror-lines padding-bottom is calculated during runtime and is represented in px, and MarkdownPreview padding-bottom is written in css 90vh.

This may need another PR for proper discussions.

[Flexo013]

@ZeroX-DG Since you reviewed the previous related PR could you also review this one?

[shmolf]

@ZeroX-DG Are there anything questions or concerns you have about the changes?

[ZeroX-DG]

This one look good to me but I haven't tested it yet. Will do it when I get home.

[ZeroX-DG]

Yup, confirm that this solve the bug:) LGTM 🎉