Scroll on Commit Diff keeps jumping around

[madhaven]

The problem

The scroll bar and content on the diff view of a file in the commit selected from the history tab is glitching.

Release version

Version 3.3.5 (x64)

Operating system

Windows 10 x64

Steps to reproduce the behavior

I use a unified view and I don't have my "hide whitespaces" checked.
I'm checking out my history, I select a commit, I select a file and I am watching the diff.
As I scroll down, I expand some of the hidden lines by clicking the arrows on the left margin on the diff code.
when I try to scroll down fast, the scrollbar and the diff scroll position glitches and flickers across a different position.

Log files

No response

Screenshots

No response

Additional context

No response

[niik]

Hey @madhaven, thanks for reaching out. Does this only happen when you expand hidden lines in a diff? Do you think you could capture a video of the behavior?

We have recently made changes to the rendering of unified diffs that I'm certain is responsible for this behavior

[tidy-dev]

I was able to reproduce this:

I found a particularly long file so that the expand hunk down button will work multiple times. I expand down once and scroll to see expander again, then, hit expand down again, now, the top of the diff will be at line x, if I try to scroll down quickly to see the expander again, it will scroll for a second and then jump back to line x and then continue scrolling.

CleanShot.2023-12-05.at.05.16.29.mp4

[tidy-dev]

Also noticed on same file after same repro steps if I was scrolled all the way to the bottom, and then I use my keyboard to scroll up. Just hit the keyboard button 2 times, it all the sudden jumps to the top. Not sure if this is the same issue from the other one above, but I believe this one is a regression from #17259 of which I have another PR #17499 open that moves away from the auto focus approach for other buggy reasons.

CleanShot.2023-12-05.at.05.31.16.mp4

[madhaven]

@steveward , @tidy-dev
Thanks for keeping up.
I was not able to provide inputs

@tidy-dev
I was curious how you got to the root of the problem and fixed it.
I would also like to know the process.

[tidy-dev]

I was curious how you got to the root of the problem and fixed it.

Because I caused it. 🙈 I was recently working on keyboard accessibility of the expand buttons and using autofocus property to focus the next one after expansion. It worked great when there are buttons on the screen to focus; however, autofocus will cause scrolling to "scroll to view" of an element. Combined with virtual scrolling applied on diff, it caused this jumpiness. The fix that was just shipped to beta was to refactor to a better approach that keeps track of the buttons that are currently rendered by the virtual scrolling and if moves focus to the closest one. This should prevent attempts to scroll to out of screen buttons.

[madhaven]

thank you @tidy-dev
That was educative.