What does this MR do?
Fix scroll flicker that can occur when you are on a page with a sidebar and your viewport has just a bit of overflow.
Are there points in the code the reviewer needs to double check?
Picks cleanly into EE master
I am only able to reproduce after these were introduced https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/12399 and https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/12299 but they were merged after https://gitlab.com/gitlab-org/gitlab-ce/issues/33984 which was a scroll issue. I want to reproduce at that earlier time to make sure I am not reintroducing the other issue but not having any luck
How can we better test this?
- It depends on the viewport size which doesn't seem to be very configurable in Karma
- We could make a rspec integration test and count the number of
affix
events
For reference, here is Bootstrap's affix.js
that we use.
Things to inspect
- MR when
affix
andaffix-top
- Issue when
affix
andaffix-top
- Look for cross-reference icon
- Issues Bulk Edit when
affix
andaffix-top
- Wiki page when
affix
andaffix-top
My reproductions:
MR
- Checkout CE
master
, 585e6aa5HEAD
- Visit 1918x717, http://192.168.1.135:3000/gitlab-org/gitlab-ce/merge_requests/12#note_1137 (MR with
test bar
for title and description and 1 diff note), http://i.imgur.com/cDbudfo.png - Scroll down to the bottom of the page (you should be on the "Discussion" tab)
- Switch to the "Commits" tab the and notice the continuous scroll flicker or switch to the "Changes" tab to see flicker until content loads in
Issue
- Checkout CE
master
, 585e6aa5HEAD
- Visit 1901x1009, http://192.168.1.135:3000/gitlab-org/gitlab-ce/issues/24 (Issue with 2 lines of text and 3 comments), http://i.imgur.com/aAQdm76.png
- Scroll down to the bottom of the page and notice continuous scroll flicker
Why was this MR needed?
- The
.right-sidebar
swaps out.affix
/.affix-top
back and forth causing fast visual scroll flicker.
Does this MR meet the acceptance criteria?
- Tests
-
Added for this feature/bug -
All builds are passing
-
-
Conform by the merge request performance guides -
Conform by the style guides -
Branch has no merge conflicts with master
(if it does - rebase it please) -
Squashed related commits together
What are the relevant issue numbers?
Closes #34407 (closed)