From fdb43453bd7b77aeb69e16deae506106b25df9aa Mon Sep 17 00:00:00 2001 From: Shawn Pearce Date: Thu, 29 Aug 2013 14:13:37 -0700 Subject: [PATCH 1/2] ChangeScreen2: Replace revision selector with a popdown Display a table of revisions when the revision selector is opened. Clicking on the hyperlink for the commit SHA-1 opens that revision in the current window, or can be copied/pasted or opened in a new tab using standard browser features. Revisions are sorted descending in age, so the most recent revision is shown at the top and the oldest revision is shown at the bottom. Some limited commit information is also used. The committer date is shown and a short name for author and committer is displayed. This can make it easier to see revisions that are collaborations. Bug: issue 2074 Change-Id: I1b866cb92d672160d287f4dd6e22d9c713a591d3 --- .../gerrit/client/change/ChangeScreen2.java | 67 ++---- .../gerrit/client/change/ChangeScreen2.ui.xml | 10 +- .../gerrit/client/change/Constants.java | 5 + .../gerrit/client/change/Constants.properties | 5 + .../gerrit/client/change/RevisionsAction.java | 37 +++ .../gerrit/client/change/RevisionsBox.java | 219 ++++++++++++++++++ .../gerrit/client/change/RevisionsBox.ui.xml | 65 ++++++ .../gerrit/client/ui/FancyFlexTableImpl.java | 4 +- .../client/ui/FancyFlexTableImplIE6.java | 4 +- 9 files changed, 358 insertions(+), 58 deletions(-) create mode 100644 gerrit-gwtui/src/main/java/com/google/gerrit/client/change/RevisionsAction.java create mode 100644 gerrit-gwtui/src/main/java/com/google/gerrit/client/change/RevisionsBox.java create mode 100644 gerrit-gwtui/src/main/java/com/google/gerrit/client/change/RevisionsBox.ui.xml diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/ChangeScreen2.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/ChangeScreen2.java index 580575cca5..498b9c244a 100644 --- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/ChangeScreen2.java +++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/ChangeScreen2.java @@ -46,7 +46,6 @@ import com.google.gerrit.client.ui.ChangeLink; import com.google.gerrit.client.ui.CommentLinkProcessor; import com.google.gerrit.client.ui.Screen; import com.google.gerrit.client.ui.UserActivityMonitor; -import com.google.gerrit.common.PageLinks; import com.google.gerrit.common.changes.ListChangesOption; import com.google.gerrit.reviewdb.client.Change; import com.google.gerrit.reviewdb.client.PatchSet; @@ -57,7 +56,6 @@ import com.google.gwt.core.client.JsArray; import com.google.gwt.core.client.JsArrayString; import com.google.gwt.dom.client.AnchorElement; import com.google.gwt.dom.client.Element; -import com.google.gwt.event.dom.client.ChangeEvent; import com.google.gwt.event.dom.client.ClickEvent; import com.google.gwt.event.dom.client.KeyPressEvent; import com.google.gwt.event.logical.shared.CloseEvent; @@ -72,10 +70,8 @@ import com.google.gwt.user.client.rpc.AsyncCallback; import com.google.gwt.user.client.ui.Button; import com.google.gwt.user.client.ui.FlowPanel; import com.google.gwt.user.client.ui.HTMLPanel; -import com.google.gwt.user.client.ui.ListBox; import com.google.gwt.user.client.ui.PopupPanel; import com.google.gwt.user.client.ui.ToggleButton; -import com.google.gwt.user.client.ui.UIObject; import com.google.gwtexpui.clippy.client.CopyableLabel; import com.google.gwtexpui.globalkey.client.GlobalKey; import com.google.gwtexpui.globalkey.client.KeyCommand; @@ -84,7 +80,6 @@ import com.google.gwtorm.client.KeyUtil; import java.sql.Timestamp; import java.util.ArrayList; -import java.util.Collections; import java.util.EnumSet; import java.util.HashMap; import java.util.List; @@ -139,14 +134,13 @@ public class ChangeScreen2 extends Screen { @UiField Element actionDate; @UiField Actions actions; - @UiField Element revisionParent; - @UiField ListBox revisionList; @UiField Labels labels; @UiField CommitBox commit; @UiField RelatedChanges related; @UiField FileTable files; @UiField FlowPanel history; + @UiField Button revisions; @UiField Button download; @UiField Button reply; @UiField Button expandAll; @@ -155,6 +149,7 @@ public class ChangeScreen2 extends Screen { @UiField QuickApprove quickApprove; private ReplyAction replyAction; private EditMessageAction editMessageAction; + private RevisionsAction revisionsAction; private DownloadAction downloadAction; public ChangeScreen2(Change.Id changeId, String revision, boolean openReplyBox) { @@ -179,8 +174,10 @@ public class ChangeScreen2 extends Screen { void loadChangeInfo(boolean fg, AsyncCallback cb) { RestApi call = ChangeApi.detail(changeId.get()); ChangeList.addOptions(call, EnumSet.of( - ListChangesOption.ALL_REVISIONS, - ListChangesOption.CURRENT_ACTIONS)); + ListChangesOption.CURRENT_ACTIONS, + fg && revision != null + ? ListChangesOption.ALL_REVISIONS + : ListChangesOption.CURRENT_REVISION)); if (!fg) { call.background(); } @@ -247,7 +244,13 @@ public class ChangeScreen2 extends Screen { } } - private void renderDownload(ChangeInfo info, String revision) { + private void initRevisionsAction(ChangeInfo info, String revision) { + revisionsAction = new RevisionsAction( + info.legacy_id(), revision, + style, headerLine, revisions); + } + + private void initDownloadAction(ChangeInfo info, String revision) { downloadAction = new DownloadAction( info.legacy_id(), info.project(), @@ -341,14 +344,9 @@ public class ChangeScreen2 extends Screen { downloadAction.show(); } - @UiHandler("revisionList") - void onChangeRevision(ChangeEvent e) { - int idx = revisionList.getSelectedIndex(); - if (0 <= idx) { - String n = revisionList.getValue(idx); - revisionList.setEnabled(false); - Gerrit.display(PageLinks.toChange2(changeId, n)); - } + @UiHandler("revisions") + void onRevision(ClickEvent e) { + revisionsAction.show(); } @UiHandler("reply") @@ -597,9 +595,9 @@ public class ChangeScreen2 extends Screen { renderOwner(info); renderReviewers(info); renderActionTextDate(info); - renderDownload(info, revision); - renderRevisions(info); renderHistory(info); + initRevisionsAction(info, revision); + initDownloadAction(info, revision); actions.display(info, revision); star.setValue(info.starred()); @@ -660,35 +658,6 @@ public class ChangeScreen2 extends Screen { ccText.setInnerSafeHtml(labels.formatUserList(cc.values())); } - private void renderRevisions(ChangeInfo info) { - if (info.revisions().size() == 1) { - UIObject.setVisible(revisionParent, false); - return; - } - - JsArray list = info.revisions().values(); - RevisionInfo.sortRevisionInfoByNumber(list); - if (Gerrit.isSignedIn() - && Gerrit.getUserAccount().getGeneralPreferences() - .isReversePatchSetOrder()) { - Collections.reverse(Natives.asList(list)); - } - - int selected = -1; - for (int i = 0; i < list.length(); i++) { - RevisionInfo r = list.get(i); - revisionList.addItem( - r._number() + ": " + r.name().substring(0, 6), - "" + r._number()); - if (revision.equals(r.name())) { - selected = i; - } - } - if (0 <= selected) { - revisionList.setSelectedIndex(selected); - } - } - private void renderOwner(ChangeInfo info) { // TODO info card hover ownerText.setInnerText(info.owner().name() != null diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/ChangeScreen2.ui.xml b/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/ChangeScreen2.ui.xml index 8e4b8f4d07..881b15b1dc 100644 --- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/ChangeScreen2.ui.xml +++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/ChangeScreen2.ui.xml @@ -256,14 +256,14 @@ limitations under the License. -
-
- Revision -
+ + +
Revisions
+
Download
 -
+ diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/Constants.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/Constants.java index ce2fe7b075..ff00e92c3a 100644 --- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/Constants.java +++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/Constants.java @@ -19,4 +19,9 @@ interface Constants extends com.google.gwt.i18n.client.Constants { String nextChange(); String openChange(); String reviewedFileTitle(); + + String ps(); + String commit(); + String date(); + String author(); } diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/Constants.properties b/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/Constants.properties index 95f378cc7e..bb9450febb 100644 --- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/Constants.properties +++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/Constants.properties @@ -2,3 +2,8 @@ previousChange = Previous related change nextChange = Next related change openChange = Open related change reviewedFileTitle = Mark file as reviewed (Shortcut: r) + +ps = PS +commit = Commit +date = Date +author = Author diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/RevisionsAction.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/RevisionsAction.java new file mode 100644 index 0000000000..d66127b28f --- /dev/null +++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/RevisionsAction.java @@ -0,0 +1,37 @@ +// Copyright (C) 2013 The Android Open Source Project +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package com.google.gerrit.client.change; + +import com.google.gerrit.reviewdb.client.Change; +import com.google.gwt.user.client.ui.UIObject; +import com.google.gwt.user.client.ui.Widget; + +class RevisionsAction extends RightSidePopdownAction { + private final RevisionsBox revisionBox; + + RevisionsAction( + Change.Id changeId, + String revision, + ChangeScreen2.Style style, + UIObject relativeTo, + Widget downloadButton) { + super(style, relativeTo, downloadButton); + this.revisionBox = new RevisionsBox(changeId, revision); + } + + Widget getWidget() { + return revisionBox; + } +} diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/RevisionsBox.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/RevisionsBox.java new file mode 100644 index 0000000000..c0b2112844 --- /dev/null +++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/RevisionsBox.java @@ -0,0 +1,219 @@ +// Copyright (C) 2013 The Android Open Source Project +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package com.google.gerrit.client.change; + +import com.google.gerrit.client.FormatUtil; +import com.google.gerrit.client.Gerrit; +import com.google.gerrit.client.changes.ChangeApi; +import com.google.gerrit.client.changes.ChangeInfo; +import com.google.gerrit.client.changes.ChangeInfo.CommitInfo; +import com.google.gerrit.client.changes.ChangeInfo.RevisionInfo; +import com.google.gerrit.client.changes.ChangeList; +import com.google.gerrit.client.rpc.NativeMap; +import com.google.gerrit.client.rpc.Natives; +import com.google.gerrit.client.rpc.RestApi; +import com.google.gerrit.client.ui.FancyFlexTableImpl; +import com.google.gerrit.common.PageLinks; +import com.google.gerrit.common.changes.ListChangesOption; +import com.google.gerrit.reviewdb.client.Change; +import com.google.gwt.core.client.GWT; +import com.google.gwt.core.client.JsArray; +import com.google.gwt.dom.client.NativeEvent; +import com.google.gwt.resources.client.CssResource; +import com.google.gwt.uibinder.client.UiBinder; +import com.google.gwt.uibinder.client.UiField; +import com.google.gwt.user.client.DOM; +import com.google.gwt.user.client.Event; +import com.google.gwt.user.client.EventListener; +import com.google.gwt.user.client.rpc.AsyncCallback; +import com.google.gwt.user.client.ui.Composite; +import com.google.gwt.user.client.ui.FlexTable; +import com.google.gwt.user.client.ui.HTMLPanel; +import com.google.gwt.user.client.ui.PopupPanel; +import com.google.gwt.user.client.ui.Widget; +import com.google.gwt.user.client.ui.impl.HyperlinkImpl; +import com.google.gwtexpui.safehtml.client.SafeHtmlBuilder; + +import java.util.Collections; +import java.util.EnumSet; + +class RevisionsBox extends Composite { + interface Binder extends UiBinder {} + private static final Binder uiBinder = GWT.create(Binder.class); + + private static final String OPEN; + private static final HyperlinkImpl link = GWT.create(HyperlinkImpl.class); + + static { + OPEN = DOM.createUniqueId().replace('-', '_'); + init(OPEN); + } + + private static final native void init(String o) /*-{ + $wnd[o] = $entry(function(e,i) { + return @com.google.gerrit.client.change.RevisionsBox::onOpen(Lcom/google/gwt/dom/client/NativeEvent;I)(e,i); + }); + }-*/; + + private static boolean onOpen(NativeEvent e, int idx) { + if (link.handleAsClick(e. cast())) { + RevisionsBox t = getRevisionBox(e); + if (t != null) { + t.onOpenRow(idx); + e.preventDefault(); + return false; + } + } + return true; + } + + private static RevisionsBox getRevisionBox(NativeEvent event) { + com.google.gwt.user.client.Element e = event.getEventTarget().cast(); + for (e = DOM.getParent(e); e != null; e = DOM.getParent(e)) { + EventListener l = DOM.getEventListener(e); + if (l instanceof RevisionsBox) { + return (RevisionsBox) l; + } + } + return null; + } + + interface Style extends CssResource { + String current(); + String legacy_id(); + String commit(); + } + + private final Change.Id changeId; + private final String revision; + private boolean loaded; + private JsArray revisions; + + @UiField FlexTable table; + @UiField Style style; + + RevisionsBox(Change.Id changeId, String revision) { + this.changeId = changeId; + this.revision = revision; + initWidget(uiBinder.createAndBindUi(this)); + } + + @Override + protected void onLoad() { + if (!loaded) { + RestApi call = ChangeApi.detail(changeId.get()); + ChangeList.addOptions(call, EnumSet.of( + ListChangesOption.ALL_REVISIONS, + ListChangesOption.ALL_COMMITS)); + call.get(new AsyncCallback() { + @Override + public void onSuccess(ChangeInfo result) { + render(result.revisions()); + loaded = true; + } + + @Override + public void onFailure(Throwable caught) { + } + }); + } + } + + private void onOpenRow(int idx) { + closeParent(); + Gerrit.display(url(revisions.get(idx))); + } + + private void render(NativeMap map) { + map.copyKeysIntoChildren("name"); + + revisions = map.values(); + RevisionInfo.sortRevisionInfoByNumber(revisions); + Collections.reverse(Natives.asList(revisions)); + + SafeHtmlBuilder sb = new SafeHtmlBuilder(); + header(sb); + for (int i = 0; i < revisions.length(); i++) { + revision(sb, i, revisions.get(i)); + } + + GWT. create(FancyFlexTableImpl.class) + .resetHtml(table, sb); + } + + private void header(SafeHtmlBuilder sb) { + sb.openTr() + .openTh() + .setStyleName(style.legacy_id()) + .append(Resources.C.ps()) + .closeTh() + .openTh().append(Resources.C.commit()).closeTh() + .openTh().append(Resources.C.date()).closeTh() + .openTh().append(Resources.C.author()).closeTh() + .closeTr(); + } + + private void revision(SafeHtmlBuilder sb, int index, RevisionInfo r) { + CommitInfo c = r.commit(); + sb.openTr(); + if (revision.equals(r.name())) { + sb.setStyleName(style.current()); + } + + sb.openTd() + .setStyleName(style.legacy_id()) + .append(r._number()) + .closeTd(); + + sb.openTd() + .setStyleName(style.commit()) + .openAnchor() + .setAttribute("href", "#" + url(r)) + .setAttribute("onclick", OPEN + "(event," + index + ")") + .append(r.name().substring(0, 10)) + .closeAnchor() + .closeTd(); + + sb.openTd() + .append(FormatUtil.shortFormatDayTime(c.committer().date())) + .closeTd(); + + String an = c.author() != null ? c.author().name() : null; + String cn = c.committer() != null ? c.committer().name() : null; + sb.openTd(); + sb.append(an); + if (!"".equals(an) && !"".equals(cn) && !an.equals(cn)) { + sb.append(" / ").append(cn); + } + sb.closeTd(); + + sb.closeTr(); + } + + private String url(RevisionInfo r) { + return PageLinks.toChange2( + changeId, + String.valueOf(r._number())); + } + + private void closeParent() { + for (Widget w = getParent(); w != null; w = w.getParent()) { + if (w instanceof PopupPanel) { + ((PopupPanel) w).hide(true); + break; + } + } + } +} diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/RevisionsBox.ui.xml b/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/RevisionsBox.ui.xml new file mode 100644 index 0000000000..bf1ddd43b3 --- /dev/null +++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/RevisionsBox.ui.xml @@ -0,0 +1,65 @@ + + + + + + @eval selectionColor com.google.gerrit.client.Gerrit.getTheme().selectionColor; + + .revisionBox { + min-width: 300px; + margin: 10px 0px 5px 5px; + } + + .scroll { + min-width: 300px; + height: 200px; + } + + .table { + border-spacing: 0; + width: 100%; + } + + .table td, .table th { + padding-left: 5px; + padding-right: 5px; + border-right: 2px solid #ddd; + white-space: nowrap; + } + + .table tr.current { + background-color: selectionColor; + } + + .legacy_id { + min-width: 50px; + text-align: right; + font-weight: bold; + } + + .commit { + font-family: monospace; + } + + + + + + + diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/ui/FancyFlexTableImpl.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/ui/FancyFlexTableImpl.java index 2c7451a54d..2836e0f131 100644 --- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/ui/FancyFlexTableImpl.java +++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/ui/FancyFlexTableImpl.java @@ -14,13 +14,13 @@ package com.google.gerrit.client.ui; -import com.google.gerrit.client.ui.FancyFlexTable.MyFlexTable; import com.google.gwt.user.client.Element; +import com.google.gwt.user.client.ui.FlexTable; import com.google.gwt.user.client.ui.HTMLTable; import com.google.gwtexpui.safehtml.client.SafeHtml; public class FancyFlexTableImpl { - public void resetHtml(final MyFlexTable myTable, final SafeHtml body) { + public void resetHtml(final FlexTable myTable, final SafeHtml body) { SafeHtml.setInnerHTML(getBodyElement(myTable), body); } diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/ui/FancyFlexTableImplIE6.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/ui/FancyFlexTableImplIE6.java index 17e8ddd723..34b6bee046 100644 --- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/ui/FancyFlexTableImplIE6.java +++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/ui/FancyFlexTableImplIE6.java @@ -14,16 +14,16 @@ package com.google.gerrit.client.ui; -import com.google.gerrit.client.ui.FancyFlexTable.MyFlexTable; import com.google.gwt.user.client.DOM; import com.google.gwt.user.client.Element; +import com.google.gwt.user.client.ui.FlexTable; import com.google.gwt.user.client.ui.HTMLTable; import com.google.gwtexpui.safehtml.client.SafeHtml; import com.google.gwtexpui.safehtml.client.SafeHtmlBuilder; public class FancyFlexTableImplIE6 extends FancyFlexTableImpl { @Override - public void resetHtml(final MyFlexTable myTable, final SafeHtml bodyHtml) { + public void resetHtml(final FlexTable myTable, final SafeHtml bodyHtml) { final Element oldBody = getBodyElement(myTable); final Element newBody = parseBody(bodyHtml); assert newBody != null; From 4ecb8eef0614ca919560913aea363b69e956d72d Mon Sep 17 00:00:00 2001 From: Shawn Pearce Date: Thu, 29 Aug 2013 14:32:11 -0700 Subject: [PATCH 2/2] ChangeScreen2: Use min-width to prevent button resizing When a popdown is selected the font is changed to bold, this makes the button slightly wider than before. Default the button to 100px to avoid the resizing jitter. Change-Id: Ia7755e80534ae050f36e53836a86c48999239829 --- .../java/com/google/gerrit/client/change/ChangeScreen2.ui.xml | 1 + 1 file changed, 1 insertion(+) diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/ChangeScreen2.ui.xml b/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/ChangeScreen2.ui.xml index 881b15b1dc..b24b311f9e 100644 --- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/ChangeScreen2.ui.xml +++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/ChangeScreen2.ui.xml @@ -97,6 +97,7 @@ limitations under the License. margin: 0; padding-left: 2px; padding-right: 2px; + min-width: 100px; } .popdown button div { padding-left: 6px;