• 
      

    Add a cleaner and more informative file index to the diff viewer.

    Review Request #4450 — Created Aug. 14, 2013 and submitted

    Information

    Review Board
    master

    Reviewers

    Add a cleaner and more informative file index to the diff viewer.
    
    The old file index was a bit of an ugly mess that never really got
    cleaned up. There was no alignment, very little consistency, and it
    didn't show much in the way of useful information.
    
    This new one has a clean layout with some useful data.
    
    When loading, a spinner appears to the left of the filename, instead of
    after it.
    
    The spinner is then replaced with a little pie chart icon representing
    the complexity of the change. This is based on the line counts to the
    file. The pie chart shows the inserts vs. deletes vs. replaces. The
    center of the pie chart is white, representing the unchanged lines.
    The larger the white center, the less of the file has changed. This
    makes it really easy to see at a glance how big a change is to a file.
    
    Then there's the filename, which links to the proper part of the diff.
    Depending on the file, there may be additional information below the
    file (renamed file info).
    
    Then there's a list of chunks, in the order of the file. This is a bit
    different from the pie graph, in that it acts as a way of seeing how
    many chunks of changes (rather than relative numbers of lines) exist,
    and jumping to them. Combined with the pie chart, it's easier to get a
    good sense of how much effort a diff is going to be to review.
    
    (We discussed possibly removing this in favor of just the graph, but I
    think that it complements the pie chart well when trying to get a sense
    for how complex a change will be to review, so I'd like to try it out in
    real-world usage for a while.)
    
    If the file is a binary file or deleted file, then a little status text
    saying "Deleted" or "Binary file" will be displayed instead of the
    chunks.
    
    When failing to load a diff, the icon is replaced with a warning icon.
    Tested with a variety of types of changes (as the screenshot demonstrates).
    This covers:
    
    * New files
    * Deleted files
    * Changed files (with different combinations of inserts, deletes and replaces)
    * Moved/renamed files
    * Binary files
    * Diff load errors

    Description From Last Updated

    These colors don't match the rest

    daviddavid

    Indentation is kind of funky here.

    daviddavid
    reviewbot
    1. This is a review from Review Bot.
        Tool: PEP8 Style Checker
        Processed Files:
          reviewboard/settings.py
          reviewboard/diffviewer/renderers.py
        Ignored Files:
          reviewboard/templates/diffviewer/diff_file_fragment.html
          reviewboard/static/rb/css/diffviewer.less
          reviewboard/templates/diffviewer/changeindex_entry.html
          reviewboard/templates/diffviewer/view_diff.html
          reviewboard/templates/diffviewer/diff_fragment_error.html
          reviewboard/templates/diffviewer/changeindex.html
          reviewboard/static/rb/js/pages/views/diffViewerPageView.js
      
      
    2. 
        
    reviewbot
    1. This is a review from Review Bot.
        Tool: Pyflakes
        Processed Files:
          reviewboard/settings.py
          reviewboard/diffviewer/renderers.py
        Ignored Files:
          reviewboard/templates/diffviewer/diff_file_fragment.html
          reviewboard/static/rb/css/diffviewer.less
          reviewboard/templates/diffviewer/changeindex_entry.html
          reviewboard/templates/diffviewer/view_diff.html
          reviewboard/templates/diffviewer/diff_fragment_error.html
          reviewboard/templates/diffviewer/changeindex.html
          reviewboard/static/rb/js/pages/views/diffViewerPageView.js
      
      
    2. 
        
    david
    1. 
        
    2. Show all issues
      These colors don't match the rest
      1. Updating the icon and the dots to be consistent. These are intentionally different from the diff chunks, though, since those colors don't stand on their own when up against a white background without the border (and the border colors make for terrible colors when not used as a border).
    3. Show all issues
      Indentation is kind of funky here.
      1. It looks funky because of the position of the function() {, but it's at the correct indentation level relative to the previous.
        
        Still, reworking this to seem less funky.
    4. 
        
    chipx86
    reviewbot
    1. This is a review from Review Bot.
        Tool: PEP8 Style Checker
        Processed Files:
          reviewboard/settings.py
          reviewboard/diffviewer/renderers.py
        Ignored Files:
          reviewboard/templates/diffviewer/diff_file_fragment.html
          reviewboard/static/rb/css/diffviewer.less
          reviewboard/templates/diffviewer/changeindex_entry.html
          reviewboard/templates/diffviewer/view_diff.html
          reviewboard/templates/diffviewer/diff_fragment_error.html
          reviewboard/templates/diffviewer/changeindex.html
          reviewboard/static/rb/js/pages/views/diffViewerPageView.js
      
      
    2. 
        
    reviewbot
    1. This is a review from Review Bot.
        Tool: Pyflakes
        Processed Files:
          reviewboard/settings.py
          reviewboard/diffviewer/renderers.py
        Ignored Files:
          reviewboard/templates/diffviewer/diff_file_fragment.html
          reviewboard/static/rb/css/diffviewer.less
          reviewboard/templates/diffviewer/changeindex_entry.html
          reviewboard/templates/diffviewer/view_diff.html
          reviewboard/templates/diffviewer/diff_fragment_error.html
          reviewboard/templates/diffviewer/changeindex.html
          reviewboard/static/rb/js/pages/views/diffViewerPageView.js
      
      
    2. 
        
    david
    1. Ship It!
    2. 
        
    chipx86
    Review request changed
    Status:
    Completed