Add a cleaner and more informative file index to the diff viewer.
Review Request #4450 — Created Aug. 13, 2013 and submitted
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
-
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
-
-
-
reviewboard/static/rb/js/pages/views/diffViewerPageView.js (Diff revision 1) Indentation is kind of funky here.
Change Summary:
* Reworked the _each(...) to iterate over all child tbodies, and handle the filtering inside the function, instead of doing it twice (once in jQuery, once in the callback). * Made the pie icon colors the same as the dot colors, and tweaked them a little bit. Note that the screenshot I posted makes it look as if the dots aren't perfectly round. This is something funky that Awesome Screenshot did, but is not how the browser looks (tested in Firefox and Chrome at various zoom levels).
-
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
-
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
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