78 Commits

Author SHA1 Message Date
Kasper Nilsson
7c33466e9a Merge "Modify reply dialog to wait for comment save" 2017-08-02 22:37:06 +00:00
Viktar Donich
b574c495b9 Merge "Document need for async title-change" 2017-08-02 22:06:09 +00:00
Kasper Nilsson
6873549786 Modify reply dialog to wait for comment save
Previously, opening the reply dialog was disabled when draft comments
were pending save. Now, the reply dialog is fully enabled, and will
silently wait for pending comments to be saved before proceeding with
any save/send operations.

Moreover, when diff drafts are in the process of saving, a label is
displayed below the comments list. When they finish saving, the list is
updated with the diff comments.

Change-Id: I2777f9e15b052e606aa210b5372dc7a89a076345
2017-08-02 21:21:06 +00:00
Wyatt Allen
c61ba25b11 Coalesce request for drafts with login check
Because requests for a user's draft diff comments is always nested
within a check for whether a user is logged in, these can be combined
into the same REST API interface call. The updated call is given a JSDoc
to describe the additional functionality and call sites are simplified.

Change-Id: Idc0cc6298c7287579ba76e7cc35701e62142bca3
2017-08-01 14:18:39 -07:00
Wyatt Allen
dc8782d762 Centralized comment requests
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
2017-08-01 14:15:38 -07:00
Wyatt Allen
e9bd3e286d Document need for async title-change
Better documentation for the fix in Ia32e76023c.

Change-Id: Ia272196248fc2acdec7041137e1d452ff70d0f06
2017-07-31 16:30:32 -07:00
Kasper Nilsson
49e3d1f47f Merge changes I7b542ddb,Ie9bea7f9
* changes:
  Call Gerrit.Nav.upgradeUrl from valid views
  Add function upgradeUrl to gr-navigation
2017-07-28 17:00:05 +00:00
Wyatt Allen
68e1e21b69 Fire diff-view title-change event in an async
Formerly, the title-change event was fired too early, and failed to
set the actual title.

Change-Id: Ia32e76023c426cf6b8a3ad1456490885fb22bd67
2017-07-25 20:27:19 +00:00
Kasper Nilsson
9060cfd1a5 Replace patchNum compare with utility function
In preparation for implementation of in-app editing, the instances of
parseInt(patchNum) must be swapped out, as a patchNum may now be either
a number or a string.

This change adds the patchNumEquals function to gr-patch-set-behavior,
and uses it everywhere patchNum is compared.

Bug: Issue 4437
Change-Id: Ib1176508cd88d60c79e952b99dd5f57b994baa77
2017-07-25 09:51:04 -07:00
Kasper Nilsson
f89608d903 Call Gerrit.Nav.upgradeUrl from valid views
The migration to project-based URLs has made evident the need for some
alias in gr-navigation to `history.replaceState`. This change calls
the function `upgradeUrl` within gr-diff-view and gr-change-view iff the
project returned by getChangeDetail differs from the one set on
app.params.

Bug: Issue 6708
Change-Id: I7b542ddb989527b6a89ccdfd79846808835ed5a3
2017-07-24 15:12:13 -07:00
Kasper Nilsson
3c2e121c9d Add more fields to Gerrit.Nav.View and utilize in app
Previously, the app relied on string matching of the view tag names in
order to perform view-based actions (e.g. routing).

This change refactors those instances to utilise the Gerrit.Nav.View
enum.

Bug: Issue 6708
Change-Id: If0212fde93e0167e3207af19006beee1a602df60
2017-07-19 17:02:08 -07:00
Becky Siegel
05d1253d8d Get hash from router instead of location
Previously, there was an issue where nagivating between diffs would
use the hash from the previous diff because the url had not been
updated when _paramsChanged was calls. Instead of getting the hash
from window.location, let the router handle it and pass it in as a
param.

Bug: Issue 6702
Change-Id: I03cb59e0f55813b0973eaf949e9e0958946d094e
2017-07-19 10:22:10 -07:00
Kasper Nilsson
daba503058 Merge "Only truncate file path in page title" 2017-07-13 18:00:02 +00:00
Kasper Nilsson
f2749236b2 Only truncate file path in page title
Uses util.truncatePath to compute an appropriately short page title.

