Scroll to top of file content when the file is collapsed
What does this MR do?
For #321377 (closed)
This MR scrolls to the top of a file's content container when it is collapsed.
Mechanisms
There are three main ways a user can collapse a file "manually":
- By clicking the small caret on the left side of the file header
- By clicking in the blank space of the file header
- By clicking the "Viewed" checkbox (if it is not currently checked)
All three of these mechanisms trigger the same collapsing behavior.
Problem
With very large files - especially when followed by a number of significantly smaller files - when the user collapses the file, the browser's scroll position is left where it was before. The file that was just collapsed has a height of basically zero, so the scroll position of the browser winds up being significantly far below the bottom of the file that was just collapsed. If the collapsed file is large, files may be scrolled above where the user is currently viewing, meaning they never see that file.
Solution in this MR
This MR resets the scroll position to the top of the diff file content when the user manually collapses a file.
Even when collapsed, this location is scrollable (it just has a height of 0). This is a better location anchor than the file hash, because that ID points to a sticky location on the page, which causes the browser to often scroll to very incorrect and unpredictable places.
Screenshots (strongly suggested)
It may not be exactly clear what these videos are showing, so here's a brief description:
- In the before video, I scroll down a long file, and then click the "Viewed" checkbox. It collapses, leaving me scrolled over a hundred lines into the next file! (it would be farther if this MR had more files, but the page couldn't scroll down any more).
- In the after video, I do the same actions, but you'll see that the browser is left with the next file already at the top: collapsing the file scrolled us to the top of that file's content, it didn't leave us at the bottom of the page.
Before | After |
---|---|
before | after |
Does this MR meet the acceptance criteria?
Conformity
-
I have included a changelog entry, or it's not needed. (Does this MR need a changelog?) -
I have added/updated documentation, or it's not needed. (Is documentation required?) -
I have properly separated EE content from FOSS, or this MR is FOSS only. (Where should EE code go?) -
I have added information for database reviewers in the MR description, or it's not needed. (Does this MR have database related changes?) -
I have self-reviewed this MR per code review guidelines. -
This MR does not harm performance, or I have asked a reviewer to help assess the performance impact. (Merge request performance guidelines) -
I have followed the style guides.
Availability and Testing
-
I have added/updated tests following the Testing Guide, or it's not needed. (Consider all test levels. See the Test Planning Process.) -
I have tested this MR in all supported browsers, or it's not needed. -
I have informed the Infrastructure department of a default or new setting change per definition of done, or it's not needed.
Security
Does this MR contain changes to processing or storing of credentials or tokens, authorization and authentication methods or other items described in the security review guidelines? If not, then delete this Security section.
- [-] Label as security and @ mention
@gitlab-com/gl-security/appsec
- [-] The MR includes necessary changes to maintain consistency between UI, API, email, or other methods
- [-] Security reports checked/validated by a reviewer from the AppSec team
Related to #321377 (closed)