28 Commits

Author SHA1 Message Date
Viktar Donich
29e1ce5e84 Collection of prospective test flake fixes
Potentially related:
https://github.com/Polymer/web-component-tester/issues/505

Bug: Issue 5792
Change-Id: I9ab6e8e40d9811dd52906335426764c052907609
2017-03-30 13:46:58 -07:00
Becky Siegel
0727efa3d1 Implement isOnParent as functions rather than an attribute on comments
Previously, the rest api interface set '__isOnParent' for comments.
When comments were added, the property from the comment thread
'isOnParent' was passed as a property. When the draft was saved, it was
expecting '__isOnParent' rather than 'isOnParent' and this caused
comments to show up on the wrong side after saved/refreshed, because
'__isOnParent' was undefined.

Instead of changing to either '__isOnParent' which would be strange to
pass as an attribute or 'isOnParent' which would look like it came from
the api directly, this change introduces isOnParent functions so that
translation doesn't need to be done with the API.

Bug: Issue 5831
Change-Id: I3b849ba5878275cda0a39638626a12bd51341a29
2017-03-21 14:57:58 -07:00
Becky Siegel
cd2d4232f1 Reduce confusion between side and commentSide
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
2017-03-15 13:50:21 -07:00
Becky Siegel
6cd5590a15 Fix local storage with PARENT revision
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
2017-03-13 15:03:09 -07:00
Viktar Donich
a2cfc65301 Save comments on Ctrl+Enter and Meta+Enter
Feature: Issue 4997
Change-Id: Ia1887b3ee558b96182e3ea987370c9f555c90502
2017-02-28 11:16:39 -08:00
Becky Siegel
2c602323e1 Move reply buttons to comment thread
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
2017-02-09 16:07:32 -08:00
Becky Siegel
41bdc04cb8 Don't merge threads on same line left/right
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
2017-02-07 21:00:52 +00:00
Kasper Nilsson
06c374d500 Add resolvable comments checkbox
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
2017-01-11 12:57:56 -08:00
Kasper Nilsson
8d1ac7e824 Display comment resolve state
This change tracks and exposes the resolved state of a comment thread
without exposing the UI for modifying that state. This enables features
to be built out while the API request does not exist in the backend.

Feature: Issue 4879
Change-Id: If002035024920a7762519cedf5a869221bbbc3c8
2017-01-09 20:21:48 +00:00
Becky Siegel
43c5ed8045 Fixes comment issues with multiple editing comments
This change addresses a few issues that existed due to multiple comments
in an editing state at the same time.

1) Fixes issue where if you create two replies and add text to the
second reply, then delete the first one, the text in the second textarea
gets removed.

2) Fixes issue where if you reply and add text, then reply again with
the first draft still editing, the second draft gets populated with the
message from the first comment.

3) Fixes issue where if you have multiple replies and delete one of
them, local storage gets erased. This change sets local storage to the
value of the first editing message found after deleting the other one
(if it exists).

Bug: Issue 4409
Change-Id: Ib5913a34a79783a4a87b4a298e25b02fc587b8dd
2016-12-28 20:18:41 +00:00
Becky Siegel
35a7682262 Add robot comments to PolyGerrit
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
2016-12-15 11:20:25 -08:00
Becky Siegel
f3a9894575 Remove draft comment from local storage upon discard
Previously, when a user discarded a draft comment, the draft was still
stored in local storage. If they tried to reply to a comment on the same
line, the previous message appeared when it shouldn't have. This change
removes drafts from local storage when they are discarded.

Bug: Issue 4361
Change-Id: Id234676972a21a879e68e754968abf7008098cf6
2016-12-01 13:38:05 -08:00
Aaron Gable
5be3252ead Add an 'Ack' button next to the 'Done' button
Bug: https://bugs.chromium.org/p/chromium/issues/detail?id=622838
Change-Id: Ie7db71615e2c657bbf24847cd8e6e2fc7606f5a9
2016-11-30 09:46:48 -08:00
Wyatt Allen
4f1b5c2dc9 Wiki-like formatting for PolyGerrit comments
This change is the PolyGerrit counterpart to [1].

Nicer rendering for reviewer comments in PolyGerrit using the Gerrit
Wiki-like format. Whereas, formerly, PG comments were set in PRE blocks
using monospaced font so that the original format and alignment of the
comment can be directly viewed. This change allows comments to default
to a variable-width font with wrapping while separately styling blocks
intended to be pre-formatted text, quotes and lists.

The logic to parse comment text into blocks is borrowed from the Java
implementation found in [1]. Test cases are additionally translated from
this change to ensure coincident behavior.

Introduces GR-FORMATTED-TEXT to display these comments, and which uses a
similar interface to GR-LINKED-TEXT. Much like [1], the comment is
parsed into a list of blocks. These blocks are then mapped to the DOM
nodes that get attached inside the element.

[1] I8e11d363b80bff0b6395f56e210b636f68db36fa

Feature: Issue 4861
Change-Id: I245d6782e2fd8982ac3eda438fe4ca80f3658195
2016-11-17 09:08:00 -08:00
Kasper Nilsson
3e297048c8 Escape only closes an empty comment box
Previously, the escape key closed and cancelled any in progress comment.
This was particularly painful for some who were used to a certain text
editor, so now escape only closes an in-progress comment if it's empty.

