Add support for generic text-based review, and improve Markdown review.
Review Request #5237 — Created Jan. 11, 2014 and submitted
Add support for generic text-based review, and improve Markdown review.
This adds support for reviewing any kind of text-based file attachment.
The UI is much like the diff viewer. We present the file, syntax
highlighted, with line numbers on the side. Users can comment on a line
or on multiple lines at a time.
TextReviewUI
and its JavaScript counterparts drive the whole review
process. They're also intended to be subclassable. The Markdown review
UI is now just a small subclass ofTextReviewUI
.
TextReviewUI
and friends can optionally present the content both as
raw source text and as rendered content. The user will see two tabs when
rendered content is available, and can comment on each the source text
or the rendered text. Markdown makes use of this. In the future, we
could add simple subclasses for other formats, like XML or JSON files.Since
DiffCommentBlockView
andTextBasedCommentBlockView
are basically
the same, I've moved most of the logic intoTextBasedCommentBlockView
.
DiffCommentBlockView
is now just a simple subclass.There's also a few fixes for rendering thumbnails, extra data for review
UIs, and a couple new blocks to give custom review UI templates some
additional control.
-
Tested viewing several types of plain text files (Python, diffs, XML files, and a README). Plain text files only had the one view, and the tabs were hidden.
-
Tested viewing Markdown files. Saw the two tabs and switched between them freely.
-
Tested making comments on source text and, for Markdown, on rendered text. The comment flags were properly showing on their appropriate tabs. Loading and saving worked fine.
-
Tested the comment thumbnails for source and rendered text in the review dialog and in reviews.
-
Tested that the links in reviews pointed to the correct line on the correct view mode for the correct file.
-
Tested comments on the first line and last line of files.
-
Tested that commenting on diffs still works.