Fix the star column not updating from mobile mode

Review Request #8155 — Created May 11, 2016 and submitted

Information

Review Board
release-2.5.x
cfb19d6...

Reviewers

Previously, when a datagrids switched from mobile mode to desktop mode,
all state (such as whether a review request or group was starred) that
changed in mobile mode would be lost. Now, we keep track of these
changes with the RB.StarManagerView, which will update the affected
rows in the datagrid when transitioning to desktop mode.

Manually tested the following star icons:

  • Review request datagrid
  • Group datagrid
  • Individual review request pages

Ran JS tests.

Description From Last Updated

Trailing ws.

daviddavid

Trailing ws.

daviddavid

Semicolon.

daviddavid

You never use this inside this function, so there's no reason to bind.

daviddavid

Should we have an else clause that asserts?

daviddavid

Since you only test whether this is falsy, you don't need the || false part.

daviddavid

Still repeating this twice.

daviddavid

Still repeating this twice.

daviddavid
reviewbot
  1. Tool: Pyflakes
    Processed Files:
        reviewboard/staticbundles.py
    
    Ignored Files:
        reviewboard/static/rb/js/views/starManagerView.js
        reviewboard/static/rb/js/common.js
        reviewboard/static/rb/js/pages/views/datagridPageView.js
        reviewboard/static/rb/js/pages/views/reviewablePageView.js
        reviewboard/static/rb/js/models/starManagerModel.js
    
    
    
    Tool: PEP8 Style Checker
    Processed Files:
        reviewboard/staticbundles.py
    
    Ignored Files:
        reviewboard/static/rb/js/views/starManagerView.js
        reviewboard/static/rb/js/common.js
        reviewboard/static/rb/js/pages/views/datagridPageView.js
        reviewboard/static/rb/js/pages/views/reviewablePageView.js
        reviewboard/static/rb/js/models/starManagerModel.js
    
    
  2. 
      
david
  1. 
      
  2. reviewboard/static/rb/js/common.js (Diff revision 1)
     
     

    Woohoo! Very happy to see common.js disappearing.

  3. Show all issues

    Trailing ws.

  4. Show all issues

    Trailing ws.

  5. Show all issues

    Semicolon.

  6. Show all issues

    You never use this inside this function, so there's no reason to bind.

  7. Show all issues

    Should we have an else clause that asserts?

  8. 
      
brennie
reviewbot
  1. Tool: Pyflakes
    Processed Files:
        reviewboard/staticbundles.py
    
    Ignored Files:
        reviewboard/scmtools/testdata/hg_repo/.hg/cache/rbc-revs-v1
        reviewboard/static/rb/js/views/starManagerView.js
        reviewboard/scmtools/testdata/hg_repo/.hg/cache/rbc-names-v1
        reviewboard/static/rb/js/models/starManagerModel.js
        reviewboard/static/rb/js/common.js
        reviewboard/static/rb/js/pages/views/datagridPageView.js
        reviewboard/static/rb/js/pages/views/reviewablePageView.js
    
    
    
    Tool: PEP8 Style Checker
    Processed Files:
        reviewboard/staticbundles.py
    
    Ignored Files:
        reviewboard/scmtools/testdata/hg_repo/.hg/cache/rbc-revs-v1
        reviewboard/static/rb/js/views/starManagerView.js
        reviewboard/scmtools/testdata/hg_repo/.hg/cache/rbc-names-v1
        reviewboard/static/rb/js/models/starManagerModel.js
        reviewboard/static/rb/js/common.js
        reviewboard/static/rb/js/pages/views/datagridPageView.js
        reviewboard/static/rb/js/pages/views/reviewablePageView.js
    
    
  2. 
      
chipx86
  1. It looks like this breaks starring for review requests? Or have those moved beyond using the code in common.js? If this is truly specific to datagrids now, I feel like the naming needs to be more specific, as it sounds generic (StarManager) and yet is only in the datagrid bundle.

    1. No, it does not. The new JS is in both the reviews and datagrids bundle.

  2. 
      
