[review-together] Cleanups to scroll position work

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

mike_conley
rb-extension-pack
197aa31...
rb-extension-pack

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. This should probably be using a constant.

  3. 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. Blank line between statement and block.

  3. 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: Closed (submitted)

Change Summary:

Pushed to master (9e5a6b9)
Loading...