diff options
author | Shawn Pearce <sop@google.com> | 2013-10-11 20:00:23 -0700 |
---|---|---|
committer | Shawn Pearce <sop@google.com> | 2013-10-11 21:37:55 -0700 |
commit | 5bf4addf76d8fe7bff67401116acd03783bbd33f (patch) | |
tree | 0bdc83717834fc50649d1b2e997393eca7b2f898 | |
parent | a88bd9b0d4644fc9dfc8764777d453d748f0dd3b (diff) |
SideBySide2: Synchronize horizontal scrolling
Disable CM3 line wrapping and rely on synchronized horizontal
scrolling instead. This fixes alignment problems caused by long lines
that exist only one of two sides. When line wrapping, the view did
not pad the other side to account for the extra line height used by
the wrapped line, skewing everything after it.
Since horizontal scrolling is now functional change the widths of the
views to be related to the width of the browser window. Keep a
maximum width to prevent the a large gap between the sides on very
wide monitors.
Change-Id: I3a3b10fefdacd7246e4856365cb91d934f0346e2
4 files changed, 27 insertions, 19 deletions
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 4c8579c7ed..18d497ce3d 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 @@ -22,13 +22,15 @@ limitations under the License. @external .CodeMirror-linenumber, .CodeMirror-vscrollbar .CodeMirror-scroll; @external .cm-keymap-fat-cursor, CodeMirror-cursor; @external .cm-searching, .cm-trailingspace, .cm-tab; - .difftable .CodeMirror-lines { - padding: 0; - } + + .difftable { max-width: 1484px; } + .difftable .CodeMirror-lines { padding: 0; } .difftable .CodeMirror pre { padding: 0; padding-bottom: 0.11em; overflow: hidden; + border-right: 0; + width: auto; } .difftable .CodeMirror pre span { padding-bottom: 0.11em; @@ -41,13 +43,15 @@ limitations under the License. width: 10px; } .table { + width: 100%; table-layout: fixed; border-spacing: 0; } - .a, .b { padding: 0; } - .a { width: 702px; min-width: 702px; } - .b { width: 718px; min-width: 718px; } + .a, .b { + padding: 0; + width: 50%; + } /* Hide left side scrollbar, right side controls both views. */ .a .CodeMirror-scroll { padding-right: 0; } diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/Header.ui.xml b/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/Header.ui.xml index 53402fc086..2814a6312d 100644 --- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/Header.ui.xml +++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/Header.ui.xml @@ -21,8 +21,7 @@ limitations under the License. <ui:style> .header { position: relative; - width: 1420px; - min-width: 1420px; + max-width: 1484px; } .reviewed input { margin: 0; @@ -34,7 +33,7 @@ limitations under the License. .navigation { position: absolute; top: 0; - right: 0; + right: 15px; font-family: Arial Unicode MS, sans-serif; } </ui:style> 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 bff8990c3c..f5ae14980b 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 @@ -37,8 +37,7 @@ class ScrollSynchronizer { cmB.on("scroll", new ScrollCallback(cmB, cmA, DisplaySide.B)); } - private void updateScreenHeader(CodeMirror cm) { - ScrollInfo si = cm.getScrollInfo(); + private void updateScreenHeader(ScrollInfo si) { if (si.getTop() == 0 && !Gerrit.isHeaderVisible()) { Gerrit.setHeaderVisible(true); diffTable.updateFileCommentVisibility(false); @@ -77,8 +76,9 @@ class ScrollSynchronizer { fixup.scheduleRepeating(20); } if (active == this) { - updateScreenHeader(src); - dst.scrollToY(src.getScrollInfo().getTop()); + ScrollInfo si = src.getScrollInfo(); + updateScreenHeader(si); + dst.scrollTo(si.getLeft(), si.getTop()); state = 0; } } 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 505b6443e0..80660fa99d 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 @@ -159,8 +159,8 @@ public class SideBySide2 extends Screen { // TODO: Re-implement necessary GlobalKey bindings. addDomHandler(GlobalKey.STOP_PROPAGATION, KeyPressEvent.getType()); keysNavigation = new KeyCommandSet(Gerrit.C.sectionNavigation()); - add(header = new Header(keysNavigation, base, revision, path)); - add(diffTable = new DiffTable(this, base, revision, path)); + header = new Header(keysNavigation, base, revision, path); + diffTable = new DiffTable(this, base, revision, path); add(uiBinder.createAndBindUi(this)); } @@ -248,8 +248,8 @@ public class SideBySide2 extends Screen { } })); resizeCodeMirror(); - Window.enableScrolling(false); + cmA.setOption("viewportMargin", 10); cmB.setOption("viewportMargin", 10); cmB.setCursor(LineCharacter.create(0)); @@ -267,8 +267,13 @@ public class SideBySide2 extends Screen { resizeHandler.removeHandler(); resizeHandler = null; } - cmA.getWrapperElement().removeFromParent(); - cmB.getWrapperElement().removeFromParent(); + if (cmA != null) { + cmA.getWrapperElement().removeFromParent(); + } + if (cmB != null) { + cmB.getWrapperElement().removeFromParent(); + } + Window.enableScrolling(true); Gerrit.setHeaderVisible(true); } @@ -463,7 +468,7 @@ public class SideBySide2 extends Screen { .set("lineNumbers", true) .set("tabSize", pref.getTabSize()) .set("mode", getContentType(meta)) - .set("lineWrapping", true) + .set("lineWrapping", false) .set("styleSelectedText", true) .set("showTrailingSpace", pref.isShowWhitespaceErrors()) .set("keyMap", "vim_ro") |