From f7d6301e8b886f9e4575a927e0568269ba2bb20e Mon Sep 17 00:00:00 2001 From: Michael Zhou Date: Wed, 31 Jul 2013 16:43:18 -0700 Subject: [PATCH] Implement PatchSetSelectBox2 Implemented a new version of PatchSetSelectBox. Used InlineHyperlink with actual hrefs so that they can be opened in new tabs / windows. The download link hasn't been implemented yet. Change-Id: I2da78663f0fbd6a6713129dff945b2a161570430 --- .../com/google/gerrit/client/Dispatcher.java | 15 ++- .../gerrit/client/change/ChangeScreen2.java | 9 +- .../gerrit/client/changes/ChangeInfo.java | 12 ++ .../google/gerrit/client/diff/DiffTable.java | 31 +++-- .../gerrit/client/diff/DiffTable.ui.xml | 10 +- .../google/gerrit/client/diff/FileInfo.java | 9 ++ .../com/google/gerrit/client/diff/Header.java | 10 +- .../gerrit/client/diff/PatchSelectBox2.ui.xml | 24 ---- .../client/diff/PatchSetSelectBox2.java | 118 ++++++++++++++++++ .../client/diff/PatchSetSelectBox2.ui.xml | 73 +++++++++++ .../gerrit/client/diff/SideBySide2.java | 48 ++++--- 11 files changed, 281 insertions(+), 78 deletions(-) delete mode 100644 gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/PatchSelectBox2.ui.xml create mode 100644 gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/PatchSetSelectBox2.java create mode 100644 gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/PatchSetSelectBox2.ui.xml diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/Dispatcher.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/Dispatcher.java index d28c4d90c8..b16dffe270 100644 --- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/Dispatcher.java +++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/Dispatcher.java @@ -102,6 +102,11 @@ public class Dispatcher { return toPatch("", diffBase, id); } + public static String toPatchSideBySide2(PatchSet.Id diffBase, + PatchSet.Id revision, String fileName) { + return toPatch("cm", diffBase, revision, fileName); + } + public static String toPatchUnified(final Patch.Key id) { return toPatch("unified", null, id); } @@ -111,14 +116,18 @@ public class Dispatcher { } private static String toPatch(String type, PatchSet.Id diffBase, Patch.Key id) { - PatchSet.Id ps = id.getParentKey(); - Change.Id c = ps.getParentKey(); + return toPatch(type, diffBase, id.getParentKey(), id.get()); + } + + private static String toPatch(String type, PatchSet.Id diffBase, + PatchSet.Id revision, String fileName) { + Change.Id c = revision.getParentKey(); StringBuilder p = new StringBuilder(); p.append("/c/").append(c).append("/"); if (diffBase != null) { p.append(diffBase.get()).append(".."); } - p.append(ps.get()).append("/").append(KeyUtil.encode(id.get())); + p.append(revision.get()).append("/").append(KeyUtil.encode(fileName)); if (type != null && !type.isEmpty()) { p.append(",").append(type); } 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 a7f0ef8aeb..996537f7f3 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 @@ -84,8 +84,6 @@ import com.google.gwtorm.client.KeyUtil; import java.sql.Timestamp; import java.util.ArrayList; -import java.util.Collections; -import java.util.Comparator; import java.util.EnumSet; import java.util.HashMap; import java.util.List; @@ -645,12 +643,7 @@ public class ChangeScreen2 extends Screen { } JsArray list = info.revisions().values(); - Collections.sort(Natives.asList(list), new Comparator() { - @Override - public int compare(RevisionInfo a, RevisionInfo b) { - return a._number() - b._number(); - } - }); + RevisionInfo.sortRevisionInfoByNumber(list); int selected = -1; for (int i = 0; i < list.length(); i++) { diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/ChangeInfo.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/ChangeInfo.java index a2d5d64f99..c0c53125ea 100644 --- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/ChangeInfo.java +++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/ChangeInfo.java @@ -29,6 +29,8 @@ import com.google.gwt.core.client.JsArrayString; import com.google.gwtjsonrpc.client.impl.ser.JavaSqlTimestamp_JsonSerializer; import java.sql.Timestamp; +import java.util.Collections; +import java.util.Comparator; import java.util.Set; import java.util.SortedSet; import java.util.TreeSet; @@ -209,12 +211,22 @@ public class ChangeInfo extends JavaScriptObject { public final native boolean has_actions() /*-{ return this.hasOwnProperty('actions') }-*/; public final native NativeMap actions() /*-{ return this.actions; }-*/; + public static void sortRevisionInfoByNumber(JsArray list) { + Collections.sort(Natives.asList(list), new Comparator() { + @Override + public int compare(RevisionInfo a, RevisionInfo b) { + return a._number() - b._number(); + } + }); + } + protected RevisionInfo () { } } public static class CommitInfo extends JavaScriptObject { public final native String commit() /*-{ return this.commit; }-*/; + public final native JsArray parents() /*-{ return this.parents; }-*/; public final native GitPerson author() /*-{ return this.author; }-*/; public final native GitPerson committer() /*-{ return this.committer; }-*/; public final native String subject() /*-{ return this.subject; }-*/; diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/DiffTable.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/DiffTable.java index e462e2b59f..da4d525f1d 100644 --- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/DiffTable.java +++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/DiffTable.java @@ -14,8 +14,11 @@ package com.google.gerrit.client.diff; +import com.google.gerrit.client.changes.ChangeInfo.RevisionInfo; import com.google.gerrit.client.diff.SideBySide2.DisplaySide; +import com.google.gerrit.reviewdb.client.PatchSet; import com.google.gwt.core.client.GWT; +import com.google.gwt.core.client.JsArray; import com.google.gwt.dom.client.Element; import com.google.gwt.resources.client.CssResource; import com.google.gwt.uibinder.client.UiBinder; @@ -55,19 +58,19 @@ class DiffTable extends Composite { SidePanel sidePanel; @UiField - Element patchsetNavRow; + Element patchSetNavRow; @UiField - Element patchsetNavCellA; + Element patchSetNavCellA; @UiField - Element patchsetNavCellB; + Element patchSetNavCellB; @UiField(provided = true) - PatchSelectBox2 patchSelectBoxA; + PatchSetSelectBox2 patchSetSelectBoxA; @UiField(provided = true) - PatchSelectBox2 patchSelectBoxB; + PatchSetSelectBox2 patchSetSelectBoxB; @UiField Element fileCommentRow; @@ -89,9 +92,12 @@ class DiffTable extends Composite { private SideBySide2 host; - DiffTable(SideBySide2 host, String path) { - patchSelectBoxA = new PatchSelectBox2(this, DisplaySide.A); - patchSelectBoxB = new PatchSelectBox2(this, DisplaySide.B); + DiffTable(SideBySide2 host, PatchSet.Id base, PatchSet.Id revision, String path) { + patchSetSelectBoxA = new PatchSetSelectBox2( + this, DisplaySide.A, revision.getParentKey(), base, path); + patchSetSelectBoxB = new PatchSetSelectBox2( + this, DisplaySide.B, revision.getParentKey(), revision, path); + PatchSetSelectBox2.link(patchSetSelectBoxA, patchSetSelectBoxB); fileCommentPanelA = new FileCommentPanel(host, this, path, DisplaySide.A); fileCommentPanelB = new FileCommentPanel(host, this, path, DisplaySide.B); initWidget(uiBinder.createAndBindUi(this)); @@ -104,7 +110,7 @@ class DiffTable extends Composite { } void updateFileCommentVisibility(boolean forceHide) { - UIObject.setVisible(patchsetNavRow, !forceHide); + UIObject.setVisible(patchSetNavRow, !forceHide); if (forceHide || (fileCommentPanelA.getBoxCount() == 0 && fileCommentPanelB.getBoxCount() == 0)) { UIObject.setVisible(fileCommentRow, false); @@ -132,7 +138,12 @@ class DiffTable extends Composite { } int getHeaderHeight() { - return fileCommentRow.getOffsetHeight() + patchSelectBoxA.getOffsetHeight(); + return fileCommentRow.getOffsetHeight() + patchSetSelectBoxA.getOffsetHeight(); + } + + void setUpPatchSetNav(JsArray list) { + patchSetSelectBoxA.setUpPatchSetNav(list); + patchSetSelectBoxB.setUpPatchSetNav(list); } void add(Widget widget) { diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/DiffTable.ui.xml b/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/DiffTable.ui.xml index 4714fca5b7..070d10c0e1 100644 --- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/DiffTable.ui.xml +++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/DiffTable.ui.xml @@ -118,12 +118,12 @@ limitations under the License. - - + - diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/FileInfo.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/FileInfo.java index 4fa6596956..4e88caf0cb 100644 --- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/FileInfo.java +++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/FileInfo.java @@ -14,6 +14,7 @@ package com.google.gerrit.client.diff; +import com.google.gerrit.client.changes.Util; import com.google.gerrit.client.rpc.Natives; import com.google.gerrit.reviewdb.client.Patch; import com.google.gwt.core.client.JavaScriptObject; @@ -46,6 +47,14 @@ public class FileInfo extends JavaScriptObject { }); } + public static String getFileName(String path) { + String fileName = Patch.COMMIT_MSG.equals(path) + ? Util.C.commitMessage() + : path; + int s = fileName.lastIndexOf('/'); + return s >= 0 ? fileName.substring(s + 1) : fileName; + } + protected FileInfo() { } } diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/Header.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/Header.java index 0f4616248c..f0da57e22a 100644 --- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/Header.java +++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/Header.java @@ -169,7 +169,7 @@ class Header extends Composite { if (info != null) { final String url = url(info); link.setTargetHistoryToken(url); - link.setTitle(getFileName(info.path())); + link.setTitle(FileInfo.getFileName(info.path())); keys.add(new KeyCommand(0, key, help) { @Override public void onKeyPress(KeyPressEvent event) { @@ -187,14 +187,6 @@ class Header extends Composite { } } - private static String getFileName(String path) { - String fileName = Patch.COMMIT_MSG.equals(path) - ? Util.C.commitMessage() - : path; - int s = fileName.lastIndexOf('/'); - return s >= 0 ? fileName.substring(s + 1) : fileName; - } - boolean hasPrev() { return hasPrev; } diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/PatchSelectBox2.ui.xml b/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/PatchSelectBox2.ui.xml deleted file mode 100644 index 28435ca08e..0000000000 --- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/PatchSelectBox2.ui.xml +++ /dev/null @@ -1,24 +0,0 @@ - - - - - - - - \ No newline at end of file diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/PatchSetSelectBox2.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/PatchSetSelectBox2.java new file mode 100644 index 0000000000..a618156bf9 --- /dev/null +++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/PatchSetSelectBox2.java @@ -0,0 +1,118 @@ +//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.diff; + +import com.google.gerrit.client.Dispatcher; +import com.google.gerrit.client.Gerrit; +import com.google.gerrit.client.changes.ChangeInfo.RevisionInfo; +import com.google.gerrit.client.diff.SideBySide2.DisplaySide; +import com.google.gerrit.client.patches.PatchUtil; +import com.google.gerrit.client.ui.InlineHyperlink; +import com.google.gerrit.reviewdb.client.Change; +import com.google.gerrit.reviewdb.client.PatchSet; +import com.google.gwt.core.client.GWT; +import com.google.gwt.core.client.JsArray; +import com.google.gwt.event.dom.client.ClickEvent; +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.uibinder.client.UiHandler; +import com.google.gwt.user.client.ui.Composite; +import com.google.gwt.user.client.ui.HTMLPanel; +import com.google.gwt.user.client.ui.Image; + +/** + * HTMLPanel to select among patch sets + * TODO: Implement download link. + */ +class PatchSetSelectBox2 extends Composite { + interface Binder extends UiBinder {} + private static final Binder uiBinder = GWT.create(Binder.class); + + interface BoxStyle extends CssResource { + String selected(); + } + + @UiField Image icon; + @UiField HTMLPanel linkPanel; + @UiField BoxStyle style; + + private DiffTable table; + private DisplaySide side; + private boolean sideA; + private String path; + private Change.Id changeId; + private PatchSet.Id revision; + private PatchSet.Id idActive; + private PatchSetSelectBox2 other; + + PatchSetSelectBox2(DiffTable table, final DisplaySide side, + final Change.Id changeId, final PatchSet.Id revision, String path) { + initWidget(uiBinder.createAndBindUi(this)); + icon.setTitle(PatchUtil.C.addFileCommentToolTip()); + icon.addStyleName(Gerrit.RESOURCES.css().link()); + this.table = table; + this.side = side; + this.sideA = side == DisplaySide.A; + this.changeId = changeId; + this.revision = revision; + this.idActive = (sideA && revision == null) ? null : revision; + this.path = path; + } + + void setUpPatchSetNav(JsArray list) { + InlineHyperlink baseLink = null; + InlineHyperlink selectedLink = null; + if (sideA) { + baseLink = createLink(PatchUtil.C.patchBase(), null); + linkPanel.add(baseLink); + } + for (int i = 0; i < list.length(); i++) { + RevisionInfo r = list.get(i); + InlineHyperlink link = createLink( + String.valueOf(r._number()), new PatchSet.Id(changeId, r._number())); + linkPanel.add(link); + if (revision != null && r._number() == revision.get()) { + selectedLink = link; + } + } + if (selectedLink != null) { + selectedLink.setStyleName(style.selected()); + } else if (sideA) { + baseLink.setStyleName(style.selected()); + } + } + + static void link(PatchSetSelectBox2 a, PatchSetSelectBox2 b) { + a.other = b; + b.other = a; + } + + private InlineHyperlink createLink(String label, PatchSet.Id id) { + assert other != null; + if (sideA) { + assert other.idActive != null; + } + return new InlineHyperlink(label, Dispatcher.toPatchSideBySide2( + sideA ? id : other.idActive, + sideA ? other.idActive : id, + path)); + } + + @UiHandler("icon") + void onIconClick(ClickEvent e) { + table.createOrEditFileComment(side); + } +} diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/PatchSetSelectBox2.ui.xml b/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/PatchSetSelectBox2.ui.xml new file mode 100644 index 0000000000..dca0cd58c7 --- /dev/null +++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/PatchSetSelectBox2.ui.xml @@ -0,0 +1,73 @@ + + + + + + + @eval selectionColor com.google.gerrit.client.Gerrit.getTheme().selectionColor; + .table { + width: 100%; + } + .linkCell { + text-align: center; + font-size: 12px; + white-space: normal; + font-family: sans-serif; + font-weight: bold; + } + .linkCell div { + padding-left: 3px; + padding-right: 3px; + vertical-align: middle; + display: inline-block; + } + .linkCell a { + padding-left: 3px; + padding-right: 3px; + text-decoration: none; + vertical-align: middle; + display: inline-block; + } + .selected { + font-weight: bold; + background-color: selectionColor; + } + .hidden { + visibility: hidden; + } + .iconCell { + width: 16px; + } + + +
- +
+ - + +
+ + +
+ + + + + + + +
+ + \ No newline at end of file diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/SideBySide2.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/SideBySide2.java index 8972102151..441dc8704a 100644 --- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/SideBySide2.java +++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/SideBySide2.java @@ -16,6 +16,10 @@ package com.google.gerrit.client.diff; import com.google.gerrit.client.Gerrit; import com.google.gerrit.client.change.ChangeScreen2; +import com.google.gerrit.client.changes.ChangeApi; +import com.google.gerrit.client.changes.ChangeInfo; +import com.google.gerrit.client.changes.ChangeInfo.RevisionInfo; +import com.google.gerrit.client.changes.ChangeList; import com.google.gerrit.client.changes.CommentApi; import com.google.gerrit.client.changes.CommentInfo; import com.google.gerrit.client.diff.DiffInfo.Region; @@ -29,10 +33,12 @@ import com.google.gerrit.client.projects.ConfigInfoCache; import com.google.gerrit.client.rpc.CallbackGroup; import com.google.gerrit.client.rpc.GerritCallback; import com.google.gerrit.client.rpc.NativeMap; +import com.google.gerrit.client.rpc.RestApi; import com.google.gerrit.client.rpc.ScreenLoadCallback; import com.google.gerrit.client.ui.CommentLinkProcessor; import com.google.gerrit.client.ui.Screen; import com.google.gerrit.common.PageLinks; +import com.google.gerrit.common.changes.ListChangesOption; import com.google.gerrit.common.changes.Side; import com.google.gerrit.reviewdb.client.AccountDiffPreference; import com.google.gerrit.reviewdb.client.Change; @@ -85,6 +91,7 @@ import net.codemirror.lib.TextMarker.FromTo; import java.util.ArrayList; import java.util.Collections; import java.util.Comparator; +import java.util.EnumSet; import java.util.HashMap; import java.util.List; import java.util.Map; @@ -106,6 +113,7 @@ public class SideBySide2 extends Screen { @UiField(provided = true) DiffTable diffTable; + private final Change.Id changeId; private final PatchSet.Id base; private final PatchSet.Id revision; private final String path; @@ -145,6 +153,7 @@ public class SideBySide2 extends Screen { String path) { this.base = base; this.revision = revision; + this.changeId = revision.getParentKey(); this.path = path; pref = Gerrit.getAccountDiffPreference(); @@ -158,7 +167,7 @@ public class SideBySide2 extends Screen { addDomHandler(GlobalKey.STOP_PROPAGATION, KeyPressEvent.getType()); keysNavigation = new KeyCommandSet(Gerrit.C.sectionNavigation()); add(header = new Header(keysNavigation, revision, path)); - add(diffTable = new DiffTable(this, path)); + add(diffTable = new DiffTable(this, base, revision, path)); add(uiBinder.createAndBindUi(this)); } @@ -206,7 +215,7 @@ public class SideBySide2 extends Screen { CommentApi.drafts(revision, group.add(getCommentCallback(DisplaySide.B, true))); } - ConfigInfoCache.get(revision.getParentKey(), group.addFinal( + ConfigInfoCache.get(changeId, group.addFinal( new ScreenLoadCallback(SideBySide2.this) { @Override protected void preDisplay(ConfigInfoCache.Entry result) { @@ -218,6 +227,18 @@ public class SideBySide2 extends Screen { display(diffInfo); } })); + + RestApi call = ChangeApi.detail(changeId.get()); + ChangeList.addOptions(call, EnumSet.of( + ListChangesOption.ALL_REVISIONS)); + call.get(new GerritCallback() { + @Override + public void onSuccess(ChangeInfo info) { + info.revisions().copyKeysIntoChildren("name"); + JsArray list = info.revisions().values(); + RevisionInfo.sortRevisionInfoByNumber(list); + diffTable.setUpPatchSetNav(list); + }}); } @Override @@ -297,8 +318,8 @@ public class SideBySide2 extends Screen { } }); cm.addKeyMap(KeyMap.create() - .on("'a'", openReplyBox()) - .on("'u'", upToChange()) + .on("'a'", upToChange(true)) + .on("'u'", upToChange(false)) .on("'r'", toggleReviewed()) .on("'o'", toggleOpenBox(cm)) .on("Enter", toggleOpenBox(cm)) @@ -354,7 +375,7 @@ public class SideBySide2 extends Screen { keysAction.add(new KeyCommand(0, 'a', PatchUtil.C.openReply()) { @Override public void onKeyPress(KeyPressEvent event) { - openReplyBox().run(); + upToChange(true).run(); } }); @@ -1074,14 +1095,13 @@ public class SideBySide2 extends Screen { }; } - private Runnable openReplyBox() { + private Runnable upToChange(final boolean openReplyBox) { return new Runnable() { public void run() { - Change.Id id = revision.getParentKey(); String rev = String.valueOf(revision.get()); Gerrit.display( - PageLinks.toChange2(id, rev), - new ChangeScreen2(id, rev, true)); + PageLinks.toChange2(changeId, rev), + new ChangeScreen2(changeId, rev, openReplyBox)); } }; } @@ -1111,16 +1131,6 @@ public class SideBySide2 extends Screen { }; } - private Runnable upToChange() { - return new Runnable() { - public void run() { - Gerrit.display(PageLinks.toChange2( - revision.getParentKey(), - String.valueOf(revision.get()))); - } - }; - } - private Runnable toggleReviewed() { return new Runnable() { public void run() {