• 
      

    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:
    Completed
    Change Summary:
    Pushed to release-2.0.x (d3f8d4f)