57 Commits

Author SHA1 Message Date
Dave Borowitz
8cdc76ba4c Add @license tags to PG HTML and JS assets
These tags are preserved by the Closure compiler and vulcanize in order
to serve the license notices embedded in the outputs. In a standalone
Gerrit server, these license are also covered in the LICENSES.txt served
with the documentation. When serving PG assets from a CDN, it's less
obvious what the corresponding LICENSES.txt file is, since the CDN is
not directly linked to a running Gerrit server. Safer to embed the
licenses in the assets themselves.

Change-Id: Id1add1451fad1baa7916882a6bda02c326ccc988
2018-03-26 10:47:55 -04:00
Becky Siegel
fc18085eeb Update gr-diff-comment unresolved toggle
This changes updates the unresolved toggle so that it only attempts to
save when explicitly tapped (as opposed to the value of resolved
changing on its own).

This was causing issues with comment synchronization between
gr-thread-view and diff comments, and also re-rendering the order of
comments in gr-thread-list within the dom-repeat.

This change also lets the resolved checkbox get selected via the enter
key.

Change-Id: I6065c745a0eabcf19df38f3cd6fd2c0d7e7434f3
2018-03-11 20:59:54 +00:00
Wyatt Allen
0b2144b367 Allow convergent comment saves
Formerly, the save button would be disabled when attempting to edit a
comment to have the same content as what's already been saved. This can
be a confusing UX where it isn't clear why a comment cannot be saved.

Allow saving comments even when the text has not been changed.

Bug: Issue 8128
Change-Id: I615c38e0cc109ea87f58c9b8a9b7de28ec459b88
2018-01-18 14:51:41 -08:00
Wyatt Allen
4f09a3f50c Merge "Restore confirm discard dialog for draft comments" 2017-12-08 20:04:56 +00:00
Kasper Nilsson
ed4e5f1f88 Modify resolved checkbox to call save()
Previously, the resolved checkbox called _saveDraft(). Using save()
instead disables the comment while waiting for the save request to
return.

Bug: Issue 7933
Change-Id: I44ce88094b271aa46a726a75aa4eb283b29f1a35
2017-12-06 13:39:52 -08:00
Kasper Nilsson
bc6f579528 Restore confirm discard dialog for draft comments
Draft comments with message text should have a confirm dialog when
discarded to prevent accidental data loss.

This is a regression introduced by I0dae814.

Bug: Issue 7900
Change-Id: Ibd9829ccf36a551445282e5cb0d37c1a6df46ab9
2017-12-05 14:33:13 -08:00
Kasper Nilsson
46a6c8eca2 Allow toggling resolved outside editing state
A recent change modified some comment editing UX. As a consequence, the
save button disabled state was erroneously based entirely on whether or
not the comment message had changed.

With this change:
- Save disabled state on a comment takes into account resolved state
- The resolved checkbox is always visible on an expanded draft comment
- When the resolved checkbox is modified on a non-`editing` draft
  comment, it is saved immediately

Bug: Issue 6023
Change-Id: I40b0b280129e5a76c13e38c97525f91a8e56482d
2017-12-05 13:20:22 -08:00
Kasper Nilsson
80ca288da5 Delete draft comment when saved with empty text
Recently, the comment editing UX was changed (the discard button was
removed from the editing view). This (obviously) made it impossible to
discard a draft comment whilst editing it.

Editing a comment, clearing the text, and saving now removes the
comment.

Bug: Issue 7832
Change-Id: I0dae814b9ce2e7a5b57530f839c708e59c215341
2017-11-27 16:37:11 -08:00
Kasper Nilsson
998e4cf5b7 Clean up comment editing UX
The removal of the 'Discard' button from comments in the editing state
complicated the UX of the 'Cancel' button a bit.

Previously:
- Discard deleted the draft completely.
- Cancel cancelled whatever edits were made, and reverted to the prior
  draft state. Cancel was also hidden on new drafts.

Currently:
- Cancel cancels whatever edits were made, and reverts to whatever edit
  was last stored.

This behavior is problematic, for a few reasons:
- Due to some vestigial code, the actual value of the comment's message
  (not just the buffer value '_messageText') was updated any time the
  draft comment was edited. This was done to prevent multiple drafts
  from being created on the same thread -- that issue was made obsolete
  by the removal of buttons from individual comments, and their
  consolidation to the thread level.
