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

Review Request #4450 — Created Aug. 13, 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