From 63543e9188f1c9025f147b0a5a35a5991f1a6d6f Mon Sep 17 00:00:00 2001 From: Shawn Pearce Date: Thu, 21 Nov 2013 01:33:15 -0800 Subject: [PATCH 1/2] SideBySide2: Simplify logic for showing/hiding header The file header and Gerrit menus should show/hide together. Consolidate the logic for this into one place and simplify the conditional value to be consistent between them: true means show headers, false means hide headers. Change-Id: Iae59a40369ef698279eee2c1faa2b147fbb622c8 --- .../google/gerrit/client/diff/DiffTable.java | 17 ++++++++--------- .../gerrit/client/diff/FileCommentPanel.java | 4 ++-- .../gerrit/client/diff/ScrollSynchronizer.java | 6 ++---- 3 files changed, 12 insertions(+), 15 deletions(-) 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 23af7208e4..f2237c59e4 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,6 +14,7 @@ package com.google.gerrit.client.diff; +import com.google.gerrit.client.Gerrit; import com.google.gerrit.client.changes.ChangeInfo.RevisionInfo; import com.google.gerrit.reviewdb.client.PatchSet; import com.google.gwt.core.client.GWT; @@ -100,14 +101,12 @@ class DiffTable extends Composite { this.host = host; } - void updateFileCommentVisibility(boolean forceHide) { - UIObject.setVisible(patchSetNavRow, !forceHide); - if (forceHide || (fileCommentPanelA.getBoxCount() == 0 && - fileCommentPanelB.getBoxCount() == 0)) { - UIObject.setVisible(fileCommentRow, false); - } else { - UIObject.setVisible(fileCommentRow, true); - } + void setHeaderVisible(boolean show) { + Gerrit.setHeaderVisible(show); + UIObject.setVisible(patchSetNavRow, show); + UIObject.setVisible(fileCommentRow, show + && (fileCommentPanelA.getBoxCount() > 0 + || fileCommentPanelB.getBoxCount() > 0)); host.resizeCodeMirror(); } @@ -117,7 +116,7 @@ class DiffTable extends Composite { void createOrEditFileComment(DisplaySide side) { getPanelFromSide(side).createOrEditFileComment(); - updateFileCommentVisibility(false); + setHeaderVisible(true); } void addFileCommentBox(CommentBox box) { diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/FileCommentPanel.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/FileCommentPanel.java index f252e65b81..37e017896d 100644 --- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/FileCommentPanel.java +++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/FileCommentPanel.java @@ -74,11 +74,11 @@ class FileCommentPanel extends Composite { void addFileComment(CommentBox box) { boxes.add(box); body.add(box); - table.updateFileCommentVisibility(false); + table.setHeaderVisible(true); } void onRemoveDraftBox(DraftBox box) { boxes.remove(box); - table.updateFileCommentVisibility(false); + table.setHeaderVisible(true); } } diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/ScrollSynchronizer.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/ScrollSynchronizer.java index f5ae14980b..425878c9bd 100644 --- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/ScrollSynchronizer.java +++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/ScrollSynchronizer.java @@ -39,12 +39,10 @@ class ScrollSynchronizer { private void updateScreenHeader(ScrollInfo si) { if (si.getTop() == 0 && !Gerrit.isHeaderVisible()) { - Gerrit.setHeaderVisible(true); - diffTable.updateFileCommentVisibility(false); + diffTable.setHeaderVisible(true); } else if (si.getTop() > 0.5 * si.getClientHeight() && Gerrit.isHeaderVisible()) { - Gerrit.setHeaderVisible(false); - diffTable.updateFileCommentVisibility(true); + diffTable.setHeaderVisible(false); } } From 545b0008862afae69be890b73d1c6a6b525241b0 Mon Sep 17 00:00:00 2001 From: Shawn Pearce Date: Thu, 21 Nov 2013 01:41:38 -0800 Subject: [PATCH 2/2] SideBySide2: Draw a line under the file header when "fullscreen" When the Gerrit menus and patch list header have been hidden to show more of the file contents, draw a line under the tiny header (reviewed box, path, navigation cluster) to separate it from the displayed file contents. This makes it less likely the header is confused with a line of text, without getting in the way. Change-Id: Icb5f77c9e67decc3f8e3e2f5e59f7278926b7a08 --- .../main/java/com/google/gerrit/client/diff/DiffTable.java | 6 ++++++ .../java/com/google/gerrit/client/diff/DiffTable.ui.xml | 4 ++++ 2 files changed, 10 insertions(+) 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 f2237c59e4..1e333210f2 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 @@ -37,6 +37,7 @@ class DiffTable extends Composite { private static final Binder uiBinder = GWT.create(Binder.class); interface DiffTableStyle extends CssResource { + String fullscreen(); String intralineBg(); String diff(); String activeLine(); @@ -107,6 +108,11 @@ class DiffTable extends Composite { UIObject.setVisible(fileCommentRow, show && (fileCommentPanelA.getBoxCount() > 0 || fileCommentPanelB.getBoxCount() > 0)); + if (show) { + host.header.removeStyleName(style.fullscreen()); + } else { + host.header.addStyleName(style.fullscreen()); + } host.resizeCodeMirror(); } 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 65a85881d2..0a03bc4ba6 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 @@ -23,6 +23,10 @@ limitations under the License. @external .cm-keymap-fat-cursor, CodeMirror-cursor; @external .cm-searching, .cm-trailingspace, .cm-tab; + .fullscreen { + border-bottom: 1px solid #ddd; + } + .difftable { max-width: 1484px; } .difftable .CodeMirror-lines { padding: 0; } .difftable .CodeMirror pre {