- If a user cancels a draft, then restarts it (having their text
  repopulated from localstorage), then cancels it again, the draft is
  rendered as a saved draft comment with the localstorage value, despite
  having never been saved at all.

So, with this change:
- Cancel cancels whatever edits were made.
- If a user tries to add a draft in the same place...
  - ...and the draft has been saved on the server, the textarea is
    prepopulated with the value from the comment received from the
    server.
  - ...and the draft has NOT been saved on the server, the textarea is
    rehydrated from localstorage.

This UX makes sense because...
- Case 1 occurs when a user clicks "Edit" on an existing draft comment -
  thus expecting to edit the content shown in the UI.
- Case 2 occurs when a user cancels a draft that they started
  constructing, but did not save -- thus there is no suggestion of any
  prepopulated value by the UI, _and_ the case in which a draft was
  erroneously cancelled during editing can be solved.

Bug: Issue 7709
Change-Id: Iacf9a91fbb0226aba3a518f56edd61f28c15a8ef
2017-11-09 11:38:59 -08:00
Wyatt Allen
5eab49acc0 Show comment saving progress indicator inside comments themselves
Bug: Issue 7596
Change-Id: I194bce633c8d746c24f71fc7f4bbdf2b87e5b883
2017-11-03 21:00:56 +00:00
Wyatt Allen
d563a713d6 Only erase unsaved drafts after save request succeeds
Unsaved diff drafts are kept in localstorage so they are not lost if
they do not get saved (for example, if the browser crashes). However,
this stored copy of the comment is cleared when the save request is
started, not after it succeeds. As such, if the save request fails (for
example, if the user credentials are invalid), the comment can be truly
lost.

With this change, the localstorage copy is only cleared after save
success. If the request fails, it allows the error message to appear
rather than showing a misleading "All comments saved" toast.

Bug: issue 7289
Change-Id: Id5c8144ecccec50de35f83af616b4e4915b62b5c
2017-10-27 12:56:04 -07:00
Wyatt Allen
fb3733ccb9 Prevent save while disabled
The [Save] button cannot be clicked when the comment is disabled (via
`pointer-events: disabled;` in the CSS), and clicking the [Save] button
results in setting disabled to true. However, it is possible to click
save again before the style is applied. It's also possible to trigger
the button using a non-pointer-event (such as a key). With this change,
an extra check is added to the [Save] button's click handler to prevent
clicks when the component is disabled.

Bug: Issue 6972
Change-Id: I16117af31c5e7b5d7d49a025eebb690d8802b64b
2017-10-24 16:34:10 -07:00
Wyatt Allen
489e2680a7 Show confirmation dialog before destructively discarding draft comments
The discard confirm dialog is added alongside the delete confirm dialog
and the dialog opening/closing code is refactored to work on either. The
confirm discard dialog only appears if the draft message is savable.

Feature: Issue 6984
Change-Id: I6d88a8734fa7736f685796790e02437b10c81f00
2017-10-19 13:11:54 -07:00
Wyatt Allen
494e7d42b7 Show toast messages for storing diff comment drafts
With this change, toast messages show when diff comments are being saved
and notify when saving is complete, even when the diff view is no-longer
loaded. This signal is intended to encourage users to write diff
comments and feel free to navigate away to view other parts of a change.

With this change, new toast messages are able to replace existing toast
messages.

Feature: Issue 6970
Change-Id: I5973673d85c73b14794cb5e5b21327dd0c27ec1d
2017-09-17 15:53:32 -07:00
Becky Siegel
8d92d53db5 Annotation updates
Change-Id: I146f76b9dcc1a92e18acec01481ad280fb431868
2017-08-12 11:49:52 -07:00
Kasper Nilsson
076adcd4f8 Fix errors introduced by eslint^4.0.0
The release of eslint 4 introduced some breaking changes -- a full
upgrade guide can be found here [1].

In this change, the "no-useless-escape" and "indent" rules are disabled,
"indent" being replaced with "indent-legacy" to minimize breaks due to
lint. Also, extra spaces before comments and other assorted new lint
issues are corrected.

The long-term plan is to move away from the deprecated "indent-legacy"
rule and also reenable the "no-useless-escape" rule, but for now we
want to minimize breakages and not affect PG developer velocity.

[1] http://eslint.org/docs/user-guide/migrating-to-4.0.0