Bug: Issue 6709
Change-Id: Id742a122a20011ac82ade865d3ea579179c38c1d
2017-07-12 20:41:42 +00:00
Kasper Nilsson
68406c48b0 Merge "Fix typo" 2017-07-12 20:31:58 +00:00
Kasper Nilsson
143a4279e1 Fix typo
Change-Id: I8f2f3fa9b4f346f951ba99a805641162573a690a
2017-07-12 12:47:14 -07:00
Kasper Nilsson
de211a56a2 Remove alert-based error reporting
Migrates all alert calls to toasts. Also corrects one test that checked
for an alert on the window.

Bug: Issue 6701
Change-Id: I4d41790f63edc15873014df4f98013ac2bb2af7c
2017-07-12 11:04:48 -07:00
Kasper Nilsson
67fc619449 Shorten long pathnames in page title
Split the path at the last slash to provide only the basename as the
page title in diff-view.

Bug: Issue 4955
Change-Id: I242d38f5823415a53f837ad7d1a5b97bd16c6fa6
2017-07-06 16:32:17 -07:00
Wyatt Allen
c310f4fbab Update URL generation in gr-diff-view
Bug: Issue 6446
Change-Id: I21be5ac4e3e89390745a6f04582cbfb9dc6535a3
2017-07-06 15:28:42 -07:00
Thomas Shafer
88c179781a Replace "attached" with observers
This allows for the functions which rely on the properties to be called when
the properties are set.

Change-Id: I306550a246c4c535ae5f165ce3fbe5282eea25f2
2017-06-27 12:24:00 -07:00
Viktar Donich
168bbc96a4 Add a setting to disable diff panel floating
window.PANEL_FLOATING_DISABLED=true prevents diff header detaching from
the page flow and sticking to the window top border.

Change-Id: Idafab7f73fb52a9165b7610ae609e0a8fe52bbd9
2017-06-19 14:50:09 -07:00
Kasper Nilsson
eb07531a6d Remove modifier pressed check for bracket key
On Latin American keyboards, the square bracket cannot be typed without
a modifier. An added check to exit if modifiers are pressed meant that
the shortcut could thus not be used on these keyboards.

Instead, check only for the meta key to avoid overriding native Chrome
shortcuts in OSX.

