The group should have a max-width of 650px. This allows the group
to show the right side drop shadow on a normally sized window,
making the comment appear to float a bit on the page.
The max width was incorrectly applied to the header line of the
comment, andwas not being used by the browser to size the group.
Change-Id: I564c7fd9b154d64cdf1b20836351f40954a4900d
Almost all of the places that will need a ChangeNotes (when we start
reading from that class) should already have a ChangeControl, so this
is a convenient way to ship it around.
To deal with the migration, do not eagerly load ChangeNotes from the
constructor. We could lazily load on e.g. getApprovals(), but that
would require catching OrmException in more places.
Change-Id: I545b555e8369f7f0f775b5188079ea3e93ed9cef
When viewing the last file pressing ] takes the user up to the
change screen. Allow [ to return them to the last file, much as [
is an undo for an accidental ] between files.
Likewise allow ] to enter into the commit message.
Change-Id: Id1fac2cb0a34ecd87f2526400019bd65bd294186
On slower networks/servers users can create duplicate drafts by:
- start typing a new draft comment
- press ctrl + s, u
The ctrl+s key combo starts an RPC to insert the new draft and
transfers focus to another part of the page where u is a valid
key binding to go up to the change. When the u key is processed
the SideBySide2 screen starts to go up, which tries to save all
pending drafts. This starts another RPC to insert the draft. Both
RPCs eventually complete and there are now two identical drafts on
the same line.
Fix this by having the draft box remember which CallbackGroup it
is currently processing for. If another save call arrives before
this is finished the groups are chained together. In the case
describe above the CallbackGroup for the "up to change/save all"
activity will become a listener on the existing save RPC's group,
receiving notification when the save completes.
Change-Id: Ia4ecbb82b6c2866b8faf3d5995c995dca77be28a
Callers should always be using the factory method, except for a
particular kind of test within the same package.
Change-Id: I5cf55f57781b3667255797d198448bd7c1f21ade
Stop implicitly adding/removing reviewers by adding an arbitrary
PatchSetApprovals with a 0 vote. This is ugly and error-prone,
especially when LabelTypes change.
Instead, use the existing ReviewerState enum, adding a REMOVED type
as a tombstone. When parsing, record these during the walk, and prune
all REMOVED tombstones after the fact.
Change-Id: Iffb0517eb0162eb6cce66bf36d905a6eb60e75da
Some users have reported missing the subject line and only reading
the commit message body on ChangeScreen2. The subject is bold and on
a "trimColor" (e.g. grey) background while the message body is on the
page background color (e.g. white). This coloring makes the subject
appear to be unrelated to the message, and reviewers find themselves
missing important context when trying to understand the commit.
Move the subject line down into the commit message body area and
slide the change number and status into the left 1/3rd of header
line bar.
Change-Id: I61c3143c636e234776189d545def9ff528eaa2bf
If two comments are placed on different lines in a single add
region the Reply/Done buttons on the first comment were broken.
The breakage was caused by the group's peer being replaced in sideA
by the second group's peer, so the first group's peer was never
attached to CM3.
Always attach both members at the same time, from either map.
Change-Id: Iccce04ef5f8758f35376829140e0e2f308d387a2
If a reply is created and discarded, do not create a range
selection on the range supplied from the published comment.
Change-Id: Id8adfc080cba252d3d47bc7b0f5eacb03853aeb6
Callers were supposed to be calling hasChange() to determine whether
getChange() would return null, but none were. Remove this error-prone
pair of methods and just use change(), which includes lazy loading.
There are a few places where we need to catch an additional
OrmException but these are easy enough to fix.
Change-Id: I23335e362715f59e2c093ffec88427739ff0cffc
If an API returns a very large response, the parseJson call can take
75ms - 200ms to complete. Detect stalls greater than 75ms and defer
the onSuccess() handling to the next event loop so it isn't as
noticeable to the user.
Change-Id: Icc904f0c0e42b45771d5c3b003e89f845dbfb038
Add a new ref namespace refs/changes/*/*/meta for storing the state of
a Gerrit change in Git. Information about the change is mostly stored
in commit messages, the entire history of which are read at load time
to populate information about the change. Eventually, information tied
to a particular patch set will be stored in a git note referring to
the patch set's commit.
This data format only allows fast access to data on a per-change
basis. Lookups by other keys (e.g. approvals by user) will depend on the
secondary index.
The commit message format is simple:
Update patch set 1
<optional cover message>
Patch-Set: 1
Label: Code-Review=+1
The subject line is machine-generated and callers should attempt to
make it descriptive, but this is not strictly required. The
cover message is user-provided (not implemented in this change). At
least the Patch-Set footer line is required for the time being; it is
likely we will want to enforce there always being at least one footer to
fend off user-provided cover messages that themselves contains
footers.
The author of the commit is the end-user performing the update, using
their full name and email address. The committer is the same user, but
includes the Gerrit account ID in the email address for
disambiguation. (Eventually, we may decide that the (name, email)
combination is enough to uniquely identify a user, but for now we
still require the account ID.)
On the code side, reading and writing changes from this ref is
implemented with two VersionedMetaData classes:
ChangeNotes implements the read operation. When a ChangeNotes is
loaded, it traverses the entire history of the ref and populates the
object with all relevant information. Traversing the entire history
should be a fast operation on most changes, which typically have
roughly tens of operations.
ChangeUpdate implements the write operation. Unlike the traditional
database, where operations are organized around read-modify-write
operations on various entities spread across tables, a ChangeUpdate
encapsulates a set of updates that can be applied atomically. Because
each commit has a single author, timestamp, and patch set, these are
logically restricted to operations performed by a single user on a
single patch set. This corresponds better to the typical usage of the
database, where a single end-user is performing an operation on a
single change via a REST endpoint.
ChangeNotes and ChangeUpdate are separate VersionedMetaData subclasses
for a few reasons, mostly technical. First, it should not be necessary
in all cases to slurp all change data via ChangeNotes before preparing
a ChangeUpdate. Second, due to subtleties like LabelType sorting, it
would be tricky to make ChangeNotes mutable in such a way that the
result of reading it after one or more mutations would be the same as
if that change were read freshly from the git data. Keeping
ChangeNotes immutable and ChangeUpdate separate makes it less tempting
to assume this behavior.
This change implements only reading/writing for
ChangeNotes/ChangeUpdate; they are not yet used in any read/write code
paths in the server.
Change-Id: Id28978a92f3b197c8b5be6e2db0c1e3923f2211e
If a draft is already present and replying to this published comment,
hide its reply and done buttons until the draft is discarded or is
also published.
Change-Id: I9ca9dc6164c00d6af58f961e3ffb43b44b8e289d
InitStep has a new postRun() method which must be implemented.
Modifications of the All-Projects project.config have to be done in
the postRun() method since during the run() method the site does not
exist yet if a new site is initialized.
Change-Id: I186817f76b328a344d1a04f982d5bc2973688c25
Signed-off-by: Edwin Kempin <edwin.kempin@sap.com>
If a draft comment box grows its text area and there is a selection
below the box, the insert comment bubble for that selected region
needs to move down the page. Update the selection whenever the box
grows to force the bubble to slide.
Change-Id: I30a137a51a60a35c2c9234126ea854fb4c02b3c0
If a comment is created and immediately destroyed (e.g. press 'c'
then 'esc'), restore the range selection for the comment. This allows
the user to quickly adjust the range and recreate the comment, fixing
simple errors where they missed a character or two at the end of the
selected range.
Change-Id: I70e74b40fa8635793360dca4f83fb63a01c9b750
Focus needs to be transferred after the browser event loop returns.
I don't recall specifically why this is necessary, but without the
defer the new comment box does not have focus.
Setting the cursor position immediately after transferring focus
appears to work, so these tasks can be combined together.
Change-Id: Ic28a1ba8d346779f20b4aef0bd794469377c3ddc
SideBySide2 needs the modified B side lines even if the lines are
"common" when compared to A using the selected whitespace ignore
option. The modified B lines must be used so the text aligns properly
with surrounding context, such as wrapping some unmodified code in
an if block.
Send these by splitting an "ab" block into "a, b, common:true",
allowing the UI to correctly reconstruct the B side.
Change-Id: I5e045c32d9e86434094e03e844fb80ef4186993f
The keyboard binding 'c' is very useful to insert a comment, as users
are about to start typing anyway. Make this more widely known by
placing a text bubble next to the insert comment bubble with the
message "press [c] to comment".
The bubble is also clickable, giving the user a bigger target than the
16x16 icon that also appears above the insertion point.
Change-Id: Ibc1f874dd5f9c852a9316c11128a691f4c21c288
When CM3 enters a skipped region the bars are removed. They also
must be removed from the SkipManager's collection.
Do not modify the CM3 selection when CM3 enters a skipped region.
This should only attempt to happen during a user click in the bar.
Change-Id: Ieebde1e1b1bdd9e4714849165def65e65b55668e
This reverts commit 1df5a8f00fbe69ec3b386ed3487a2ae0c229a966.
Tracking the bar using a SortedMap is not necessary. The skip
bars are removed automatically by CM3 now when search enters into
a skipped region.
Change-Id: Ia3194692c7a8e7c098bc97d9dc2ebd3fa89fc5ca
Allow CM3 to expand an entire skipped region of a single
search result is found within the region.
Change-Id: Iaaf139a2ff9a0d7a2c018b7195d093eac1634ab2
Allow plugins to contribute messages to the UI during initial page
load. Gerrit displays the messages in a butter bar at the top of
the page and allows users to hide them with the "Dismiss" button.
Messages are hidden per-browser using a cookie named after the
message id, set to expire at a date supplied by the server. After
this date the same message could redisplay if the server sends the
same message again.
Change-Id: I0bcca845f501cbeb8c31356fff398c3adb43099a
When creating a new comment on a selected region, trim off any empty
line at the end of the selection.
This fixes a common problem when the user has selected an entire
paragraph of text and finished the selection at the start of the next
line, which logically appears to be the end of the paragraph.
Comments were inserting below this empty line, as the empty line was
set as the end of the range.
Change-Id: Ib5fc67f5a0c563b701c84d92e11886e0116e3a73
When a line is inserted at the start of the file the A side inserts a
padding widget above the line. CM3 doesn't always include this widget
height in the heightAtLine() computation on the A side, causing the
files to become unaligned after scrolling down and then back up.
Special case the 0th line in the editors by assuming these will be
aligned at the very top of the page on either side and just use pixel
coordinates.
Change-Id: I91f7d8afe89fa7c22ec4dcf385d24a4a8d284421
Make Commit-Id copyable similar to Change-ID
so that users can easily copy it.
Feature: Issue 2191
Change-Id: I5f0e955432307f8c32c733ea7e0ed2568c4dd82d
Use a SortedMap to track the SkipBars, indexed by first line number
hidden by the bar. This simple refactoring allows for a future
change to find if a line is covered by a skip bar.
Change-Id: Iee71856e0c24140a10c0ee4938abfcef6490d6e9
In the old UI "M" marked the reviewed flag (if not already set)
and moved to the next file in the change. Implement a similar
behavior in the new SideBySide2 UI.
Bind as "shift m" instead of "M" due to vim mode using lowercase
"m" as a mark character, which some users may actually use to make
bookmarks during a review.
Change-Id: I1e7780e6265fa3380aae09c034583dec7ec4830a
The old screen supported N and P to jump to next and previous
comments in the file, similar to n and p moving over modified
regions.
Bind shift-n and shift-p to navigate through comments.
Change-Id: I8fb788f438987e2448d39b62ced1e7615d32546e
The background worker thread creates its own database connection
to read the record. The change could have been modified between
the time it is enqueued, and the time the indexing is processed.
Always read from the database in the background thread to ensure
the index gets the most recent possible data.
This may fix some index state errors on gerrit-review (and friends).
Change-Id: Ifab8d0ba7c9dcac54b7e168b86e9a82a3c681ea5
