From 0d99067b8ddae3554c1e067d5a9cc2fc6d8c8629 Mon Sep 17 00:00:00 2001 From: Zalan Blenessy Date: Fri, 20 Mar 2015 17:31:54 +0100 Subject: [PATCH] Improve rebase usability with the RebaseDialog Since [1] the Rebase button is always visible for the current patch set. This leads to somewhat degraded usability, since its no longer obvious whether a change can be rebased or the change is up-to-date and consequently any rebase attempts will fail. This patch solves the problem by: 1. Displaying an orange circle next to the 'Parent(s)' text when the parent commit is not current, meaning the parent branch or change has moved on. 2. The Rebase (send) button in the RebaseDialog will be disabled, if the change is up-to-date and the "Change parent revision" checkbox is disabled (default). [1] https://gerrit-review.googlesource.com/#/c/63196/ Change-Id: Iaa1b0ce4cf3309330a55d1ab48d5422536089667 --- .../google/gerrit/client/change/Actions.java | 15 ++++++++- .../gerrit/client/change/CommitBox.java | 16 ++++++++++ .../gerrit/client/change/CommitBox.ui.xml | 22 +++++++++++-- .../gerrit/client/change/RebaseAction.java | 4 +-- .../client/changes/ChangeConstants.java | 1 + .../client/changes/ChangeConstants.properties | 1 + .../google/gerrit/client/ui/RebaseDialog.java | 32 +++++++++++++++---- .../gerrit/server/change/ActionJson.java | 8 +++-- .../gerrit/server/change/ChangeJson.java | 8 +++-- .../gerrit/server/change/ChangeResource.java | 9 ++++-- .../server/change/ChangesCollection.java | 10 ++++-- .../google/gerrit/server/change/Rebase.java | 8 ++++- .../server/changedetail/RebaseChange.java | 19 +++++++++-- .../gerrit/server/change/CommentsTest.java | 4 +-- 14 files changed, 130 insertions(+), 27 deletions(-) diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/Actions.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/Actions.java index 525684d44d..edf11058e3 100644 --- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/Actions.java +++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/Actions.java @@ -128,6 +128,10 @@ class Actions extends Composite { } a2b(actions, "cherrypick", cherrypick); a2b(actions, "rebase", rebase); + if (rebase.isVisible()) { + // it is the rebase button in RebaseDialog that the server wants to disable + rebase.setEnabled(true); + } for (String id : filterNonCore(actions)) { add(new ActionButton(info, revInfo, actions.get(id))); } @@ -177,7 +181,16 @@ class Actions extends Composite { @UiHandler("rebase") void onRebase(@SuppressWarnings("unused") ClickEvent e) { - RebaseAction.call(rebase, project, changeInfo.branch(), changeId, revision); + boolean enabled = true; + RevisionInfo revInfo = changeInfo.revision(revision); + if (revInfo.has_actions()) { + NativeMap actions = revInfo.actions(); + if (actions.containsKey("rebase")) { + enabled = actions.get("rebase").enabled(); + } + } + RebaseAction.call(rebase, project, changeInfo.branch(), changeId, revision, + enabled); } @UiHandler("submit") diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/CommitBox.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/CommitBox.java index 4bdd17fc03..93db1a7afe 100644 --- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/CommitBox.java +++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/CommitBox.java @@ -19,11 +19,13 @@ import com.google.gerrit.client.FormatUtil; import com.google.gerrit.client.Gerrit; import com.google.gerrit.client.GitwebLink; import com.google.gerrit.client.WebLinkInfo; +import com.google.gerrit.client.actions.ActionInfo; import com.google.gerrit.client.account.AccountInfo; import com.google.gerrit.client.changes.ChangeInfo; import com.google.gerrit.client.changes.ChangeInfo.CommitInfo; import com.google.gerrit.client.changes.ChangeInfo.GitPerson; import com.google.gerrit.client.changes.ChangeInfo.RevisionInfo; +import com.google.gerrit.client.rpc.NativeMap; import com.google.gerrit.client.rpc.Natives; import com.google.gerrit.client.ui.CommentLinkProcessor; import com.google.gerrit.client.ui.InlineHyperlink; @@ -46,6 +48,7 @@ import com.google.gwt.user.client.ui.HTML; import com.google.gwt.user.client.ui.HTMLPanel; import com.google.gwt.user.client.ui.Image; import com.google.gwt.user.client.ui.ScrollPanel; +import com.google.gwt.user.client.ui.UIObject; import com.google.gwtexpui.clippy.client.CopyableLabel; import com.google.gwtexpui.safehtml.client.SafeHtmlBuilder; @@ -77,6 +80,7 @@ class CommitBox extends Composite { @UiField HTML text; @UiField ScrollPanel scroll; @UiField Button more; + @UiField Element parentNotCurrentText; private boolean expanded; CommitBox() { @@ -121,7 +125,19 @@ class CommitBox extends Composite { if (revInfo.commit().parents().length() > 1) { mergeCommit.setVisible(true); } + setParents(change.project(), revInfo.commit().parents()); + + // display the orange ball if parent has moved on (not current) + boolean parentNotCurrent = false; + if (revInfo.has_actions()) { + NativeMap actions = revInfo.actions(); + if (actions.containsKey("rebase")) { + parentNotCurrent = actions.get("rebase").enabled(); + } + } + UIObject.setVisible(parentNotCurrentText, parentNotCurrent); + parentNotCurrentText.setInnerText(parentNotCurrent ? "\u25CF" : ""); } private void setWebLinks(ChangeInfo change, String revision, diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/CommitBox.ui.xml b/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/CommitBox.ui.xml index 5b7fb89b06..93312fa874 100644 --- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/CommitBox.ui.xml +++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/CommitBox.ui.xml @@ -68,7 +68,7 @@ limitations under the License. padding: 0; width: 560px; } - .header th { width: 70px; } + .header th { width: 72px; } .header td { white-space: nowrap; } .date { width: 132px; } @@ -106,6 +106,16 @@ limitations under the License. height: 16px !important; vertical-align: bottom; } + + .parent { + margin-right: 3px; + float: left; + } + .parentNotCurrent { + color: #FFA62F; + font-weight: bold; + } + @@ -163,7 +173,15 @@ limitations under the License. - Parent(s) + +
+ Parent(s) +
+