Bug: Issue 6217
Change-Id: Ia737c4c411b73b2ba42fe5f33fff5082c488a5fb
2017-05-26 11:36:55 -07:00
Kasper Nilsson
ef51194c11 ES6ify /gr-diff-view/*
Bug: Issue 6179
Change-Id: Ia2268dba166b9f98cd43fb407666a4a29bf285d2
2017-05-17 15:34:20 -07:00
Aaron Gable
10af4156f2 Use 'Merge list' as display name for /MERGE_LIST
Bug: issue 6041
Change-Id: I4cd4ce47173a277cf917ced5b274225aa982000d
2017-05-04 18:26:02 -07:00
Kasper Nilsson
7aebc5fdc8 Fix Closure errors in PolyGerrit
The Closure Compiler is very picky with regard to JSDoc formatting. This
change fixes a few invalid JSDoc issues, and removes the corresponding
suppresses from the BUILD file. Additionally, modify the transpilation
target language to ES5.

After this change, there should no longer be warnings from the Closure
Compiler during building of PolyGerrit.

Change-Id: If7566e40c2c419c55f2a634c0f558c6cc263f61c
2017-04-27 09:00:43 +02:00
Becky Siegel
7a45400e24 Move diff overlay to gr-diff-preferences
Previously, gr-diff-preferences was the contents that was displayed in
an overlay contained in the gr-diff-view.  There were a handful of
functions that crossed between the two-- the diff preferences element
would fire events that needed to be handled by the diff view.

Because the gr-diff-preferences element will be added to the change
view as well, the overlay has been moved to be part of the diff
preferences element. This way, the element can handle all of the actions
taken by the panel, and all the parent element needs to do is call
the open function.

A separate change will come with the addition of diff preferences in the
change view.

Bug: Issue 5426
Change-Id: Id7396147e73354122ea3825bde2c324b5daa1d26
2017-04-19 13:25:25 -07:00
Paladox none
8b0d046dfb Fix PolyGerrit URLs to support prefixed URL
This updates most of polygerrit links to use the implementation added in
I2b2d704fe33c90ea2f2a2183fc79897642a48175 .

Change-Id: Ib3bb694969e903fd76a1dad13cfb642bde086142
2017-04-13 23:44:31 +01:00
Becky Siegel
0304907817 Fix dropdown tap on touch device
Previously, there was an issue where click events did not fire before
tap handlers closed a dropdown. If the dropdown was closed first, the
dropdown is gone and something random becomes the event target.

This change adds a short setTimeOut to ensure that the link is the click
target prior to closing the dropdown.

Bug: Issue 5932
Change-Id: I218893fd479eeb68361dd2c94d919740a8f542f2
2017-04-06 17:02:38 -07:00
Paladox none
d43abe9fe8 Adds a up button to diff's screen in polygerrit
GWT UI has this button but polygerrit dosen't. Makes it easier to view
your other diff's if you need to pick another file but it is way down
the list.

Change-Id: Ic62480315b3b00d7623d4ac444a0b554d255b4d1
2017-03-31 15:53:51 +00:00
Wyatt Allen
1333e30bd7 Remove loading state from the diff view earlier
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
2017-02-10 15:29:36 -08:00
Kasper Nilsson
6cff0e60d6 Fix race between comment saving and reply dialog
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
2017-02-07 16:41:50 -08:00
Wyatt Allen
1bc4f2f565 Support jumping to next/previous file with comments via shortcut
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
2017-01-19 12:18:52 -08:00
Kasper Nilsson
de8dc08d3a Allow for prev/next navigation out of diffbase
If a user in diff view changes the diff range to one that no longer
includes the file that they are on, the Prev/Next links are grayed out
and they are forced to return to the change view to navigate the diff.

This change sets Next to direct to the first file in the list and Prev
to point to the last file in the list.

Bug: Issue 4932
Change-Id: Ifb460c9721bfafbc19afa68253402b9dcd2f2c3e
2017-01-06 15:31:32 -08:00
Wyatt Allen
18406b5b85 Merge "Move diff view defaults to the rest api interface" 2016-12-16 18:33:08 +00:00
Becky Siegel
76ec59681d Move diff view defaults to the rest api interface
Previously, there was logic regarding diff view defaults in both the
diff view and also the file list views.  When the unified view became
the mobile default for the diff view, the file list was forgotten, and
if a user visited a change view first and then a diff view (without
refreshing the page) they wouldn't get defaulted to unified on mobile.

This change fixes the issue and moves the logic for which view type to
display to the rest interface, so that it doesn't have to be implemented
in multiple places.

Bug: Issue 5119
Change-Id: I95bfe1540cc9439bd6d3e3e39d13a5e32962b7fa
2016-12-15 10:37:59 -08:00
Wyatt Allen
b148a81817 Merge "Make file list more useful on mobile" 2016-12-15 18:16:04 +00:00
Becky Siegel
acf455a9b6 Make file list more useful on mobile
Previously, the file path was truncated and often the file name was cut
off completely, which had made it hard to tell what files were actually
changed.  With this change, the text appearing on the file list just
show ellipsis and the file name (ex: '.../filename.txt').

Additionally, for both mobile and larger screens, the full filename
appears (line wrapped if needed) when the file list item is expanded.
This way, if enough content is cut off that it's still not useful, there
is a way to see the path in it's entirety.

Bug: Issue 4609
Change-Id: Ic4aaf45bafbc3c5b31add8f7c43b18c9d2b2913b
2016-12-14 14:46:17 -08:00
Kasper Nilsson
fa4b2fe374 Add check for modifiers in keyboard shortcut handlers
Bug: Issue 5079
Change-Id: I221bfcfc42e14159b5457d7177902b947906f75d
2016-12-09 08:36:59 -08:00
Kasper Nilsson
1ba850e594 Fix comment copy logic
The addition of formatting in comments broke a variety of things having
to do with the copying logic. This change updates the logic and tests
to reflect the new DOM.

This issue arose because of a lack of integration tests for copying and
selection. That test is coming in a descendant change.

Bug: Issue 4969
Change-Id: I4e1994ab07947506c77b07877a46a9369d666d50
2016-12-02 15:25:27 -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
Becky Siegel
6a7085e5ab Make line marker more distinguished
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
2016-11-03 13:34:07 -07:00
Wyatt Allen
20693de09f Properly encode file path in diff view
Bug: Issue 4827
Change-Id: Ia0815aca6649324fe9f3ad7c5e2168c25d1f8a7d
2016-11-01 16:11:07 -07:00
Kasper Nilsson
c59e88058e Add raw diff download link to PG diff-view
A dependent change added the ability to download a single file diff to
Gerrit. This change utilizes that new feature via a download link in the
diff-view.

Feature: Issue 4669
Change-Id: I87ef2324ff2cd7fab6eb4b2e066dd08defe7c4f0
2016-10-31 18:06:51 -07:00
Wyatt Allen
43ddaf6b1c Set line number hash using replaceState
When clicking a line number in the diff view, set the URL hash using
`history.replaceState` rather than `history.pushState` to avoid an
additional history entry.

Bug: Issue 4820
Change-Id: If2101508a49ac15e955d2981f7c7f93f22d5b9f9
2016-10-27 10:51:07 -07:00
Wyatt Allen
4e1d1b8427 Accept base patch line links
PolyGerrit uses the format b<line number> in URLs to indicate a specific
line number in the base patch, but the GWT UI would use the format of
a<line number>. Update PolyGerrit to understand both.

Bug: Issue 4792
Change-Id: I8d09be9cb952c66e085d3a4777c04e6d79eab518
2016-10-20 16:53:34 +00:00
Andrew Bonventre
a8e4332ba0 Remove extra network call to get change detail on diff view
An observer was causing an extra call to grab the change detail
when it wasn’t needed since it’s already called in _paramsChanged.

Change-Id: I9a9904145e5d88c377cd9d130a7f9a4be67454e3
2016-10-16 13:32:19 -07:00
Logan Hanks
610f80d3ec Add namespacing for keyboard shortcut disabling
The gr-overlay element attempts to manage disabling and enabling
keyboard shortcuts. When multiple gr-overlay elements are available
on a page and one of them opens immediately, that overlay tries to
disable keyboard shortcuts, but the other elements initialize as
closed and enable them.

This change offers a new method for disabling keyboard shortcuts.
The caller can pass in an identifier to enable or disable. If keyboard
shortcuts are disabled by one or more identifiers, then they are
suppressed.

Change-Id: I82fe6efd922f09279e76a2f2c8cb5781f3afe395
2016-10-14 14:05:43 -07:00
Becky Siegel
6c06e78cf6 Fix horizontal scrolling in file dropdown, comments, and gr diff view
Previously, long file names would display outside of the window size,
causing a horizontal scroll on mobile browsers.

With this change:
* In the file list on the change view, the rightControls are adjusted so
that they wrap on a new line on smaller/mobile screens.
* In the comment list, word-wrapping is used for long file paths
displayed above each comment.
* In the diff view, instead of displaying full paths in the dropdown,
the path is truncated to include an ellipsis, a slash and the file name
so that it can be seen both in the dropdown and also the native browser
select.

Bug: Issue 4722
Change-Id: Icd4644a45db71bc6666c21d62c864d91e9874654
2016-10-13 15:44:59 -07:00
Andrew Bonventre
99642e348e Merge "Default to unified view on small screen sizes" 2016-10-13 00:20:32 +00:00
Becky Siegel
74f531c6a6 Default to unified view on small screen sizes
Previously, the default view type was displayed for all screen sizes.
If side by side was the user's preference, and they viewed polygerrit
on mobile, side by side was displayed.

This change ignores user preference for small screen sizes (under 900
px wide). The user can still toggle to a split view, but unified will
be the first view shown.

Bug: Issue 4682
Change-Id: Id4e1cab17de433033e103c3cca582b7e9c656acf
2016-10-12 14:51:09 -07:00