Bug: Issue 4816
Change-Id: I2749a995ebf914c4387261c387b008f6ce91dd86
2016-11-02 16:31:52 -07:00
Urs Wolfer
a010547eef Fix issues detected by 'JSHint' and 'JSCS'
Change-Id: Id16d7abe53d5f65c97bf778dc532e404b41283d8
2016-10-20 20:41:00 +02:00
Becky Siegel
eb4ea184f7 Keyboard shortcuts to expand and collapse all comments
This change adds keyboard shortcuts to the "gr-diff-comment-thread"
expand all comments when 'e' is pressed and collapse all comments when
'shift + e' is pressed. Note that the keyboard event is detected on the
thread instead of the comment to minimize the number of events getting
triggered.

Feature: Issue 4738
Change-Id: Iab77349bd1527d7af5e05a827919a78a86909835
2016-10-12 11:13:56 -07:00
Becky Siegel
6bf4e4f14e Allow for comments to be collapsible
Previously in Polygerrit, comments were always expanded. You could
always see the full comment (if multiline) and any applicable actions.

This change creates a collapsed comment view.  It adds a preview of the
text to the header row when collapsed, and can be toggled open when any
part of the header is clicked.

Bug: Issue 4698
Change-Id: Idca5caf92eb32518b6737dbb5a3380d227513996
2016-10-06 15:28:42 -07:00
Kasper Nilsson
b852326209 Add ctrl+s as keyboard shortcut to save comment draft
Shortcut only functions while editing the actual text area of the
comment. This makes sense, as one should have to save each individual
comment as a draft. This also mimics the current behavior of GWT UI.

Bug: Issue 3848
Change-Id: I7037d3dbdc5dae3048fcae580ed09c41e54f0669
2016-08-24 09:58:42 -07:00
Viktar Donich
bed05854a7 Cancel comment updates on discard
Fixes race condition triggering re-rendering of a deleted comment by
canceling debounced updates for `comment.editing` changes.

Bug: Issue 4319
Change-Id: I2a7578443783360088d6f65decd7dcb8be5e636b
2016-08-05 15:45:22 -07:00
Wyatt Allen
d0dd392794 Establish annotation pipeline
Apply diff annotations (intraline differences and comment ranges) by
executing the annotation layers in order to each line. The diff builder
maintains an ordered array of annotation layers which are communicated
to GrDiffBuilder subclass instances. The builder also listens to each
layer for notifications that annotations have changed for some line
range and re-renders (i.e. re-applies the pipeline on DIV.contentText
elements) accordingly.

Change-Id: Iea0599d4869cafaadc0974158153a91d927913e8
2016-07-20 12:25:40 -07:00
Viktar Donich
b2198e8233 Ranged comments integration
- 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
2016-06-13 22:44:47 +00:00
Urs Wolfer
9b8b237431 Fix 'gr-diff-comment' test when run multiple times
The test worked fine on CI since they are run with a clean profile.
When tests were run in a normal browser profile manually, tests
'draft creation/cancelation' and 'draft saving/editing' failed
alternately because of local storage use for saving draft comments.

Since these tests do not test local storage (yet), we can just create
a stub for 'gr-storage#getDraftComment' which simulates a clean state.

Change-Id: Ibb63be4f1c395d47e0022f63d0d2ba0a7c87b137
2016-06-05 12:20:11 +02:00
Viktar Donich
7ad28920e9 Gr-diff retrofit: store comment state
Fixing UI data pipe line, re-rendering:
 - side, draft text and editing status in UI comment objects
 - update gr-diff UI model on comment save/update

Feature: Issue 3910
Change-Id: I96f714c7de9add6e316dcf64bb7d566690b9d3ae
2016-05-27 10:46:54 -07:00
Wyatt Allen
035c74f508 Cleanup of diff comment draft saving work
- Refactors the _editDraft property of gr-diff-comment to _messageText.
- Renames the gr-storage element as instantiated in gr-diff-comment from
  "localStorage" to "storage".
- The gr-storage diff comment functions are refactored to include
  "comment" in their names and to accept a location object rather than 4
  separate arguments.
- Added a comment describing the use of a promise in the _loadLocalDraft
  method of gr-diff-comment.
- Throttled the invocation of the _cleanupDrafts method of gr-storage to
  avoid potentially expensive crawls over all localStorage entries.
- Various other smaller fixes.

Bug: Issue 3787
Change-Id: Idf96051c0d56d6ce8b15f55ca2680363bd1ca805
2016-05-23 14:12:19 -07:00
Andrew Bonventre
180693c180 gr-request/gr-ajax cleanup (gr-diff-comment)
Bug: Issue 3988
Change-Id: I0c100a0b77a256c1eb34f94e8fff0515add5dea1
2016-05-04 12:25:15 -04:00
Andrew Bonventre
6a9312f7b1 Properly remove discarded comments from model in gr-new-diff
Change-Id: Icb5d3ba1edd2cd3a75aa6568a152bacc5e0babda
2016-03-25 16:17:06 +00:00
Andrew Bonventre
78792e8e98 Refactor directory structure of components
There is no change in functionality. Only moving things around.

+ Separate html from the js.
+ Place the unit test for a component within the same folder.
+ Organize the components in subfolders.

Change-Id: I51fdc510db75fc1b33f040ca63decbbdfd4d5513
2016-03-04 18:02:24 -05:00