Add diff pref whether the diff table header should be auto hidden

Add a new diff preference that allows the user to decide whether the
diff table header with the patch set selection should be automatically
hidden when scrolling down more than half of a page. At the moment
this behaviour cannot be disabled, but some users find it annoying
that the header disappears and that they need to scroll up to be able
to change the patch set selection.

By default the diff table header is automatically hidden as this is
the current behaviour.

Change-Id: I1d65c5058222c924ccaf3276f7d2d83fac7f4d32
Signed-off-by: Edwin Kempin <edwin.kempin@sap.com>
This commit is contained in:
Edwin Kempin
2014-09-11 23:36:43 +02:00
parent 2970f85486
commit 1b6c6d843c
13 changed files with 132 additions and 40 deletions

Binary file not shown.

Before

Width:  |  Height:  |  Size: 140 KiB

After

Width:  |  Height:  |  Size: 19 KiB

View File

@@ -1417,7 +1417,7 @@ The `DiffPreferencesInfo` entity contains information about the diff
preferences of a user. preferences of a user.
[options="header",width="50%",cols="1,^1,5"] [options="header",width="50%",cols="1,^1,5"]
|===================================== |===========================================
|Field Name ||Description |Field Name ||Description
|`context` || |`context` ||
The number of lines of context when viewing a patch. The number of lines of context when viewing a patch.
@@ -1444,7 +1444,7 @@ Whether Windows EOL/Cr-Lf should be displayed as '\r' in a dotted-line
box. box.
|`show_tabs` |not set if `false`| |`show_tabs` |not set if `false`|
Whether tabs should be shown. Whether tabs should be shown.
|`show_whitespace_errors`|not set if `false`| |`show_whitespace_errors` |not set if `false`|
Whether whitespace errors should be shown. Whether whitespace errors should be shown.
|`skip_deleted` |not set if `false`| |`skip_deleted` |not set if `false`|
Whether deleted files should be skipped on file switch. Whether deleted files should be skipped on file switch.
@@ -1454,11 +1454,14 @@ Whether uncommented files should be skipped on file switch.
Whether syntax highlighting should be enabled. Whether syntax highlighting should be enabled.
|`hide_top_menu` |not set if `false`| |`hide_top_menu` |not set if `false`|
If true the top menu header and site header is hidden. If true the top menu header and site header is hidden.
|`auto_hide_diff_table_header` |not set if `false`|
If true the diff table header is automatically hidden when
scrolling down more than half of a page.
|`hide_line_numbers` |not set if `false`| |`hide_line_numbers` |not set if `false`|
If true the line numbers are hidden. If true the line numbers are hidden.
|`tab_size` || |`tab_size` ||
Number of spaces that should be used to display one tab. Number of spaces that should be used to display one tab.
|===================================== |===========================================
[[diff-preferences-input]] [[diff-preferences-input]]
=== DiffPreferencesInput === DiffPreferencesInput
@@ -1467,7 +1470,7 @@ diff preferences of a user. Fields which are not set will not be
updated. updated.
[options="header",width="50%",cols="1,^1,5"] [options="header",width="50%",cols="1,^1,5"]
|===================================== |===========================================
|Field Name ||Description |Field Name ||Description
|`context` |optional| |`context` |optional|
The number of lines of context when viewing a patch. The number of lines of context when viewing a patch.
@@ -1494,7 +1497,7 @@ Whether Windows EOL/Cr-Lf should be displayed as '\r' in a dotted-line
box. box.
|`show_tabs` |optional| |`show_tabs` |optional|
Whether tabs should be shown. Whether tabs should be shown.
|`show_whitespace_errors`|optional| |`show_whitespace_errors` |optional|
Whether whitespace errors should be shown. Whether whitespace errors should be shown.
|`skip_deleted` |optional| |`skip_deleted` |optional|
Whether deleted files should be skipped on file switch. Whether deleted files should be skipped on file switch.
@@ -1504,11 +1507,14 @@ Whether uncommented files should be skipped on file switch.
Whether syntax highlighting should be enabled. Whether syntax highlighting should be enabled.
|`hide_top_menu` |optional| |`hide_top_menu` |optional|
True if the top menu header and site header should be hidden. True if the top menu header and site header should be hidden.
|`auto_hide_diff_table_header` |optional|
True if the diff table header is automatically hidden when
scrolling down more than half of a page.
|`hide_line_numbers` |optional| |`hide_line_numbers` |optional|
True if the line numbers should be hidden. True if the line numbers should be hidden.
|`tab_size` |optional| |`tab_size` |optional|
Number of spaces that should be used to display one tab. Number of spaces that should be used to display one tab.
|===================================== |===========================================
[[email-info]] [[email-info]]
=== EmailInfo === EmailInfo

