Previously, if a PARENT revision was getting compared to a patchset,
the local storage key would be the same for both, and there were issues
if a user was trying to write drafts on the same line on either side.
This addresses the issue by storing 'PARENT' as the patch number in the
local storage key, so that each key is unique.
Bug: Issue 5412
Change-Id: Ia8b2f0abe1d24a3849628fe335c931f07bcaff52
Formerly, nonsensical comment ranges would prevent comments from
appearing in diffs. With this change, ranges are normalized by the
ranged comment layer so that they can be translated into valid
annotations.
See also
* Ibcab6e537abe8b81e764b09982a2581ae81463f8 should reject such ranges.
* I019e00063ab5c45c99379f3f9fb74eda0408d63f should avoid this error when
constructing comment emails.
Bug: Issue 5744
Change-Id: Ib5834017f81f81a877b32264ee57d72302911a6c
Previously, we did not scroll to target in the cursor manager if the top
was visible. However, there were times where the bottom content was
not visible, and it could have moved up into view.
This change passes an optional function for calcuating target height to
the cursor manager. If it is passed, the function is used to calculate
height instead of targetHeight. Height is ultimately used to determine
if the bottom is visible.
If the top is visible, but the bottom is not, scroll to the target if
the scroll position is farther down than the current position. Don't
scroll if the condition is not met because more content related to the
target is actually visible without scrolling, and do not want to reduce
it.
Bug: Issue 5498
Change-Id: I1708c921093b6e8b1916ae68fb468816f7c67633
Removes the timezone from the timestamps, as the tooltip shows UTC
offset on mouseover.
Also upgrades most instances of gr-date-formatter to use gr-tooltip.
Bug: Issue 5587
Change-Id: I57c7dcacf0618ffd967eff3cb4ff37a5e1876180
There was an issue where gr-formatted-text would not display when the
config was not loaded yet. This change adds a function to display text
as is, in the event that the config is not yet loaded. It also refactors
the existing functions to make it more clear where config is needed.
Bug: Issue 5690
Change-Id: I74896bd59793b26b2b8fe289c13e5762c60fe8df
Previously diff cursors initialized when their diffs completely finished
rendering. This means waiting longer than needed because the diff render
process includes processing and rendering syntax highlights whereas the
diff cursor only needs to wait on the content of the diff.
With this change diffs fire an event when they've finished with their
content only, and diff cursors await this event instead of the full
render.
Bug: Issue 5681
Change-Id: Ida9da83a20fe4c2dcd8920304504ec7e1185eb5d
The logic inside the syntax layer that decides whether the next step of
the processing is done synchronously or after a timeout would not choose
to use a timeout if it is processing the zeroth section of the diff.
For diffs with very large initial shared chunks (for example a chunk of
more than 20,000 lines as linked in the bug) this results in the syntax
layer processing the entire chunk using synchronous recursion.
As a result, the (1) UI would lock up while processing the syntax for
this chunk, and (2) when rendering all diffs on the change, the call
stack would be exceeded.
With this change, the syntax layer allows asynchrony when processing the
zeroth chunk of the diff.
Bug: Issue 5654
Change-Id: I0e60479b2c59c9c626199e7a6b8d63ccb55ebaa7
Previously, the logic was only checking exact line wrap boundaries and
missed tabs advancing over the boundary.
Also, line breaks were added inside span elements wrapping tabs, which
prevented correct rendering.
Bug: Issue 5597
Change-Id: I750547cb574c02965d5a30ba57f791841779297a
HLJS emits a syntax class for function parameters which the PolyGerrit
syntax stylesheet would color blue. However, HLJS did not always apply
this class accurately, for example, in the C++ case described in the
linked issue.
Because the class was not very informative anyway, and the HLJS default
stylesheet does not style it either, this change removes it from the
PolyGerrit syntax styles.
Bug: Issue 4975
Change-Id: I26ed0b8f745ac6add994a5d1cfc8eb1303dac8cf
Unresolved state is only shown when the comment actions (reply/Ack/etc)
are shown AND when the thread is unresolved.
Feature: Issue 5442
Change-Id: I7e0859530198fa17172e9f9efa73a20c7aa03975
In a recent change (Ib83ff5157), the promise strategy for processing and
rendering diffs changed so that they resolve after all the work (i.e.
processing, diff content, and syntax highlighting) had completed.
Beforehand the promise would actually resolve after the diff processing
had finished. The diff view would wait for this promise to resolve
(along with the resolution of network calls) before removing the
`_loading` flag.
Because the promise resolution now represents all of the diff rendering
work being complete, rather than the first step only, this use of the
promise was outdated.
With this change, the diff view sets `_loading` to `false` immediately
before starting the diff processing/content/syntax pipeline.
Bug: Issue 5533
Change-Id: I24fafe4d55166e2acbf1849e6a75fbcba122b997
Previously, there was an issue where if you create a draft comment in
side by side view and switch to unified view, the comment thinks it's in
the later patch set rather than the earlier one and a second copy gets
added to local storage with the later patchset as a component of the
key.
This was because the the thread group assumed all threads inside of it
had the same patch number. This change fixes that, so in the event that
a user switches from side by side to unified, the patch number will get
taken from the comment rather than the thread group.
Bug: Issue 5493
Change-Id: I7f00997bcb2e6f1001a5d58ac206acf5af3367d2
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
Currently, when a response is created via keyboard shortcut 'c' or
clicking a line number, the comment is not created with the unresolved
state of the last comment in the thread.
This change checks for the previous state and adds that to the new
draft.
Bug: Issue 5408
Change-Id: I20eb039864120d5175cc016bfc695da564bc174d
At the moment, gr-diff-highlight react to selection change event, so if
there are multiple instances on the page, for example in gr-change-view,
all instances react to selection.
This change makes gr-diff-highlight to ignore selections for other
instances, thus reacting only its own selection.
Bug: Issue 5504
Change-Id: I9f0e1dc7fe9f316400d9c96fb0c8c5f7ff78d779
In some cases, the reply dialog could be opened before all comment
drafts have been saved, causing the draft to not appear in the dialog.
This change maintains an array corresponding to each draft request and
refers to it before opening the reply dialog. If the array is populated,
a non-blocking alert is shown with the text 'Try again when all comments
have saved.'
Bug: Issue 5369
Change-Id: Ieb73e7d7b4f66daff6cc2278a84c2195b7d0e541
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
Double-clicking selects the word underneath it on OS X and Linux text
editors. Triple-clicking selects the whole line.
Selected line can be copied or commented on.
Feature: Issue 5375
Change-Id: I79fff57e7dbfde18ab741a3a67b61869fb52cecf
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
- Adds range when parent exists when comment POSTed
- Finds range via filtering comments for backfill
Bug: Issue 5459
Change-Id: Ied19cc4b33749ba81fad6d1a5030abab419b1e6f
- 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
Previously, drafts were only based on line number and not patch range.
This adds range to the key in localstorage so that multiple drafts can
be stored for a line range. It also prevents the wrong draft from
surfacing in the incorrect context.
This change is also important for the multi-thread changes that are
currently in development as well, because it is possible to have
separate threads for these ranges, each of which can have its own draft
saved in storage.
Bug: Issue 3548
Change-Id: Id4c1beb5d73a47a1f98b0b169091905c80f8c64a
Because the `_computeAccountTitle` method was only used in a binding to
an HTML attribute, the manual HTML escaping performed in that method was
in not needed.
Since removing this, the only use-site for the `util.escapeHTML`
function is the `GrDiffBuilder`. To discourage general use of manual
HTML escaping (in favor of default escaping in Polymer bindings) -- as
well as to lighten util.js -- the escapeHTML function is moved into
`GrDiffBuilder`, its regex is made a constant, and is given a unit test.
Change-Id: I28c9f546cf50461e96995ecd6da8653e75554023
In some cases, even though the copy selection was a valid one, the
target is something existing outside the content element. One specific
instance of this is when selecting lines by clicking/dragging along line
numbers.
Bug: Issue 5325
Change-Id: I3496a9d2432201aae6ef43a63ccca601a3bcd309
Adds keyboard shortcuts to the diff view to navigate to the next or
previous file in the change's file list that has comments in the current
patch range.
Feature: Issue 5235
Change-Id: I1ad39089c1ac227e335093f25b72311f7e98b3f7
Occasionally comment threads would render in such a way that the bottom
border is covered by the following diff line. Giving 1px of margin below
the thread elements makes the border consistent generally.
Bug: Issue 5083
Change-Id: I3f29905d1dc388d2d160444c16f09ddeaaec36b5
Adds the 'resolved' checkbox to the front end. Unresolved comment
threads are indicated by #fcfaa6 as background-color, whereas resolved
threads have the background #fcfad6.
Feature: Issue 4879
Change-Id: Ie1eeba61ccba559f89b707542acab2198c99b8a7
Include workarounds with bug references for a pair of simple HLJS bugs
related to parsing character literals.
* In Go, HLJS misunderstands backslash character literals.
* In C++, HLJS misunderstands wchar_t character literals.
Bug: Issue 5007
Bug: Issue 5242
Change-Id: I4c92b254062198dbf043ccd401e3224961a84a33
This change leverages the unresolved flag to automatically expand only
the last UNRESOLVED_EXPAND_COUNT comments in unresolved threads.
Feature: Issue 4752
Change-Id: Ia23920e1a210246838645d56a6bc81d0dff7da07