brennie
reviewbot
  1. Tool: PEP8 Style Checker
    Processed Files:
        reviewboard/staticbundles.py
    
    Ignored Files:
        reviewboard/static/rb/js/views/starManagerView.js
        reviewboard/static/rb/js/common.js
        reviewboard/static/rb/js/pages/views/datagridPageView.js
        reviewboard/static/rb/js/pages/views/reviewablePageView.js
        reviewboard/static/rb/js/models/starManagerModel.js
    
    
    
    Tool: Pyflakes
    Processed Files:
        reviewboard/staticbundles.py
    
    Ignored Files:
        reviewboard/static/rb/js/views/starManagerView.js
        reviewboard/static/rb/js/common.js
        reviewboard/static/rb/js/pages/views/datagridPageView.js
        reviewboard/static/rb/js/pages/views/reviewablePageView.js
        reviewboard/static/rb/js/models/starManagerModel.js
    
    
  2. 
      
brennie
  1. Ping.

  2. 
      
david
  1. We really shouldn't duplicate the same files in multiple bundles. Stick it into common?

  2. 
      
brennie
david
  1. 
      
  2. Show all issues

    Since you only test whether this is falsy, you don't need the || false part.

  3. reviewboard/staticbundles.py (Diff revision 4)
     
     
    Show all issues

    Still repeating this twice.

  4. reviewboard/staticbundles.py (Diff revision 4)
     
     
    Show all issues

    Still repeating this twice.

  5. 
      
reviewbot
  1. Tool: Pyflakes
    Processed Files:
        reviewboard/staticbundles.py
    
    Ignored Files:
        reviewboard/static/rb/js/views/starManagerView.js
        reviewboard/static/rb/js/common.js
        reviewboard/static/rb/js/pages/views/datagridPageView.js
        reviewboard/static/rb/js/pages/views/reviewablePageView.js
        reviewboard/static/rb/js/models/starManagerModel.js
    
    
    
    Tool: PEP8 Style Checker
    Processed Files:
        reviewboard/staticbundles.py
    
    Ignored Files:
        reviewboard/static/rb/js/views/starManagerView.js
        reviewboard/static/rb/js/common.js
        reviewboard/static/rb/js/pages/views/datagridPageView.js
        reviewboard/static/rb/js/pages/views/reviewablePageView.js
        reviewboard/static/rb/js/models/starManagerModel.js
    
    
  2. 
      
brennie
reviewbot
  1. Tool: Pyflakes
    Processed Files:
        reviewboard/staticbundles.py
    
    Ignored Files:
        reviewboard/static/rb/js/views/starManagerView.js
        reviewboard/static/rb/js/common.js
        reviewboard/static/rb/js/pages/views/datagridPageView.js
        reviewboard/static/rb/js/pages/views/reviewablePageView.js
        reviewboard/static/rb/js/models/starManagerModel.js
    
    
    
    Tool: PEP8 Style Checker
    Processed Files:
        reviewboard/staticbundles.py
    
    Ignored Files:
        reviewboard/static/rb/js/views/starManagerView.js
        reviewboard/static/rb/js/common.js
        reviewboard/static/rb/js/pages/views/datagridPageView.js
        reviewboard/static/rb/js/pages/views/reviewablePageView.js
        reviewboard/static/rb/js/models/starManagerModel.js
    
    
  2. 
      
brennie
reviewbot
  1. Tool: Pyflakes
    Processed Files:
        reviewboard/staticbundles.py
    
    Ignored Files:
        reviewboard/static/rb/js/views/starManagerView.js
        reviewboard/static/rb/js/common.js
        reviewboard/static/rb/js/pages/views/datagridPageView.js
        reviewboard/static/rb/js/pages/views/reviewablePageView.js
        reviewboard/static/rb/js/models/starManagerModel.js
    
    
    
    Tool: PEP8 Style Checker
    Processed Files:
        reviewboard/staticbundles.py
    
    Ignored Files:
        reviewboard/static/rb/js/views/starManagerView.js
        reviewboard/static/rb/js/common.js
        reviewboard/static/rb/js/pages/views/datagridPageView.js
        reviewboard/static/rb/js/pages/views/reviewablePageView.js
        reviewboard/static/rb/js/models/starManagerModel.js
    
    
  2. 
      
david
  1. Ship It!
  2. 
      
brennie
Review request changed
Status:
Completed
Change Summary:
Pushed to release-2.5.x (c979e8b)