Change-Id: I8044256c95e3fe3f94fa9acc7922600908f8a0f4
2017-07-05 19:34:14 +00:00
Kasper Nilsson
9fee82ef7d Migrate gr-diff-comment to iron-a11y-keys
Bug: Issue 4198
Change-Id: I68ca64a9b0ef5cc9e83f19521529971d260fda55
2017-06-22 10:32:05 -07:00
Becky Siegel
99625b5a6c Add gr-autogrow-textarea to gr-diff-comment
This replaces iron-autogrow-textarea so that the diff comments can have
emoji support.

Change-Id: I8bfa31b789604f28aca0ff28dc5c0836afdb6005
2017-05-25 10:08:33 -07:00
Viktar Donich
e16c279304 Fix gr-diff-comment_test in Firefox, Safari
Change-Id: I0d11ffa48c254622cbc86ec7c0ce265f24212859
2017-05-23 21:05:33 +00:00
Becky Siegel
64cede2a14 Fix delete button again
In the previous fix, the function was not called and since it was
the function that set the class to 'hidden' it did not hide.

Change-Id: Ic8f908802cb515086860f20e2c82d0ddd06c0278
2017-05-19 23:08:16 +00:00
Becky Siegel
94cf669dc0 Do not show delete button for admins when comment is a draft
There was a formatting issue caused by delete buttons in a draft change.
It really does not make sense for the button to be there for drafts
anyway. This change hides the delete button for admins on draft changes.

Bug: Issue 6257
Change-Id: I236eff9db1f0ca76ed658f3048132c9c675ee407
2017-05-18 13:15:09 -07:00
Kasper Nilsson
a4c24de33c ES6ify /gr-diff-comment/*
Bug: Issue 6179
Change-Id: Id103711e6a7ca24785a301dbb8341bb0c8f63617
2017-05-17 19:29:04 +00:00
Viktar Donich
9f047389f4 UI for comments deletion by admins
This change depends on API implemented in Change 105052.

Feature: Issue 4644
Change-Id: I91137aaa69eef419e08c8a568cef9350d8a1448b
2017-05-17 10:29:08 -07:00
Becky Siegel
10a7126d9b Fix draft storage issue
Previously, there was a race condition in which erasing a draft comment
could occur prior to the updated version of the comment getting saved.
When this happened, if the user replied to the same thread, their old
draft would be pre-populated in the new reply.

This change cancels the 'store' debouncer when erasing a draft comment,
to avoid this race condition.

Bug: Issue 6141
Change-Id: I11a646acf01b1ac986c587e1e8fdb4fd7efc25b7
2017-05-16 15:13:52 -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
Wyatt Allen
23929fc5ff Add null/undefined checks for comment ranges
Change-Id: Ia66529e44b047ca8938708110d75e7bf910e73ae
2017-02-08 10:29:04 -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
Becky Siegel
64ce59f48a Store/retrieve unsaved drafts by range when available
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
2017-01-31 18:46:34 -08:00
Kasper Nilsson
87cb82f5a1 Expand unresolved comment threads in diff view by default
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
2017-01-10 18:42:37 +00: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
5c611bf1a0 Fix lint errors
Change-Id: I6d198498a7bd480a2b0081bff20ba89c3941ad48
2016-12-19 17:59:48 -08: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
Andrew Bonventre
4d22c7e835 Cleanup: use iron-a11y-keys-behavior for keyboard shortcuts
+ This does not cover on-keydown handlers within elements.
  A follow-up change will account for those.
+ Keyboard shortcuts are disabled within gr-overlay, input,
  and textarea elements.
+ Added tests for new behavior (plus some missing ones covering
  broken behavior).
+ Removed blur hacks used on elements to placate the kb
  shortcuts due to restrictions that have been removed.

Bug: Issue 4198
Change-Id: Ide8009a3bfc340a35a8ec8b9189a85b49c8a95aa
2016-11-17 15:27:59 -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
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
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
Andrew Bonventre
a12d3cdfe8 Fix bugs due to non-idiomatic use of property values in ready
+ dom-repeat reuses elements for the sake of efficiency, and
  according to [1], it’s considered an anti-pattern to do one-shot
  state setup by inspecting attributes/properties in ready/attached.
+ Move state management to be more explicit instead of one-time
  setup in ready().

[1] https://github.com/Polymer/polymer/issues/1713

Bug: Issue 3979
Change-Id: I5a7d2ae222223cf1ac8703cd30c66f004b606cfc
2016-05-24 14:50:13 -04: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