Fix the display of the floating review banners while scrolling.

Review Request #6293 — Created Sept. 3, 2014 and submitted

Information

Review Board
release-2.0.x
886fe00...

Reviewers

The floating review draft banners had some issues when scrolling off the
edge of a review. Due to some imprecise calculations, the page would
appear to stutter for a brief moment while scrolling, as the banner
jumped back and forth between being docked to the page and floating over
the page.

To fix the stutter, we needed to set the placeholder to have the exact
height and margins of the original banner, pre-float. This kept the
container the same size. We also needed to fix some of the calculations
to not go in/out of float mode too soon.

With this fixed, the page no longer jumped, but the banner would just
disappear as we hit the edge of the review. Now, the banner will appear
to dock at the bottom of the review, and just scroll out of view as the
user scrolls further down.

The end result is a buttery-smooth scrolling experience.

Tested on Chrome and Firefox.

I edited a field to get a draft banner and saw it docked at the top of the
review. I began scrolling and saw it smoothly undock, without any page
contents jumping. I then scrolled until it was out of view. The banner never
went past the bottom of the review, and it remained fixed there until it was
no longer in view.

(Checking the DOM verified that it did leave the floating state, yet there was
no weird page jump/stutter.)

Description From Last Updated

How about calling this 'wasFloating'? The tense of this variable name compared with your comments is pretty confusing.

daviddavid
reviewbot
  1. Tool: PEP8 Style Checker
    Ignored Files:
        reviewboard/static/rb/css/reviews.less
        reviewboard/static/rb/js/views/floatingBannerView.js
    
    
    
    Tool: Pyflakes
    Ignored Files:
        reviewboard/static/rb/css/reviews.less
        reviewboard/static/rb/js/views/floatingBannerView.js
    
    
  2. 
      
david
  1. 
      
  2. Show all issues

    How about calling this 'wasFloating'? The tense of this variable name compared with your comments is pretty confusing.

  3. 
      
chipx86
reviewbot
  1. Tool: PEP8 Style Checker
    Ignored Files:
        reviewboard/static/rb/css/reviews.less
        reviewboard/static/rb/js/views/floatingBannerView.js
    
    
    
    Tool: Pyflakes
    Ignored Files:
        reviewboard/static/rb/css/reviews.less
        reviewboard/static/rb/js/views/floatingBannerView.js
    
    
  2. 
      
david
  1. Ship It!

  2. 
      
chipx86
Review request changed

Status: Closed (submitted)

Change Summary:

Pushed to release-2.0.x (d3f8d4f)
Loading...