SideBySide2: Revise SkipBar and disable horizontal scroll on LineWidgets
CodeMirror finally supports hiding the first line completely, revise
SkipBar implementation to be less hackish.
While we now use horizontal scrolling, line widgets (SkipBars and
CommentBoxes) should not scroll with the content. Set "noHScroll"
to true when creating them.
Remove clipping when calling scrollTo() since CodeMirror now handles that.
Change-Id: I0c08597bb1c856b332fe96fa700fea0a68504c9a
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 b8963e2..23af720 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
@@ -39,7 +39,6 @@
String intralineBg();
String diff();
String activeLine();
- String hideNumber();
String range();
String rangeHighlight();
String showtabs();
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 18d497c..2579cb8 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
@@ -94,12 +94,6 @@
height: 1.11em;
cursor: pointer;
}
- .hideNumber .CodeMirror-linenumber {
- color: transparent;
- background-color: #def;
- padding-right: 20px;
- height: 1.3em;
- }
.difftable .CodeMirror.cm-keymap-fat-cursor div.CodeMirror-cursor {
opacity: 0.8;
z-index: 2;
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 e0c0cd4..9897f9a 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
@@ -626,7 +626,8 @@
manager.insert(box, index);
Configuration config = Configuration.create()
.set("coverGutter", true)
- .set("insertAt", index);
+ .set("insertAt", index)
+ .set("noHScroll", true);
LineWidget boxWidget = cm.addLineWidget(line, box.getElement(), config);
box.setPaddingManager(manager);
box.setSelfWidgetWrapper(new PaddingWidgetWrapper(boxWidget, box.getElement()));
@@ -793,29 +794,25 @@
private SkipBar renderSkipHelper(CodeMirror cm, SkippedLine skip) {
int size = skip.getSize();
- int markStart = cm == cmA ? skip.getStartA() - 1 : skip.getStartB() - 1;
+ int markStart = cm == cmA ? skip.getStartA() : skip.getStartB();
int markEnd = markStart + size;
SkipBar bar = new SkipBar(cm);
diffTable.add(bar);
- /**
- * Due to CodeMirror limitation, there's no way to make the first
- * line disappear completely, and CodeMirror doesn't like manually
- * setting the display of a line to "none". The workaround here uses
- * inline widget for the first line and regular line widgets for others.
- */
- Configuration markerConfig;
- if (markStart == -1) {
- markerConfig = Configuration.create()
+ Configuration markerConfig = Configuration.create()
+ .set("collapsed", true)
.set("inclusiveLeft", true)
- .set("inclusiveRight", true)
- .set("replacedWith", bar.getElement());
- cm.addLineClass(0, LineClassWhere.WRAP, DiffTable.style.hideNumber());
+ .set("inclusiveRight", true);
+ Configuration lineWidgetConfig = Configuration.create()
+ .set("coverGutter", true)
+ .set("noHScroll", true);
+ if (markStart == 0) {
+ bar.setWidget(cm.addLineWidget(
+ markEnd + 1, bar.getElement(), lineWidgetConfig.set("above", true)));
} else {
- markerConfig = Configuration.create().set("collapsed", true);
- Configuration config = Configuration.create().set("coverGutter", true);
- bar.setWidget(cm.addLineWidget(markStart, bar.getElement(), config));
+ bar.setWidget(cm.addLineWidget(
+ markStart - 1, bar.getElement(), lineWidgetConfig));
}
- bar.setMarker(cm.markText(CodeMirror.pos(markStart),
+ bar.setMarker(cm.markText(CodeMirror.pos(markStart, 0),
CodeMirror.pos(markEnd), markerConfig), size);
return bar;
}
@@ -894,7 +891,8 @@
div.getStyle().setHeight(height, unit);
Configuration config = Configuration.create()
.set("coverGutter", true)
- .set("above", line == -1);
+ .set("above", line == -1)
+ .set("noHScroll", true);
if (index != null) {
config = config.set("insertAt", index);
}
@@ -1117,10 +1115,9 @@
CodeMirror targetCm = getCmFromSide(target.getSide());
targetCm.setCursor(LineCharacter.create(target.getStart()));
targetCm.focus();
- targetCm.scrollToY(Math.max(
- 0,
+ targetCm.scrollToY(
targetCm.heightAtLine(target.getStart(), "local") -
- 0.5 * cmB.getScrollbarV().getClientHeight()));
+ 0.5 * cmB.getScrollbarV().getClientHeight());
}
};
}
diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/SidePanel.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/SidePanel.java
index fb1a96f..6a19b30 100644
--- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/SidePanel.java
+++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/SidePanel.java
@@ -125,7 +125,7 @@
@Override
public void onClick(ClickEvent event) {
cm.setCursor(LineCharacter.create(line));
- cm.scrollToY(Math.max(0, height - 0.5 * scrollbarHeight));
+ cm.scrollToY(height - 0.5 * scrollbarHeight);
cm.focus();
}
});
diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/SkipBar.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/SkipBar.java
index 6d63e63..cc8ac2e 100644
--- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/SkipBar.java
+++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/SkipBar.java
@@ -30,7 +30,6 @@
import com.google.gwt.user.client.ui.HTMLPanel;
import net.codemirror.lib.CodeMirror;
-import net.codemirror.lib.CodeMirror.LineClassWhere;
import net.codemirror.lib.Configuration;
import net.codemirror.lib.LineWidget;
import net.codemirror.lib.TextMarker;
@@ -42,8 +41,10 @@
private static final Binder uiBinder = GWT.create(Binder.class);
private static final int NUM_ROWS_TO_EXPAND = 10;
private static final int UP_DOWN_THRESHOLD = 30;
- private static final Configuration COLLAPSED =
- Configuration.create().set("collapsed", true);
+ private static final Configuration MARKER_CONFIG = Configuration.create()
+ .set("collapsed", true)
+ .set("inclusiveLeft", true)
+ .set("inclusiveRight", true);
private LineWidget widget;
@@ -124,11 +125,8 @@
private void clearMarkerAndWidget() {
marker.clear();
- if (widget != null) {
- widget.clear();
- } else {
- cm.removeLineClass(0, LineClassWhere.WRAP, DiffTable.style.hideNumber());
- }
+ assert (widget != null);
+ widget.clear();
}
private void expandAll() {
@@ -143,10 +141,12 @@
int newStart = oldStart + NUM_ROWS_TO_EXPAND;
int end = fromTo.getTo().getLine();
clearMarkerAndWidget();
- marker = cm.markText(CodeMirror.pos(newStart), CodeMirror.pos(end), COLLAPSED);
- Configuration config = Configuration.create().set("coverGutter", true);
- LineWidget newWidget = cm.addLineWidget(newStart, getElement(), config);
- setWidget(newWidget);
+ marker = cm.markText(
+ CodeMirror.pos(newStart, 0), CodeMirror.pos(end), MARKER_CONFIG);
+ Configuration config = Configuration.create()
+ .set("coverGutter", true)
+ .set("noHScroll", true);
+ setWidget(cm.addLineWidget(newStart - 1, getElement(), config));
updateSkipNum();
cm.focus();
}
@@ -157,17 +157,15 @@
int oldEnd = fromTo.getTo().getLine();
int newEnd = oldEnd - NUM_ROWS_TO_EXPAND;
marker.clear();
- if (widget == null) { // First line workaround
- marker = cm.markText(CodeMirror.pos(-1),
- CodeMirror.pos(newEnd),
- Configuration.create()
- .set("inclusiveLeft", true)
- .set("inclusiveRight", true)
- .set("replacedWith", getElement()));
- } else {
- marker = cm.markText(CodeMirror.pos(start),
- CodeMirror.pos(newEnd),
- COLLAPSED);
+ marker = cm.markText(
+ CodeMirror.pos(start, 0), CodeMirror.pos(newEnd), MARKER_CONFIG);
+ if (start == 0) { // First line workaround
+ Configuration config = Configuration.create()
+ .set("coverGutter", true)
+ .set("noHScroll", true)
+ .set("above", true);
+ widget.clear();
+ setWidget(cm.addLineWidget(newEnd + 1, getElement(), config));
}
updateSkipNum();
cm.focus();
