opendev/gerrit
Code Issues Proposed changes

Unify SideBySide2 header widgets

Browse Source 
Combine the two header widgets into a single panel and compact
the entire display to take up less overall screen space. The
header is now only a single line of text.

The format looks like:

  [x] path/of/FILE                                   < ^ >

The reviewed checkbox is on the left next to the file name.

The path uses plain text for the directory part and BOLD for the
file name itself. This makes it easier to spot the specific file
name being reviewed.

The navigation arrows are now clustered on the right using a
small icon for each. This saves a lot of screen space.

Change-Id: Ibd6b7e1022bfe3d6e97638df1fc94f492dae9af1
This commit is contained in:
Shawn Pearce
2013-08-09 15:39:36 -07:00
parent 7fd72a6b4c
commit 45b5ddeae8
10 changed files with 140 additions and 231 deletions
Download Patch File Download Diff File Expand all files Collapse all files

16
gerrit-gwtexpui/src/main/java/com/google/gwtexpui/safehtml/client/SafeHtml.java
View File

@@ -15,10 +15,10 @@
package com.google.gwtexpui.safehtml.client;
import com.google.gwt.core.client.GWT;
import com.google.gwt.dom.client.Element;
import com.google.gwt.regexp.shared.MatchResult;
import com.google.gwt.regexp.shared.RegExp;
import com.google.gwt.user.client.DOM;
import com.google.gwt.user.client.Element;
import com.google.gwt.user.client.ui.HTML;
import com.google.gwt.user.client.ui.HTMLTable;
import com.google.gwt.user.client.ui.HasHTML;
@@ -86,13 +86,13 @@ public abstract class SafeHtml
}
/** @return the existing inner HTML of any element. */
public static SafeHtml get(final Element e) {
return new SafeHtmlString(DOM.getInnerHTML(e));
public static SafeHtml get(Element e) {
return new SafeHtmlString(e.getInnerHTML());
}
/** Set the inner HTML of any element. */
public static Element set(final Element e, final SafeHtml str) {
DOM.setInnerHTML(e, str.asString());
public static Element setInnerHTML(Element e, SafeHtml str) {
e.setInnerHTML(str.asString());
return e;
}
@@ -109,8 +109,10 @@ public abstract class SafeHtml
}
/** Parse an HTML block and return the first (typically root) element. */
public static Element parse(final SafeHtml str) {
return DOM.getFirstChild(set(DOM.createDiv(), str));
public static com.google.gwt.user.client.Element parse(SafeHtml html) {
com.google.gwt.user.client.Element e = DOM.createDiv();
setInnerHTML(e, html);
return DOM.getFirstChild(e);
}
/** Convert bare http:// and https:// URLs into &lt;a href&gt; tags. */

2
gerrit-gwtui/src/main/java/com/google/gerrit/client/change/ChangeScreen2.ui.xml
View File

@@ -213,7 +213,7 @@ limitations under the License.
<div class='{style.headerButtons}'>
<g:Button ui:field='reply'
styleName=''
title='Reply and score (Shortcut: r)'>
title='Reply and score (Shortcut: a)'>
<ui:attribute name='title'/>
<div><ui:msg>Reply&#8230;</ui:msg></div>
</g:Button>

85
gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/NavLinks2.java → gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/Header.java
View File

@@ -16,10 +16,13 @@ package com.google.gerrit.client.diff;
import com.google.gerrit.client.Gerrit;
import com.google.gerrit.client.changes.ChangeApi;
import com.google.gerrit.client.changes.ReviewInfo;
import com.google.gerrit.client.changes.Util;
import com.google.gerrit.client.patches.PatchUtil;
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.ui.InlineHyperlink;
import com.google.gerrit.common.PageLinks;
import com.google.gerrit.reviewdb.client.Change;
@@ -27,9 +30,14 @@ import com.google.gerrit.reviewdb.client.Patch;
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.dom.client.Style.Visibility;
import com.google.gwt.event.dom.client.KeyPressEvent;
import com.google.gwt.event.logical.shared.ValueChangeEvent;
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.CheckBox;
import com.google.gwt.user.client.ui.Composite;
import com.google.gwt.user.client.ui.HTMLPanel;
import com.google.gwtexpui.globalkey.client.KeyCommand;
@@ -38,28 +46,47 @@ import com.google.gwtexpui.safehtml.client.SafeHtml;
import com.google.gwtexpui.safehtml.client.SafeHtmlBuilder;
import com.google.gwtorm.client.KeyUtil;
class NavLinks2 extends Composite {
interface Binder extends UiBinder<HTMLPanel, NavLinks2> {}
class Header extends Composite {
interface Binder extends UiBinder<HTMLPanel, Header> {}
private static final Binder uiBinder = GWT.create(Binder.class);
@UiField InlineHyperlink prevLink;
@UiField InlineHyperlink nextLink;
@UiField InlineHyperlink upLink;
@UiField CheckBox reviewed;
@UiField Element filePath;
@UiField InlineHyperlink prev;
@UiField InlineHyperlink up;
@UiField InlineHyperlink next;
private final KeyCommandSet keys;
private final PatchSet.Id patchSetId;
private final String path;
NavLinks2(KeyCommandSet keys, PatchSet.Id patchSetId, String path) {
Header(KeyCommandSet keys, PatchSet.Id patchSetId, String path) {
initWidget(uiBinder.createAndBindUi(this));
this.keys = keys;
this.patchSetId = patchSetId;
this.path = path;
upLink.setTargetHistoryToken(PageLinks.toChange2(
SafeHtml.setInnerHTML(filePath, formatPath(path));
up.setTargetHistoryToken(PageLinks.toChange2(
patchSetId.getParentKey(),
String.valueOf(patchSetId.get())));
}
private static SafeHtml formatPath(String path) {
SafeHtmlBuilder b = new SafeHtmlBuilder();
if (Patch.COMMIT_MSG.equals(path)) {
return b.append(Util.C.commitMessage());
}
int s = path.lastIndexOf('/') + 1;
b.append(path.substring(0, s));
b.openElement("b");
b.append(path.substring(s));
b.closeElement("b");
return b;
}
@Override
protected void onLoad() {
ChangeApi.revision(patchSetId).view("files").get(
@@ -75,14 +102,35 @@ class NavLinks2 extends Composite {
index = i;
}
}
setupNav('[', PatchUtil.C.previousFileHelp(),
setupNav(prev, '[', PatchUtil.C.previousFileHelp(),
index == 0 ? null : files.get(index - 1));
setupNav(']', PatchUtil.C.nextFileHelp(),
setupNav(next, ']', PatchUtil.C.nextFileHelp(),
index == files.length() - 1 ? null : files.get(index + 1));
}
});
}
void setReviewed(boolean r) {
reviewed.setValue(r, true);
}
boolean isReviewed() {
return reviewed.getValue();
}
@UiHandler("reviewed")
void onValueChange(ValueChangeEvent<Boolean> event) {
RestApi api = ChangeApi.revision(patchSetId)
.view("files")
.id(path)
.view("reviewed");
if (event.getValue()) {
api.put(CallbackGroup.<ReviewInfo>emptyCallback());
} else {
api.delete(CallbackGroup.<ReviewInfo>emptyCallback());
}
}
private String url(FileInfo info) {
Change.Id c = patchSetId.getParentKey();
StringBuilder p = new StringBuilder();
@@ -92,21 +140,11 @@ class NavLinks2 extends Composite {
return p.toString();
}
private void setupNav(int key, String help, FileInfo info) {
private void setupNav(InlineHyperlink link, int key, String help, FileInfo info) {
if (info != null) {
final String url = url(info);
String fileName = getFileNameOnly(info.path());
if (key == '[') {
prevLink.setTargetHistoryToken(url);
prevLink.setHTML(new SafeHtmlBuilder()
.append(SafeHtml.asis(Util.C.prevPatchLinkIcon()))
.append(fileName));
} else {
nextLink.setTargetHistoryToken(url);
nextLink.setHTML(new SafeHtmlBuilder()
.append(fileName)
.append(SafeHtml.asis(Util.C.nextPatchLinkIcon())));
}
link.setTargetHistoryToken(url);
link.setTitle(getFileName(info.path()));
keys.add(new KeyCommand(0, key, help) {
@Override
public void onKeyPress(KeyPressEvent event) {
@@ -114,11 +152,12 @@ class NavLinks2 extends Composite {
}
});
} else {
link.getElement().getStyle().setVisibility(Visibility.HIDDEN);
keys.add(new UpToChangeCommand2(patchSetId, 0, key));
}
}
private static String getFileNameOnly(String path) {
private static String getFileName(String path) {
String fileName = Patch.COMMIT_MSG.equals(path)
? Util.C.commitMessage()
: path;

55
gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/Header.ui.xml Normal file
View File

@@ -0,0 +1,55 @@
<?xml version="1.0" encoding="UTF-8"?>
<!--
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.
-->
<ui:UiBinder
xmlns:ui='urn:ui:com.google.gwt.uibinder'
xmlns:g='urn:import:com.google.gwt.user.client.ui'
xmlns:x='urn:import:com.google.gerrit.client.ui'>
<ui:style>
.header {
position: relative;
}
.reviewed input {
margin: 0;
padding: 0;
vertical-align: bottom;
}
.path {
}
.navigation {
position: absolute;
top: 0;
right: 15px;
}
</ui:style>
<g:HTMLPanel styleName='{style.header}'>
<g:CheckBox ui:field='reviewed'
styleName='{style.reviewed}'
title='Mark file as reviewed (Shortcut: r)'>
<ui:attribute name='title'/>
</g:CheckBox>
<span ui:field='filePath' class='{style.path}'/>
<div class='{style.navigation}'>
<x:InlineHyperlink ui:field='prev'>&#x21e6;</x:InlineHyperlink>
<x:InlineHyperlink ui:field='up' title='Up to change (Shortcut: u)'>
<ui:attribute name='title'/>
&#x21e7;
</x:InlineHyperlink>
<x:InlineHyperlink ui:field='next'>&#x21e8;</x:InlineHyperlink>
</div>
</g:HTMLPanel>
</ui:UiBinder>

53
gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/NavLinks2.ui.xml
View File

@@ -1,53 +0,0 @@
<?xml version="1.0" encoding="UTF-8"?>
<!--
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.
-->
<ui:UiBinder
xmlns:ui='urn:ui:com.google.gwt.uibinder'
xmlns:g='urn:import:com.google.gwt.user.client.ui'
xmlns:ge='urn:import:com.google.gerrit.client.ui'>
<ui:style>
.table {
table-layout: fixed;
width: 100%;
}
.prev {
text-align: left;
}
.up {
text-align: center;
}
.next {
text-align: right;
}
</ui:style>
<g:HTMLPanel>
<table ui:field='table' class='{style.table}'>
<tr>
<td class='{style.prev}'>
<ge:InlineHyperlink ui:field='prevLink'/>
</td>
<td class='{style.up}'>
<ge:InlineHyperlink ui:field='upLink'>
<ui:msg>&#x21e7;Up to change</ui:msg>
</ge:InlineHyperlink>
</td>
<td class='{style.next}'>
<ge:InlineHyperlink ui:field='nextLink'/>
</td>
</tr>
</table>
</g:HTMLPanel>
</ui:UiBinder>

86
gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/ReviewedPanel.java
View File

@@ -1,86 +0,0 @@
// 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.changes.ChangeApi;
import com.google.gerrit.client.changes.ReviewInfo;
import com.google.gerrit.client.changes.Util;
import com.google.gerrit.client.patches.PatchUtil;
import com.google.gerrit.client.rpc.CallbackGroup;
import com.google.gerrit.client.rpc.RestApi;
import com.google.gerrit.reviewdb.client.PatchSet;
import com.google.gwt.core.client.GWT;
import com.google.gwt.dom.client.Element;
import com.google.gwt.event.dom.client.ClickEvent;
import com.google.gwt.event.logical.shared.ValueChangeEvent;
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.Anchor;
import com.google.gwt.user.client.ui.CheckBox;
import com.google.gwt.user.client.ui.Composite;
import com.google.gwt.user.client.ui.HTMLPanel;
class ReviewedPanel extends Composite {
interface Binder extends UiBinder<HTMLPanel, ReviewedPanel> {}
private static UiBinder<HTMLPanel, ReviewedPanel> uiBinder =
GWT.create(Binder.class);
@UiField
Element fileName;
@UiField
CheckBox checkBox;
@UiField
Anchor nextLink;
private PatchSet.Id patchId;
private String fileId;
ReviewedPanel(PatchSet.Id id, String path) {
initWidget(uiBinder.createAndBindUi(this));
patchId = id;
fileId = path;
fileName.setInnerText(path);
nextLink.setHTML(PatchUtil.C.next() + Util.C.nextPatchLinkIcon());
}
void setReviewed(boolean reviewed) {
checkBox.setValue(reviewed, true);
}
boolean isReviewed() {
return checkBox.getValue();
}
@UiHandler("checkBox")
void onValueChange(ValueChangeEvent<Boolean> event) {
RestApi api = ChangeApi.revision(patchId)
.view("files")
.id(fileId)
.view("reviewed");
if (event.getValue()) {
api.put(CallbackGroup.<ReviewInfo>emptyCallback());
} else {
api.delete(CallbackGroup.<ReviewInfo>emptyCallback());
}
}
// TODO: Implement this to go to the next file in the patchset.
void onNext(ClickEvent e) {
setReviewed(true);
}
}

45
gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/ReviewedPanel.ui.xml
View File

@@ -1,45 +0,0 @@
<?xml version="1.0" encoding="UTF-8"?>
<!DOCTYPE ui:UiBinder SYSTEM "http://dl.google.com/gwt/DTD/xhtml.ent">
<!--
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.
-->
<ui:UiBinder xmlns:ui='urn:ui:com.google.gwt.uibinder'
xmlns:g='urn:import:com.google.gwt.user.client.ui'
xmlns:d='urn:import:com.google.gerrit.client.diff'>
<ui:style>
.reviewedPanel, .table {
width: 100%;
}
.fileName {
font-size: larger;
font-weight: bold;
}
.reviewed {
width: 20%;
text-align: right;
}
</ui:style>
<g:HTMLPanel styleName='{style.reviewedPanel}'>
<table class='{style.table}'>
<tr>
<td ui:field='fileName' class='{style.fileName}' />
<td class='{style.reviewed}'>
<g:CheckBox ui:field='checkBox'><ui:msg>Reviewed &amp; </ui:msg></g:CheckBox>
<g:Anchor ui:field='nextLink' href='javascript:;'/>
</td>
</tr>
</table>
</g:HTMLPanel>
</ui:UiBinder>

20
gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/SideBySide2.java
View File

@@ -56,7 +56,7 @@ import com.google.gwt.user.client.DOM;
import com.google.gwt.user.client.Timer;
import com.google.gwt.user.client.Window;
import com.google.gwt.user.client.rpc.AsyncCallback;
import com.google.gwt.user.client.ui.HTMLPanel;
import com.google.gwt.user.client.ui.FlowPanel;
import com.google.gwtexpui.globalkey.client.GlobalKey;
import com.google.gwtexpui.globalkey.client.KeyCommand;
import com.google.gwtexpui.globalkey.client.KeyCommandSet;
@@ -85,17 +85,14 @@ import java.util.List;
import java.util.Map;
public class SideBySide2 extends Screen {
interface Binder extends UiBinder<HTMLPanel, SideBySide2> {}
interface Binder extends UiBinder<FlowPanel, SideBySide2> {}
private static Binder uiBinder = GWT.create(Binder.class);
private static final JsArrayString EMPTY =
JavaScriptObject.createArray().cast();
@UiField(provided = true)
ReviewedPanel reviewed;
@UiField(provided = true)
NavLinks2 navLinks;
Header header;
@UiField(provided = true)
DiffTable diffTable;
@@ -139,9 +136,8 @@ public class SideBySide2 extends Screen {
this.handlers = new ArrayList<HandlerRegistration>(6);
// TODO: Re-implement necessary GlobalKey bindings.
addDomHandler(GlobalKey.STOP_PROPAGATION, KeyPressEvent.getType());
reviewed = new ReviewedPanel(revision, path);
keysNavigation = new KeyCommandSet(Gerrit.C.sectionNavigation());
add(navLinks = new NavLinks2(keysNavigation, revision, path));
add(header = new Header(keysNavigation, revision, path));
add(diffTable = new DiffTable(this, path));
add(uiBinder.createAndBindUi(this));
}
@@ -972,7 +968,7 @@ public class SideBySide2 extends Screen {
private Runnable toggleReviewed() {
return new Runnable() {
public void run() {
reviewed.setReviewed(!reviewed.isReviewed());
header.setReviewed(!header.isReviewed());
}
};
}
@@ -1057,8 +1053,10 @@ public class SideBySide2 extends Screen {
if (cmA == null) {
return;
}
int h = Gerrit.getHeaderFooterHeight() + reviewed.getOffsetHeight() +
navLinks.getOffsetHeight() + diffTable.getHeaderHeight() + 10; // Estimate
int h = Gerrit.getHeaderFooterHeight()
+ header.getOffsetHeight()
+ diffTable.getHeaderHeight()
+ 10; // Estimate
cmA.setHeight(Window.getClientHeight() - h);
cmA.refresh();
cmB.setHeight(Window.getClientHeight() - h);

7
gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/SideBySide2.ui.xml
View File

@@ -17,9 +17,8 @@ limitations under the License.
<ui:UiBinder xmlns:ui='urn:ui:com.google.gwt.uibinder'
xmlns:g='urn:import:com.google.gwt.user.client.ui'
xmlns:d='urn:import:com.google.gerrit.client.diff'>
<g:HTMLPanel>
<d:ReviewedPanel ui:field='reviewed'/>
<d:NavLinks2 ui:field='navLinks'/>
<g:FlowPanel>
<d:Header ui:field='header'/>
<d:DiffTable ui:field='diffTable'/>
</g:HTMLPanel>
</g:FlowPanel>
</ui:UiBinder>

2
gerrit-gwtui/src/main/java/com/google/gerrit/client/ui/FancyFlexTableImpl.java
View File

@@ -21,7 +21,7 @@ import com.google.gwtexpui.safehtml.client.SafeHtml;
public class FancyFlexTableImpl {
public void resetHtml(final MyFlexTable myTable, final SafeHtml body) {
SafeHtml.set(getBodyElement(myTable), body);
SafeHtml.setInnerHTML(getBodyElement(myTable), body);
}
protected static native Element getBodyElement(HTMLTable myTable)