View File

@@ -1046,6 +1046,11 @@ patch diff is opened, this preference is reset to `Show`.
+ +
Controls whether the top menu is shown. Controls whether the top menu is shown.
- `Auto Hide Diff Table Header`:
+
Controls whether the diff table header should be automatically hidden
when scrolling down more than half of a page.
[[mark-reviewed]] [[mark-reviewed]]
- `Mark Reviewed`: - `Mark Reviewed`:
+ +

View File

@@ -35,6 +35,7 @@ public class DiffPreferences extends JavaScriptObject {
p.showWhitespaceErrors(in.isShowWhitespaceErrors()); p.showWhitespaceErrors(in.isShowWhitespaceErrors());
p.syntaxHighlighting(in.isSyntaxHighlighting()); p.syntaxHighlighting(in.isSyntaxHighlighting());
p.hideTopMenu(in.isHideTopMenu()); p.hideTopMenu(in.isHideTopMenu());
p.autoHideDiffTableHeader(in.isAutoHideDiffTableHeader());
p.hideLineNumbers(in.isHideLineNumbers()); p.hideLineNumbers(in.isHideLineNumbers());
p.expandAllComments(in.isExpandAllComments()); p.expandAllComments(in.isExpandAllComments());
p.manualReview(in.isManualReview()); p.manualReview(in.isManualReview());
@@ -55,6 +56,7 @@ public class DiffPreferences extends JavaScriptObject {
p.setShowWhitespaceErrors(showWhitespaceErrors()); p.setShowWhitespaceErrors(showWhitespaceErrors());
p.setSyntaxHighlighting(syntaxHighlighting()); p.setSyntaxHighlighting(syntaxHighlighting());
p.setHideTopMenu(hideTopMenu()); p.setHideTopMenu(hideTopMenu());
p.setAutoHideDiffTableHeader(autoHideDiffTableHeader());
p.setHideLineNumbers(hideLineNumbers()); p.setHideLineNumbers(hideLineNumbers());
p.setExpandAllComments(expandAllComments()); p.setExpandAllComments(expandAllComments());
p.setManualReview(manualReview()); p.setManualReview(manualReview());
@@ -82,6 +84,7 @@ public class DiffPreferences extends JavaScriptObject {
public final native void showWhitespaceErrors(boolean s) /*-{ this.show_whitespace_errors = s }-*/; public final native void showWhitespaceErrors(boolean s) /*-{ this.show_whitespace_errors = s }-*/;
public final native void syntaxHighlighting(boolean s) /*-{ this.syntax_highlighting = s }-*/; public final native void syntaxHighlighting(boolean s) /*-{ this.syntax_highlighting = s }-*/;
public final native void hideTopMenu(boolean s) /*-{ this.hide_top_menu = s }-*/; public final native void hideTopMenu(boolean s) /*-{ this.hide_top_menu = s }-*/;
public final native void autoHideDiffTableHeader(boolean s) /*-{ this.auto_hide_diff_table_header = s }-*/;
public final native void hideLineNumbers(boolean s) /*-{ this.hide_line_numbers = s }-*/; public final native void hideLineNumbers(boolean s) /*-{ this.hide_line_numbers = s }-*/;
public final native void expandAllComments(boolean e) /*-{ this.expand_all_comments = e }-*/; public final native void expandAllComments(boolean e) /*-{ this.expand_all_comments = e }-*/;
public final native void manualReview(boolean r) /*-{ this.manual_review = r }-*/; public final native void manualReview(boolean r) /*-{ this.manual_review = r }-*/;
@@ -110,6 +113,7 @@ public class DiffPreferences extends JavaScriptObject {
public final native boolean showWhitespaceErrors() /*-{ return this.show_whitespace_errors || false }-*/; public final native boolean showWhitespaceErrors() /*-{ return this.show_whitespace_errors || false }-*/;
public final native boolean syntaxHighlighting() /*-{ return this.syntax_highlighting || false }-*/; public final native boolean syntaxHighlighting() /*-{ return this.syntax_highlighting || false }-*/;
public final native boolean hideTopMenu() /*-{ return this.hide_top_menu || false }-*/; public final native boolean hideTopMenu() /*-{ return this.hide_top_menu || false }-*/;
public final native boolean autoHideDiffTableHeader() /*-{ return this.auto_hide_diff_table_header || false }-*/;
public final native boolean hideLineNumbers() /*-{ return this.hide_line_numbers || false }-*/; public final native boolean hideLineNumbers() /*-{ return this.hide_line_numbers || false }-*/;
public final native boolean expandAllComments() /*-{ return this.expand_all_comments || false }-*/; public final native boolean expandAllComments() /*-{ return this.expand_all_comments || false }-*/;
public final native boolean manualReview() /*-{ return this.manual_review || false }-*/; public final native boolean manualReview() /*-{ return this.manual_review || false }-*/;

View File

@@ -80,6 +80,7 @@ class DiffTable extends Composite {
private SideBySide2 parent; private SideBySide2 parent;
private boolean header; private boolean header;
private boolean headerVisible; private boolean headerVisible;
private boolean autoHideHeader;
private boolean visibleA; private boolean visibleA;
private ChangeType changeType; private ChangeType changeType;
@@ -134,7 +135,11 @@ class DiffTable extends Composite {
} }
void setHeaderVisible(boolean show) { void setHeaderVisible(boolean show) {
headerVisible = show; headerVisible = !autoHideHeader || show;
showHeader(headerVisible);
}
private void showHeader(boolean show) {
UIObject.setVisible(patchSetNavRow, show); UIObject.setVisible(patchSetNavRow, show);
UIObject.setVisible(diffHeaderRow, show && header); UIObject.setVisible(diffHeaderRow, show && header);
if (show) { if (show) {
@@ -145,6 +150,13 @@ class DiffTable extends Composite {
parent.resizeCodeMirror(); parent.resizeCodeMirror();
} }
void setAutoHideDiffHeader(boolean hide) {
autoHideHeader = hide;
if (!hide) {
showHeader(true);
}
}
int getHeaderHeight() { int getHeaderHeight() {
int h = patchSetSelectBoxA.getOffsetHeight(); int h = patchSetSelectBoxA.getOffsetHeight();
if (header) { if (header) {
@@ -159,6 +171,7 @@ class DiffTable extends Composite {
void set(DiffPreferences prefs, JsArray<RevisionInfo> list, DiffInfo info) { void set(DiffPreferences prefs, JsArray<RevisionInfo> list, DiffInfo info) {
this.changeType = info.change_type(); this.changeType = info.change_type();
this.autoHideHeader = prefs.autoHideDiffTableHeader();
patchSetSelectBoxA.setUpPatchSetNav(list, info.meta_a(), patchSetSelectBoxA.setUpPatchSetNav(list, info.meta_a(),
Natives.asList(info.web_links_a())); Natives.asList(info.web_links_a()));
patchSetSelectBoxB.setUpPatchSetNav(list, info.meta_b(), patchSetSelectBoxB.setUpPatchSetNav(list, info.meta_b(),

View File

@@ -90,6 +90,7 @@ class PreferencesBox extends Composite {
@UiField ToggleButton leftSide; @UiField ToggleButton leftSide;
@UiField ToggleButton emptyPane; @UiField ToggleButton emptyPane;
@UiField ToggleButton topMenu; @UiField ToggleButton topMenu;
@UiField ToggleButton autoHideDiffTableHeader;
@UiField ToggleButton manualReview; @UiField ToggleButton manualReview;
@UiField ToggleButton expandAllComments; @UiField ToggleButton expandAllComments;
@UiField ToggleButton renderEntireFile; @UiField ToggleButton renderEntireFile;
@@ -157,6 +158,7 @@ class PreferencesBox extends Composite {
leftSide.setEnabled(!(prefs.hideEmptyPane() leftSide.setEnabled(!(prefs.hideEmptyPane()
&& view.diffTable.getChangeType() == ChangeType.ADDED)); && view.diffTable.getChangeType() == ChangeType.ADDED));
topMenu.setValue(!prefs.hideTopMenu()); topMenu.setValue(!prefs.hideTopMenu());
autoHideDiffTableHeader.setValue(!prefs.autoHideDiffTableHeader());
manualReview.setValue(prefs.manualReview()); manualReview.setValue(prefs.manualReview());
expandAllComments.setValue(prefs.expandAllComments()); expandAllComments.setValue(prefs.expandAllComments());
renderEntireFile.setValue(prefs.renderEntireFile()); renderEntireFile.setValue(prefs.renderEntireFile());
@@ -322,6 +324,13 @@ class PreferencesBox extends Composite {
view.resizeCodeMirror(); view.resizeCodeMirror();
} }
@UiHandler("autoHideDiffTableHeader")
void onAutoHideDiffTableHeader(ValueChangeEvent<Boolean> e) {
prefs.autoHideDiffTableHeader(!e.getValue());
view.setAutoHideDiffHeader(!e.getValue());
view.resizeCodeMirror();
}
@UiHandler("manualReview") @UiHandler("manualReview")
void onManualReview(ValueChangeEvent<Boolean> e) { void onManualReview(ValueChangeEvent<Boolean> e) {
prefs.manualReview(e.getValue()); prefs.manualReview(e.getValue());

View File

@@ -249,6 +249,13 @@ limitations under the License.
<g:downFace><ui:msg>Show</ui:msg></g:downFace> <g:downFace><ui:msg>Show</ui:msg></g:downFace>
</g:ToggleButton></td> </g:ToggleButton></td>
</tr> </tr>
<tr>
<th><ui:msg>Auto Hide Diff Table Header</ui:msg></th>
<td><g:ToggleButton ui:field='autoHideDiffTableHeader'>
<g:upFace><ui:msg>Yes</ui:msg></g:upFace>
<g:downFace><ui:msg>No</ui:msg></g:downFace>
</g:ToggleButton></td>
</tr>
<tr> <tr>
<th><ui:msg>Mark Reviewed</ui:msg></th> <th><ui:msg>Mark Reviewed</ui:msg></th>
<td><g:ToggleButton ui:field='manualReview'> <td><g:ToggleButton ui:field='manualReview'>

View File

@@ -715,6 +715,10 @@ public class SideBySide2 extends Screen {
}); });
} }
void setAutoHideDiffHeader(boolean hide) {
diffTable.setAutoHideDiffHeader(hide);
}
private void render(DiffInfo diff) { private void render(DiffInfo diff) {
header.setNoDiff(diff); header.setNoDiff(diff);
chunkManager.render(diff); chunkManager.render(diff);

View File

@@ -92,6 +92,7 @@ public class AccountDiffPreference {
p.setContext(DEFAULT_CONTEXT); p.setContext(DEFAULT_CONTEXT);
p.setManualReview(false); p.setManualReview(false);
p.setHideEmptyPane(false); p.setHideEmptyPane(false);
p.setAutoHideDiffTableHeader(true);
return p; return p;
} }
@@ -156,6 +157,9 @@ public class AccountDiffPreference {
@Column(id = 20) @Column(id = 20)
protected boolean hideEmptyPane; protected boolean hideEmptyPane;
@Column(id = 21)
protected boolean autoHideDiffTableHeader;
protected AccountDiffPreference() { protected AccountDiffPreference() {
} }
@@ -183,6 +187,7 @@ public class AccountDiffPreference {
this.hideLineNumbers = p.hideLineNumbers; this.hideLineNumbers = p.hideLineNumbers;
this.renderEntireFile = p.renderEntireFile; this.renderEntireFile = p.renderEntireFile;
this.hideEmptyPane = p.hideEmptyPane; this.hideEmptyPane = p.hideEmptyPane;
this.autoHideDiffTableHeader = p.autoHideDiffTableHeader;
} }
public Account.Id getAccountId() { public Account.Id getAccountId() {
@@ -343,4 +348,12 @@ public class AccountDiffPreference {
public void setHideEmptyPane(boolean hideEmptyPane) { public void setHideEmptyPane(boolean hideEmptyPane) {
this.hideEmptyPane = hideEmptyPane; this.hideEmptyPane = hideEmptyPane;
} }
public void setAutoHideDiffTableHeader(boolean hide) {
autoHideDiffTableHeader = hide;
}
public boolean isAutoHideDiffTableHeader() {
return autoHideDiffTableHeader;
}
} }

View File

@@ -70,6 +70,7 @@ public class GetDiffPreferences implements RestReadView<AccountResource> {
info.skipDeleted = p.isSkipDeleted() ? true : null; info.skipDeleted = p.isSkipDeleted() ? true : null;
info.skipUncommented = p.isSkipUncommented() ? true : null; info.skipUncommented = p.isSkipUncommented() ? true : null;
info.hideTopMenu = p.isHideTopMenu() ? true : null; info.hideTopMenu = p.isHideTopMenu() ? true : null;
info.autoHideDiffTableHeader = p.isAutoHideDiffTableHeader() ? true : null;
info.hideLineNumbers = p.isHideLineNumbers() ? true : null; info.hideLineNumbers = p.isHideLineNumbers() ? true : null;
info.syntaxHighlighting = p.isSyntaxHighlighting() ? true : null; info.syntaxHighlighting = p.isSyntaxHighlighting() ? true : null;
info.tabSize = p.getTabSize(); info.tabSize = p.getTabSize();
@@ -93,6 +94,7 @@ public class GetDiffPreferences implements RestReadView<AccountResource> {
public Boolean skipUncommented; public Boolean skipUncommented;
public Boolean syntaxHighlighting; public Boolean syntaxHighlighting;
public Boolean hideTopMenu; public Boolean hideTopMenu;
public Boolean autoHideDiffTableHeader;
public Boolean hideLineNumbers; public Boolean hideLineNumbers;
public Boolean renderEntireFile; public Boolean renderEntireFile;
public Boolean hideEmptyPane; public Boolean hideEmptyPane;

View File

@@ -48,6 +48,7 @@ public class SetDiffPreferences implements RestModifyView<AccountResource, Input
Boolean skipUncommented; Boolean skipUncommented;
Boolean syntaxHighlighting; Boolean syntaxHighlighting;
Boolean hideTopMenu; Boolean hideTopMenu;
Boolean autoHideDiffTableHeader;
Boolean hideLineNumbers; Boolean hideLineNumbers;
Boolean renderEntireFile; Boolean renderEntireFile;
Integer tabSize; Integer tabSize;
@@ -127,6 +128,9 @@ public class SetDiffPreferences implements RestModifyView<AccountResource, Input
if (input.hideTopMenu != null) { if (input.hideTopMenu != null) {
p.setHideTopMenu(input.hideTopMenu); p.setHideTopMenu(input.hideTopMenu);
} }
if (input.autoHideDiffTableHeader != null) {
p.setAutoHideDiffTableHeader(input.autoHideDiffTableHeader);
}
if (input.hideLineNumbers != null) { if (input.hideLineNumbers != null) {
p.setHideLineNumbers(input.hideLineNumbers); p.setHideLineNumbers(input.hideLineNumbers);
} }

View File

@@ -32,7 +32,7 @@ import java.util.List;
/** A version of the database schema. */ /** A version of the database schema. */
public abstract class SchemaVersion { public abstract class SchemaVersion {
/** The current schema version. */ /** The current schema version. */
public static final Class<Schema_98> C = Schema_98.class; public static final Class<Schema_99> C = Schema_99.class;
public static class Module extends AbstractModule { public static class Module extends AbstractModule {
@Override @Override

View File

@@ -0,0 +1,25 @@
// Copyright (C) 2014 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.server.schema;
import com.google.inject.Inject;
import com.google.inject.Provider;
public class Schema_99 extends SchemaVersion {
@Inject
Schema_99(Provider<Schema_98> prior) {
super(prior);
}
}