Since the diff view now has access to the same comment structure as the
change view/file list header via the comment api, patchsets can now
display comment counts in the dropdown.
Bug: Issue 7212
Change-Id: I85697c770862de48b2b2ce82649f1b8873ab9480
Previously, nested components (that are never standalone) were making
their own requests for comments, creating unecessary duplicate API
requests.
Some requests were made to the rest API directly (for example,
gr-change-view requested comments this way), and some were through the
gr-comment-api, which also helped handle comment manipulation and
reorganization.
Instead of ad-hoc comment requesting, move all comment requests to the
top level (standalone) component, and use an object prototype to
generate _changeComments as a property on gr-comment-api. This is an
immutable object that can be passed to the inner components
(file list, etc), and includes the methods needed to manipulate
comments into the forms
necessary.
When a child component needs to trigger a refresh of the comments, fire
an event that the parent event handles (see gr-file-list l.867).
Bug: Issue 6953
Change-Id: Ic4b6cf16520baae65d8cf956c311a60f2a70a2e1
Previously, the patch range selector was updated to not use two way
bindings (https://gerrit-review.googlesource.com/c/gerrit/+/130453) but
the diff view's usage was not updated as well. This change updates the
diff view to use the patch range select in the same way as the file
list.
Bug: Issue 7420
Change-Id: I1baaf643e08d1a4661962021be890bafd6d5b4ef
Previously, PolyGerrit relied on the default value of the loading flag
in the diff view to show/hide the appropriate diff panels (or loading
message). Because of this, stale information is displayed when a user
navigates between different files within the same diff view.
With this change, the `_loading` flag is set before the major API calls
are made, and the loading message is made more obvious.
Bug: Issue 7381
Change-Id: Id70d981a2c72fef9f44a32c5463dc43c3ce41cf6
This also moves the change reloading logic back to the change view,
where it gets updated patch ranges via a two-way data binding.
Change-Id: Ib09ad1a176ba96bac77a513d344226df029aef7b
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
The gr-diff-cursor#getAddress method returns null when it has no
position or no address corresponds to it's position. However, the
gr-diff-view#_onLineSelected method did not account for this case and
attempted to use null addresses to construct URLs when moving the cursor
to a "File" line of a diff.
With this change, the diff view neither uses line number nor the diff
side when constructing URLs if the cursor does not yield an address.
Change-Id: I628658295bca1f49e0c2d3484e2e0d01e71bcd91
Slight refactoring required in order to satisfy both use cases:
- Fire an event when patch range changes and let parent element
handle it.
- Support comment strings
- availablePatches becomes an array of objects instead of an
array of integers.
Change-Id: Ia8da9296f41eb2d45c9358d03fbec3940273ad9d
Some URLs (for example, comment links in Gerrit emails) specify the line
number following an @-sign rather than in the hash using an octothorp
because the URL is already mostly inside a hash (for backward
compatibility with the GWT UI). When these URLs do not include the
project name (as they currently do not in Gerrit emails) the project is
loaded and the parsed route is "upgraded" to include the project.
However, when generating the upgrade URL, the `_generateUrl` method
looks for the `lineNum` and `leftSide` properties to generate the
address. While the address specified by the @-sign is properly converted
to an octothorp-hash before this point, it appears on the properties
object as `hash`, and is thus ignored by `_generateUrl`.
With this change, instead of passing the raw hash through the app params
and parsing it in the `gr-diff-view`, the hash is parsed into its
`leftSide` and `lineNum` values and attached to the `app.params` by the
router. In this way, the location is preserved through URL upgrade, the
param format used in navigation matches that used in URL generation and
the diff view no longer parses the route.
Bug: Issue 7087
Change-Id: Idb2e3cccf2884fae742247cf1ebbde1ad97e53ab
Formerly, the gr-diff-view generated and updated the URL hash that
indicates the location of the diff cursor, regardless of the hash
generation scheme implemented in _generateUrl. With this change, the
diff view uses the _generateUrl for the cursor-location-specific URL.
Change-Id: Ia70a298f114c9eae9cdc5b0a8f73d7ecab55896e
To prevent redundant (and sometimes incorrect) change/diff loads, this
change moves the logic for rewriting the URL schema to within the
router.
This has the drawback of adding a step -- and a potential API call --
between initial URL interpretation and first render, but ensures that
render will be correct. This only occurs when legacy routes are used.
Bug: Issue 7027
Change-Id: Ia1571e2f946f6d9e334626b6b0fd4a8c0c81f35d
Modifying the reviewed state of an edit patchset is an illogical action,
and should be disabled.
This change adds an _editLoaded property to gr-diff-view to
conditionally hide the reviewed checkbox. In addition, attempts to
modify the reviewed state are treated as a no-op.
TODO: Further utilize this approach to conditionally show buttons for
modifying an edit patchset. Will be done in a later change.
Bug: Issue 4437
Change-Id: I0ea98e49c5ec66e73f45fef9db5e3849d6e594df
ORIGINALLY: Iafd4b80af53624027c347f0f443b4cdfde292e29
This change makes the change view and the diff view change edit aware.
Mutation operations on edit itself, like editing files in editor, are
beyond the scope of this change.
Change edits must be fetched with a separate change edit endpoint and
merged into the change.revisions object. The _number of an editInfo is
'edit'. It has a special property called 'basePatchNum' that marks
which patch set the edit is based on. In patch set selectors, the edit
is sorted right after its basePatchNum.
Alternative implementation considerations:
It could be easier to handle edits on the Polygerrit UI, if
GET /changes/<id>/detail endpoint
would optionally include the change edit in the resulting
change.revsions map.
TODOs:
* Overwrite change.current_revision with change edit commit in some
use cases
* Mark the change edit as Change Edit in the header of the change view
* Allow for modification of the edit in the change/diff view
* Disable commenting on files in edit patchsets
* Modify file list rows to have appropriate actions when edit is
selected (e.g. allow revert, disable marking as reviewed)
* Identify and whitelist valid change/revision actions for an edit
Bug: Issue 4437
Change-Id: Ia4690d20954de730cd625ac76920e849beb12f7c
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
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
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
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
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
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
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
Migrates all alert calls to toasts. Also corrects one test that checked
for an alert on the window.
Bug: Issue 6701
Change-Id: I4d41790f63edc15873014df4f98013ac2bb2af7c
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
This allows for the functions which rely on the properties to be called when
the properties are set.
Change-Id: I306550a246c4c535ae5f165ce3fbe5282eea25f2
window.PANEL_FLOATING_DISABLED=true prevents diff header detaching from
the page flow and sticking to the window top border.
Change-Id: Idafab7f73fb52a9165b7610ae609e0a8fe52bbd9
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
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
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
This updates most of polygerrit links to use the implementation added in
I2b2d704fe33c90ea2f2a2183fc79897642a48175 .
Change-Id: Ib3bb694969e903fd76a1dad13cfb642bde086142
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
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
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
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
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
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
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
