• 
      

    [review-together] Cleanups to scroll position work

    Review Request #6884 — Created Feb. 1, 2015 and submitted

    Information

    rb-extension-pack
    197aa31...

    Reviewers

    Peter Tran did a lot of preparatory work for showing the scroll
    position of your peers in a TogetherJS session. This work was
    landed in a project branch, awaiting some clean-up once the term
    was over.

    This is the clean-up I had in mind. I've fixed the following:

    * Scroll position indicators now have a high z-index to avoid being
    hidden behind other elements.
    * If no hub URL has been specified, properly falls back to the Mozilla
    TogetherJS hub.
    * Moves the ReviewTogether JS code into a closure to avoid polluting the
    global namespace.
    * If a peer's colour changes during a session, this is now manifested
    for everybody in the session without requiring a page reload.
    * Switched a lot of double-quotes to single-quotes.
    * Leaving a TogetherJS session removes all scroll position indicators.
    * When somebody leaves a session, their scroll position indicator is
    removed.

    Installed the extension locally. Then, viewed a long diff in a local review request.
    Clicked on the 'Chat' link in the header to open the TogetherJS interface. Got the
    sharing link from TogetherJS.

    Then, in a second browser, went to the sharing link. I was able to view the scroll
    position for both browser sessions on the long diff.

    Description From Last Updated

    This should probably be using a constant.

    brenniebrennie

    Blank line between statement and block.

    brenniebrennie

    This variable should be declared in the top-level var for the anonymous function.

    brenniebrennie

    Unify these into a single var

    brenniebrennie
    reviewbot
    1. Tool: Pyflakes
      Processed Files:
          review_together/review_together/extension.py
      
      Ignored Files:
          review_together/review_together/static/js/togetherjs.js
          review_together/review_together/static/js/review-together.js
          review_together/review_together/static/js/together.js
          review_together/review_together/static/css/review-together.less
      
      
      
      Tool: PEP8 Style Checker
      Processed Files:
          review_together/review_together/extension.py
      
      Ignored Files:
          review_together/review_together/static/js/togetherjs.js
          review_together/review_together/static/js/review-together.js
          review_together/review_together/static/js/together.js
          review_together/review_together/static/css/review-together.less
      
      
    2. 
        
    brennie
    1. 
        
    2. Show all issues

      This should probably be using a constant.

    3. Show all issues

      Unify these into a single var

    4. 
        
    mike_conley
    reviewbot
    1. Tool: PEP8 Style Checker
      Processed Files:
          review_together/review_together/extension.py
      
      Ignored Files:
          review_together/review_together/static/js/togetherjs.js
          review_together/review_together/static/js/review-together.js
          review_together/review_together/static/js/together.js
          review_together/review_together/static/css/review-together.less
      
      
      
      Tool: Pyflakes
      Processed Files:
          review_together/review_together/extension.py
      
      Ignored Files:
          review_together/review_together/static/js/togetherjs.js
          review_together/review_together/static/js/review-together.js
          review_together/review_together/static/js/together.js
          review_together/review_together/static/css/review-together.less
      
      
    2. 
        
    brennie
    1. 
        
    2. Show all issues

      Blank line between statement and block.

    3. Show all issues

      This variable should be declared in the top-level var for the anonymous function.

    4. 
        
    mike_conley
    reviewbot
    1. Tool: PEP8 Style Checker
      Processed Files:
          review_together/review_together/extension.py
      
      Ignored Files:
          review_together/review_together/static/js/togetherjs.js
          review_together/review_together/static/js/review-together.js
          review_together/review_together/static/js/together.js
          review_together/review_together/static/css/review-together.less
      
      
      
      Tool: Pyflakes
      Processed Files:
          review_together/review_together/extension.py
      
      Ignored Files:
          review_together/review_together/static/js/togetherjs.js
          review_together/review_together/static/js/review-together.js
          review_together/review_together/static/js/together.js
          review_together/review_together/static/css/review-together.less
      
      
    2. 
        
    brennie
    1. I'm not very familiar with review-together or TogetherJS but it looks good to me :)

    2. 
        
    mike_conley
    Review request changed
    Status:
    Completed
    Change Summary:
    Pushed to master (9e5a6b9)