[WIP] Core functionality for file diff collapsing
Review Request #9431 — Created Dec. 9, 2017 and discarded — Latest diff uploaded
When viewing the diff of a review request not all files need to be
viewed, for example minified files don't need to be reviewed. It is
also helpful to have a way to hide files you have already reviewed.Now files can be collapsed and expanded as the user wishes. Also,
minified files are collapsed by default.If a minified file is also a deleted, moved/copied, binary (I'm not
sure if this is even a possibility), or a new empty file then it will
not be collapsed by default because there will just be a short
message displayed instead of a large, one-line diff.Collapsing a diff does not work especially elegantly when the file only
contains whitespace changes. In this case the Hide/Show whitespace
changes button can also be used to collapse the file. So the file
can't be viewed unless both buttons allow it to be. One fix for this
would be to hide the new button whenever a file is collapsed with the
Hide/Show whitespace changes button. (This fix can be done in a
separate review request)A few small changes were also made related to chunk highlighting.
Previously the minimum height of the highlight was 1, this was changed
to 0 because when a chunk was highlighted then its file was collapsed
there would be a 1px tall highlight left behind.Also, previously collapsing a file with either button would leave the
highlight in the same position. Now it will be updated to match the
shifted position of the chunk whenever either button is used to
collapse or expand.
A change I made to renderers.py had caused an error in the test
test_make_context_with_chunk_index
, but this has been fixed now.I have added tests to verify that the
minified
property added to
diffutils is working correctly (returns true if
depot_filename
contains .min., and false otherwise).Unit tests have also been added to verify that the new context
variablecollapse_file
is set properly in various situations.
A couple of the new tests
test_make_context_with_collapse_file_using_unchanged_file
and
test_make_context_with_collapse_file_using_file_with_no_chunks
may be unnecessary, because I'm not sure if a file can have
'num_changes': 0
without having'moved_or_copied': True
or'num_chunks': 0
without'newfile': True
Two new unit tests have been added to verify that the new onClick
event properly updates the classes of the button icon and tbodies
related to that file.Preferably, additional unit tests would be written to verify that
the onClick event updates only the classes of elements related
to the desired file. I don't think this will be difficult, but
right now I'm not sure how to do it without doubling the size of
the html template indiffReviewableViewTests.js
and possibly
needing to adjust many of the other tests in that file.
This is the only reason the review request is still WIP