From 49cd82b0e14dad02baf3597ad0e338376776a1cf Mon Sep 17 00:00:00 2001 From: Martin Fick Date: Wed, 30 Nov 2011 12:32:30 -0700 Subject: [PATCH 1/2] Add decoration regions to the Screen header title This is primarily a feature to support moving the reviewed checkbox into the title line on the diff screen. This change does not actually move the checkbox. The screen header is mainly a text title. Previously a widget could only be inserted to the left of the title, this was done for the star on the change screen. Enhance the title decoration functionality so it can be decorated in the West, the East, and the FarEast by any Widget. The West and East decorations will surround the text on the left and right respectively, and the FarEast will be right justified to the right edge of the screen. The East decoration will expand to take up any extra space. Change-Id: Iaa6b7775ae16bd94e24c5d54ad9e8c029a7893bb --- .../gerrit/client/changes/ChangeScreen.java | 2 +- .../com/google/gerrit/client/ui/Screen.java | 42 ++++++++++++++++--- 2 files changed, 38 insertions(+), 6 deletions(-) diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/ChangeScreen.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/ChangeScreen.java index ed0f2f2ef4..1b382a864d 100644 --- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/ChangeScreen.java +++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/ChangeScreen.java @@ -204,7 +204,7 @@ public class ChangeScreen extends Screen { toggleStar(); } }); - insertTitleWidget(starChange); + setTitleWest(starChange); } descriptionBlock = new ChangeDescriptionBlock(); diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/ui/Screen.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/ui/Screen.java index 68026ae485..5cb47271b5 100644 --- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/ui/Screen.java +++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/ui/Screen.java @@ -18,12 +18,24 @@ import com.google.gerrit.client.Gerrit; import com.google.gerrit.common.PageLinks; import com.google.gwt.user.client.History; import com.google.gwt.user.client.ui.FlowPanel; +import com.google.gwt.user.client.ui.Grid; +import com.google.gwt.user.client.ui.HasHorizontalAlignment; import com.google.gwt.user.client.ui.InlineLabel; import com.google.gwt.user.client.ui.Widget; import com.google.gwtexpui.user.client.View; +/** + * A Screen layout with a header and a body. + * + * The header is mainly a text title, but it can be decorated + * in the West, the East, and the FarEast by any Widget. The + * West and East decorations will surround the text on the + * left and right respectively, and the FarEast will be right + * justified to the right edge of the screen. The East + * decoration will expand to take up any extra space. + */ public abstract class Screen extends View { - private FlowPanel header; + private Grid header; private InlineLabel headerText; private FlowPanel body; private String token; @@ -46,13 +58,25 @@ public abstract class Screen extends View { public void registerKeys() { } + private static enum Cols { + West, Title, East, FarEast; + } + protected void onInitUI() { final FlowPanel me = (FlowPanel) getWidget(); - me.add(header = new FlowPanel()); + me.add(header = new Grid(1, Cols.values().length)); me.add(body = new FlowPanel()); + FlowPanel title = new FlowPanel(); + title.add(headerText = new InlineLabel()); + title.setStyleName(Gerrit.RESOURCES.css().screenHeader()); + header.setWidget(0, Cols.Title.ordinal(), title); + header.setStyleName(Gerrit.RESOURCES.css().screenHeader()); - header.add(headerText = new InlineLabel()); + header.getCellFormatter().setHorizontalAlignment(0, Cols.FarEast.ordinal(), + HasHorizontalAlignment.ALIGN_RIGHT); + // force FarEast all the way to the right + header.getCellFormatter().setWidth(0, Cols.FarEast.ordinal(), "100%"); } protected void setWindowTitle(final String text) { @@ -73,8 +97,16 @@ public abstract class Screen extends View { } } - protected void insertTitleWidget(final Widget w) { - header.insert(w, 0); + protected void setTitleEast(final Widget w) { + header.setWidget(0, Cols.East.ordinal(), w); + } + + protected void setTitleFarEast(final Widget w) { + header.setWidget(0, Cols.FarEast.ordinal(), w); + } + + protected void setTitleWest(final Widget w) { + header.setWidget(0, Cols.West.ordinal(), w); } protected void add(final Widget w) { From be7d961f7a04712769d026f561c175211c1d8867 Mon Sep 17 00:00:00 2001 From: Martin Fick Date: Wed, 30 Nov 2011 12:26:47 -0700 Subject: [PATCH 2/2] Move checkbox to mark file as reviewed into title bar The reviewed checkbox did not fit well on the preferences panel. It was not a preference and it operated immediately without having to hit Update or Save. To encourage its usage more, move it to the title bar on the far right of the screen above the link to the next file. This makes it much more accessible since it will now always be on the screen and it is positioned nicely near the action a user will most likely take next after checking the reviewed box. This last piece should make reviewing on smaller screens such as tablets much more convenient. Change-Id: Ia6d47cc8b976805297231916dc28f7d8002fbaf4 --- .../gerrit/client/patches/PatchScreen.java | 24 ++++++++++++------- .../patches/PatchScriptSettingsPanel.java | 8 ------- .../patches/PatchScriptSettingsPanel.ui.xml | 9 ------- 3 files changed, 16 insertions(+), 25 deletions(-) diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/patches/PatchScreen.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/patches/PatchScreen.java index 910bf618a9..1671d6e1fb 100644 --- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/patches/PatchScreen.java +++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/patches/PatchScreen.java @@ -39,6 +39,7 @@ import com.google.gwt.event.logical.shared.ValueChangeEvent; import com.google.gwt.event.logical.shared.ValueChangeHandler; import com.google.gwt.event.shared.HandlerRegistration; import com.google.gwt.user.client.rpc.AsyncCallback; +import com.google.gwt.user.client.ui.CheckBox; import com.google.gwt.user.client.ui.FlowPanel; import com.google.gwt.user.client.ui.Label; import com.google.gwtexpui.globalkey.client.GlobalKey; @@ -105,6 +106,7 @@ public abstract class PatchScreen extends Screen implements protected PatchScriptSettingsPanel settingsPanel; protected TopView topView; + private CheckBox reviewed; private HistoryTable historyTable; private FlowPanel topPanel; private FlowPanel contentPanel; @@ -151,6 +153,15 @@ public abstract class PatchScreen extends Screen implements idSideB = diffSideB != null ? diffSideB : id.getParentKey(); this.patchIndex = patchIndex; + reviewed = new CheckBox(Util.C.reviewed()); + reviewed.addValueChangeHandler( + new ValueChangeHandler() { + @Override + public void onValueChange(ValueChangeEvent event) { + setReviewedByCurrentUser(event.getValue()); + } + }); + prefs = fileList != null ? fileList.getPreferences() : new ListenableAccountDiffPreference(); if (Gerrit.isSignedIn()) { @@ -165,13 +176,6 @@ public abstract class PatchScreen extends Screen implements }); settingsPanel = new PatchScriptSettingsPanel(prefs); - settingsPanel.getReviewedCheckBox().addValueChangeHandler( - new ValueChangeHandler() { - @Override - public void onValueChange(ValueChangeEvent event) { - setReviewedByCurrentUser(event.getValue()); - } - }); } @Override @@ -234,6 +238,10 @@ public abstract class PatchScreen extends Screen implements protected void onInitUI() { super.onInitUI(); + if (Gerrit.isSignedIn()) { + setTitleFarEast(reviewed); + } + keysNavigation = new KeyCommandSet(Gerrit.C.sectionNavigation()); keysNavigation.add(new UpToChangeCommand(patchKey.getParentKey(), 0, 'u')); keysNavigation.add(new FileListCmd(0, 'f', PatchUtil.C.fileList())); @@ -444,7 +452,7 @@ public abstract class PatchScreen extends Screen implements // Mark this file reviewed as soon we display the diff screen if (Gerrit.isSignedIn() && isFirst) { - settingsPanel.getReviewedCheckBox().setValue(true); + reviewed.setValue(true); setReviewedByCurrentUser(true /* reviewed */); } diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/patches/PatchScriptSettingsPanel.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/patches/PatchScriptSettingsPanel.java index 70aade1b7d..df0fff5d50 100644 --- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/patches/PatchScriptSettingsPanel.java +++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/patches/PatchScriptSettingsPanel.java @@ -78,9 +78,6 @@ public class PatchScriptSettingsPanel extends Composite implements @UiField CheckBox showTabs; - @UiField - CheckBox reviewed; - @UiField CheckBox skipDeleted; @@ -119,7 +116,6 @@ public class PatchScriptSettingsPanel extends Composite implements initIgnoreWhitespace(ignoreWhitespace); initContext(context); if (!Gerrit.isSignedIn()) { - reviewed.setVisible(false); save.setVisible(false); } @@ -188,10 +184,6 @@ public class PatchScriptSettingsPanel extends Composite implements syntaxHighlighting.setTitle(title); } - public CheckBox getReviewedCheckBox() { - return reviewed; - } - public AccountDiffPreference getValue() { return listenablePrefs.get(); } diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/patches/PatchScriptSettingsPanel.ui.xml b/gerrit-gwtui/src/main/java/com/google/gerrit/client/patches/PatchScriptSettingsPanel.ui.xml index f320ce5304..e819ffa625 100644 --- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/patches/PatchScriptSettingsPanel.ui.xml +++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/patches/PatchScriptSettingsPanel.ui.xml @@ -164,15 +164,6 @@ limitations under the License. - - - - - -