With this change a blame column is added to the left side of diff
tables. The column is empty and hidden until blame is loaded. A button
is added to the change view to trigger a load of the blame for that
diff, as well as a unload it if already loaded. In this stage, the blame
information is non-interactive and only displays the SHA, date and
commit author.
Feature: Issue 6075
Change-Id: Ifcb951265d0e6339094e6b7c9574ec9c69e60b51
This appears to have broke in I0428fead7a9d1f1dc5d6aa9efc3d81ecbe6b5c64
The computation to get the comment thread used to pass the range object,
but stopped doing so in the above change. This change adds it back in
and also separates the range string computation so that additional
unit tests can be added.
Bug: Issue 7081
Change-Id: I846d0c24b894fa0577f4fefaf8dcf4fc751daa15
Formerly, if a formatted text component tried to render without the
project config (used by inner linked text components) it would
temporarily fall-back to rendering the unformatted (and un-linkified)
text via `.textContent` -- mirroring the behavior of gr-linked-text.
The result is formatted text elements (when rendered without a project
config) appear as one long line of text. Unlike linkification, however,
text can be accurately formatted with or without the project config --
so this disruptive, poor UX is unnecessary.
The formatted text component is updated to format text when the project
config has not provided, and to re-render when the config has been
provided.
In order to propagate project config loads to the formatted text
components hosted by diff comments, the REST calls must be made by the
diff thread component. To make this call, the thread must have the
project's name, so gr-diff-builder is updated to provide this name to
thread components.
Bug: Issue 6686
Change-Id: I8d09c740930500e99cb5f87b92f4d72f3f50a9ce
Formerly, gr-diff would make its own REST calls for diff comments in the
patch range and path being viewed. However, these calls by the diff view
were redundant to the more general comment requests made by either
gr-diff-view (which requests all comments to support skipping to the
next file with comments) or gr-change-view (which requests comments
individually for each inline diff).
With this change, the diff component no longer loads comments for
itself, but is rather provided the comments from its host view via a
public property. In this way the host view can load the more-general
comment data, and filter it down for the diff view's params.
Introduces the gr-comment-api component to simplify loading and
filtering diff comments for use by diff views. By using this new
component:
* The diff view makes only one request for comments (including drafts
and robot comments) to support skipping to the next comment and
displaying comments in the diff view.
* The change view makes only one request for comments for all inline
diffs.
Bug: Issue 5299
Change-Id: Ia14a01619f1f9881aa0d253fd3f49af9a17f3dce
If the user views a very large diff while "Whole file" is enabled in
their preferences, rendering may lock up their browser. With this
change, instead of rendering, users are warned and allowed to bypass the
warning (risking browser lock up) or render with a context that is less
than whole file.
Bug: Issue 6402
Change-Id: I6e97c06598fb5f6900c925127ab6a99693b8aa7f
Previously, when a diff was reloaded, it would clear the diff content
(the previous diff) and open requests for the data regarding the new
diff. When the requests finished, it would cancel async processing and
start it again on the new data. In this arrangement, a narrow race
condition was present where the async rendering could emit a diff from
the previous diff after the diff table had been cleared, but before the
network requests resolved -- resulting is content form the previous diff
appearing in the new one.
With this change, the async processing is cancelled at the same time
that the diff content is cleared.
Bug: Issue 6170
Change-Id: Ie5a807082ab51156595f9eeb9ba6c958d41e890a
Inline scrollbars are invisible until scrolling starts, or, for
ChromeOS, scrollbar area is hovered.
When scrollbar appears, it overlaps last line of diff.
This change adds a padding to prevent scrollbar overlapping last line of
diff.
Bug: Issue 5964
Change-Id: I072f8e8a4d4d2750f122bc6177b8db492b258f5f
Noteworthy decisions:
- Preferences are hidden when diff prefs are not loaded or the user is not
logged in.
- Preferences are hidden on small screens
- The trigger button is in gr-change-view but the gr-diff-preferences
is part of gr-file-list. This is because gr-file-list because diff
preferences and local preferences are more closely tied with that
than the change view. In order to put it in the change view, local
prefs would also have to be two-way bound back.
Also fixes computePrefsButtonHidden in gr-diff-view as well. The
function did not work as intended before. If preferences didn't exist,
the function would not get called, and the container would not be
hidden.
Bug: Issue 5426
Change-Id: I361cdf132c6e15b5ae2f15e62af318cfa05161ce
There was an issue where line numbers would not properly highlight in
inline-diffs rendered in the file view.
There were two separate issues that contributed to this problem:
1 - The diff cursor was not notified of the line being selected and
needing to move to that cursor stop.
2 - The diff cursor was not properly updated after being moved inside
a dom-if statement.
Additionally, the way that line selection was previously done was not
extendable to a diff cursor that stored multiple files. It queried
stops based on line number and side. This change also stores the
path of the file in the cursor stop so that the correct diff/line is
selected in the case of multiples.
Bug: Issue 5977
Change-Id: I7496293e19a4e59a3855dc78e273de6a9852e556
Adds the header section to the diff. The header is only displayed when
relevant information is contained within, e.g. when a file is renamed
but otherwise left unchanged.
Feature: Issue 5752
Change-Id: I5f8cb56522decd8d3c57bb403cb43f87203d5c6a
Show "Error: not logged in" error toast with a "Log in" link when one of
the following actions taken while not being logged in:
- reply hotkey pressed (A)
- create comment hotkey pressed (C)
Bug: Issue 4818
Change-Id: I4ceaf12888b8cb2aae9da7ab2fdd47eb409dd984
Use '__isOnParent' as a boolean in place if 'side' ('PARENT vs
'REVISION'). In doing so, it's necessary to convert to/from 'side'
whenever interacting with the REST API.
Change-Id: Ic023c9be1969597e4b9c73a51cfed9f5eb9bc23e
Move all buttons that generate a reply of some sort (done, ack, reply,
quote) to the comment thread instead of the comment [1].
When there is a draft for a particular comment thread, all reply buttons
are hidden [2]. For example, if you click reply, you cannot click done
on the same thread, unless you remove the draft.
Each thread can have up to 1 draft. It's also worth noting that if a
thread has a draft, and the user clicks on the line or 'c' at the same
range, the existing draft will switch to 'editing' form.
[1] With the exception of "please fix" for robot comments.
[2] In this case, The please fix button will be disabled when other
reply buttons are hidden.
Feature: Issue 5410
Change-Id: Id847ee0cba0d0ce4e5b6476f58141866d41ffdad
Changes with large numbers of files could overwhelm PolyGerrit when a
user selects [Expand all] for inline diffs. This was because the
asynchronous processing/rendering/annotating process would be kicked off
for all unexpanded diffs simultaneously, resulting in browser lock-up
and general slowness even after rendering had completed.
With this change, inline diffs are rendered in serial rather than
parallel. In this way the benefits of the async features of diff
rendering extend to the file list, even for changes with many large
diffs (such as the one in the linked issue).
With this change, the `__expanded` property is removed from file objects
in GR-FILE-LIST. Instead, that element maintains a list
(`_expandedFilePaths`) which records the same information. Because the
expanded files are recorded in a list, however, splices on the list can
be observed, batch diff expansion can be handled sequentially.
Tests are updated to respect the new expanded paths list.
Bug: Issue 5396
Change-Id: Ib83ff5157177e1c890db8a82fbc25df8fecbe065
Goes along with c/95273/. Adds commentSide attribute to comments to see
which side of the diff view they belong on. This is also used as part
of the locationRange for the gr-diff-comment-thread-group, so that two
thread groups can be on the same line or range for the unified group (
one for the right, one for the left).
Note: there is already a 'side' attribute on the gr-diff-comment, which
is confusing. This side actually references 'PARENT' or 'REVISION', to
identify whether the comment belongs to the parent or any revision. On
diffs where two revisions are compared to each other, this cannot be
used to determine left/right. However, because 'side' is part of
the CommentInfo entity[1], it is difficult to change the name and make
more sense out of that.
[1] https://gerrit-review.googlesource.com/Documentation/rest-api-changes.html#comment-info
Bug: Issue 5114
Change-Id: I5cc4c17d4bb134e31e5cc07ff9b08ed349488c97
- Add concept of diff comment thread groups, which are all of the
threads at a particular line number.
- The thread group is responsible for breaking up comments into threads
based on the range of the comment.
- Thread groups are ordered by the updated time of the first comment in
the group.
- Thread groups are given a key, based on comment range, which is used
to determine what thread group a new comment should go in (or if it
needs a new one).
Feature: Issue 5292
Change-Id: If544e8bb879262de3ce5397e86124837b66ada04
This change adds an API request to get robot comments for displaying
inline in the diff view. They are styled in a different color, contain
build and robotId information, and a "please fix" action rather than the
standard set of actions.
Feature: Issue 5089
Change-Id: I1f5954a2ed01920bb7c3dc897e3285687ff7d3ca
Previously, the line marker was only subtly visible by the highlighted
line number. This change adds a bottom border to the selected line if
the user is using keycodes (j, k, up, down) to more the cursor. When
the escape key is pressed, the distinguished line marker will dissapear.
Feature: Issue 4739
Change-Id: If8c751efc137ef87cfdad1c8bf7d905de1219107
Previously in Polygerrit, diff views were always displayed in the width
specified in diff preferences. This change gives the option to wrap
lines instead, which takes precedence over column width (the column
width option is hidden when line wrapping is selected), and fits the
diff view to screen.
The gerrit API already supports the 'lineWrapping' preference so this
change uses that already existing option.
Feature: Issue 4809
Change-Id: I0d9e292739b5910abfd04af63ec4c745bf06e446
Added temporary safeguard for the font preferences change so that inputs
are hidden and custom styles are not set if font-size was not part of
the preferneces response.
Change-Id: I5b34b317bf953591889fa25e1c351d4f490d989e
If a file is too large for the server to deliver the diff, the API will
respond with HTTP 409. PolyGerrit interprets th this as a network error
and covers the entire page with an error message. Furthermore, the
handler for loading the diff is not written for this kind of failure and
tries to dereference the diff object -- null in that case.
With this change, the 409 message does not block use of the whole page
and the handler does not break when the diff is null.
Issue: Bug 4770
Change-Id: I68fe50c474fc03d4217e969649f62df38ca5b632
This change adds a font size field in diff preferences in both the
generic preferences page and also the preferences modal from the diff
view. The new field allows users to update their preferred font size
for the diff view to be displayed. This updates both the line number
font size as well as the diff font size.
Feature: Issue 4417
Change-Id: I0fb75fd2186d2ff63b3627e8b0f15f2b3cddbfa3
Refactors bindings in file lists to change the expanded state of a diff
using a property name that does not collide with the hidden attribute.
Change-Id: I7f7e38a910a8d4dd19be2d591033be6f6d3cb7a1
In https://gerrit-review.googlesource.com/c/86117/ the data binding
between the _files array and the dom-repeat in gr-file-list was not
properly enforced. This change fixes that, and also properly
references a static file-list property.
In addition, tests were implemented to ensure this regression is caught
in the future.
Bug: Issue 4575
Change-Id: I32a5645ec47d953ee6a7a71f4b14fc8c9483df3d
This commit enables the user to manually expand individual diffs
inline in the file list of the change view.
Bug: Issue 4382
Change-Id: I87d5af9971fed3aa5e1eb64523f4623a5ff2ac8d
Adds a polymorphic method to GrDiffBuilder subclasses named
`_getNextContentOnSide` which gets the a content element by traversing
from its preceding content on the same side. This method is dramatically
faster than the query-based method when diff sections are large.
In preparation for the syntax highlighting change, the gr-diff-builder
is refactored to use a property for the diff object, rather than
accepting it as a parameter to the `render` function.
Tests are included for new functions.
Change-Id: Ifd0edb530a303de2626d55a691c3ba1eaed6167c
Formerly, diff content elements mixed text with comment threads. For
example, a diff content node with an intraline highlight, a ranged
comment, and a gr-diff-comment-thread may have been organized as below:
TD.content
╠ #text
╠ HL (intraline difference)
║ ╚ #text
╠ #text
╠ HL.range (ranged comment highlight)
║ ╚ #text
╠ #text
╚ GR-DIFF-COMMENT-THREAD
╠ GR-DIFF-COMMENT
╚ ...
Note that the comment thread was inserted at the same level as the text
of the diff line.
With this change, the text is separated from the comment thread by
introducing a DIV to contain the text with class `contentText` as
sibling to comment threads.
TD.content
╠ DIV.contentText
║ ╠ #text
║ ╠ HL
║ ║ ╚ #text
║ ╠ #text
║ ╠ HL.range
║ ║ ╚ #text
║ ╚ #text
╚ GR-DIFF-COMMENT-THREAD
╠ GR-DIFF-COMMENT
╚ ...
Modifies the `getContentByLine` method of gr-diff-builder to return the
`DIV.contentText` element rather than the `TD.content` element which is
its parent. In most uses of this function, the text is what is needed
rather than the TD or comment thread, but in other cases, they can be
easily DOM traversed.
Change-Id: I0eded34afd3d22963252efc7eabfee290ae21a9c
Building on existing support for asynchronous diff processing, the
rendering stage is now also asynchronous. The `emitGroup` method of
gr-diff-builder (which constructs a DOM fragment and attaches it to the
document) is now called whenever the processor emits a new group, rather
than waiting for all groups to be available and looping over them.
Adds support for canceling the processor, and removes a behavior where
the diff was being re-rendered needlessly, causing visible flicker.
Updates a test that broke when rendering became asynchronous.
Change-Id: I37fcd65efc8c901b85fc93e91611c022edc10dc4
When a user taps a line number in a diff, the cursor is moved to that
line and that side of the diff. However, if the user taps the text
content of a diff, the cursor was not moved. With this change, the
cursor is moved to the appropriate line and side of a diff when either
is tapped.
Bug: Issue 4215
Change-Id: I47d24f678f487eb3f8173ea5572865a589d845e4
If the gr-diff-view is loaded with a line number address in the URL
hash, moves the diff cursor to that line.
Bug: Issue 4206
Change-Id: I359a29d7a3fd97b0ce72228aec3f49ae2295bfee
Sets the URL hash to the address of the cursor when a user selects a
diff line (either by clicking the line number or using the 'c' hotkey).
If the cursor is on a line of the revision, the address is the line
number. If the cursor is on a line of the base patch, the address is the
letter 'b' followed by the line number. Otherwise the address is the
empty string.
Bug: Issue 4206
Change-Id: Ic85da197eb6264ea1111cc34e781dccbc24d4d40
- gr-file-list recognizes local preferences (for hasRangedComments flag)
- gr-file-list reacts to cursor hotkey only if there is no range
selected (currently always false).
- Remove dead code from GrDiffBuilderSideBySide, GrDiffBuilder,
gr-diff-builder.html
- Bugfix: GrDiffBuilder.prototype.getGroupsByLineRange handles one-line
BOTH code sections correctly. Test updated as well.
- Added utitily methods added to gr-diff-builder.html to reduce
dependency on DOM structure and reduce amount of code copy-pasting:
- renderLineRange, getContentByLine, etc
- For gr-diff.js and gr-diff-comment-thread.js addDraft renamed to
addOrEditDraft because that's what it does.
- For both, addDraft method always creates a draft comment.
- Added support for ranged comments in gr-diff, gr-diff-comment-thread.
- Added mouseenter and mouseout events to gr-comment.js
- Refactored gr-comment.js to reduce code copy-paste, unify event
payload, and to eliminate need of accessing component instance for
patchNum. Tests updated as well.
- Refactored gr-diff.js UI data model update using gr-diff-builder.html
utility methods to make code more readable.
- Added support for creating ranged comments to gr-diff.js.
- gr-selection-action-box now reacts to click and tap to create a
comment.
Change-Id: I01480a4c6f460774a8b2826915702800b3f81d25
