opendev/gerrit
Code Issues Proposed changes

SideBySide2: Fix misplaced widgets at bottom of window

Browse Source 
In 5b83d94271 ("SideBySide2: Batch updates to CodeMirror3 during
onShowView") Colby and I tried to improve rendering performance by
avoiding multiple redisplays of the CM3 widgets during onShowView().

That change worked to reduce DOM update cost, but the batched
setHeight and setOption caused the widgets to be drawn at the
bottom of the window when the file first opened.

A LineWidget needs to be added to diffTable to respond to browser
events, but CodeMirror doesn't render the widget until the containing
line is scrolled into viewportMargin, causing it to appear at the
bottom of the DOM upon loading. Fix by hiding the widget until it is
first scrolled into view (when CodeMirror fires a "redraw" event on
the widget).

With this fix, viewportMargin does not need to be set to infinity
anymore. Leave it at the default of 10 lines. During onShowView only
the height has to be updated, so wrapping the update in an operation
is no longer required.

This fixes the skip bar and comment widget display bugs, apparently
without reintroducing the latency Colby observed on Thursday.

Change-Id: Id55232853bcfde12d065fd88b69f3538115f2dfe
This commit is contained in:
Shawn Pearce
2013-11-22 19:51:11 -08:00
committed by Michael Zhou
parent 77863fc0ee
commit 5c8a75b4b5
4 changed files with 43 additions and 34 deletions
Download Patch File Download Diff File Expand all files Collapse all files

60
gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/SideBySide2.java
View File

@@ -59,10 +59,11 @@ import com.google.gwt.event.logical.shared.ResizeHandler;
import com.google.gwt.event.shared.HandlerRegistration;
import com.google.gwt.uibinder.client.UiBinder;
import com.google.gwt.uibinder.client.UiField;
import com.google.gwt.user.client.DOM;
import com.google.gwt.user.client.Window;
import com.google.gwt.user.client.rpc.AsyncCallback;
import com.google.gwt.user.client.ui.FlowPanel;
import com.google.gwt.user.client.ui.SimplePanel;
import com.google.gwt.user.client.ui.Widget;
import com.google.gwtexpui.globalkey.client.GlobalKey;
import com.google.gwtexpui.globalkey.client.KeyCommand;
import com.google.gwtexpui.globalkey.client.KeyCommandSet;
@@ -234,8 +235,8 @@ public class SideBySide2 extends Screen {
Window.enableScrolling(false);
final int height = getCodeMirrorHeight();
onShowView(cmA, height);
onShowView(cmB, height);
cmA.setHeight(height);
cmB.setHeight(height);
diffTable.sidePanel.adjustGutters(cmB);
cmB.setCursor(LineCharacter.create(0));
cmB.focus();
@@ -243,17 +244,6 @@ public class SideBySide2 extends Screen {
prefetchNextFile();
}
private void onShowView(final CodeMirror cm, final int height) {
cm.operation(new Runnable() {
@Override
public void run() {
cm.setHeight(height);
cm.setOption("viewportMargin", 10);
cm.refresh();
}
});
}
@Override
protected void onUnload() {
super.onUnload();
@@ -502,10 +492,6 @@ public class SideBySide2 extends Screen {
.set("showTrailingSpace", pref.isShowWhitespaceErrors())
.set("keyMap", "vim_ro")
.set("value", meta != null ? contents : "");
// Without this, CM will put line widgets too far down in the right spot,
// and padding widgets will be informed of wrong offset height. Reset to
// 10 (default) after initial rendering.
cfg.setInfinity("viewportMargin");
return CodeMirror.create(parent, cfg);
}
@@ -654,7 +640,7 @@ public class SideBySide2 extends Screen {
.set("coverGutter", true)
.set("insertAt", index)
.set("noHScroll", true);
LineWidget boxWidget = cm.addLineWidget(line, box.getElement(), config);
LineWidget boxWidget = addLineWidget(cm, line, box, config);
box.setPaddingManager(manager);
box.setSelfWidgetWrapper(new PaddingWidgetWrapper(boxWidget, box.getElement()));
box.setParent(this);
@@ -832,11 +818,11 @@ public class SideBySide2 extends Screen {
.set("coverGutter", true)
.set("noHScroll", true);
if (markStart == 0) {
bar.setWidget(cm.addLineWidget(
markEnd + 1, bar.getElement(), lineWidgetConfig.set("above", true)));
bar.setWidget(addLineWidget(
cm, markEnd + 1, bar, lineWidgetConfig.set("above", true)));
} else {
bar.setWidget(cm.addLineWidget(
markStart - 1, bar.getElement(), lineWidgetConfig));
bar.setWidget(addLineWidget(
cm, markStart - 1, bar, lineWidgetConfig));
}
bar.setMarker(cm.markText(CodeMirror.pos(markStart, 0),
CodeMirror.pos(markEnd), markerConfig), size);
@@ -913,8 +899,8 @@ public class SideBySide2 extends Screen {
private PaddingWidgetWrapper addPaddingWidget(CodeMirror cm,
int line, double height, Unit unit, Integer index) {
Element div = DOM.createDiv();
div.getStyle().setHeight(height, unit);
SimplePanel padding = new SimplePanel();
padding.getElement().getStyle().setHeight(height, unit);
Configuration config = Configuration.create()
.set("coverGutter", true)
.set("above", line == -1)
@@ -922,8 +908,28 @@ public class SideBySide2 extends Screen {
if (index != null) {
config = config.set("insertAt", index);
}
LineWidget widget = cm.addLineWidget(line == -1 ? 0 : line, div, config);
return new PaddingWidgetWrapper(widget, div);
LineWidget widget = addLineWidget(cm, line == -1 ? 0 : line, padding, config);
return new PaddingWidgetWrapper(widget, padding.getElement());
}
/**
* A LineWidget needs to be added to diffTable in order to respond to browser
* events, but CodeMirror doesn't render the widget until the containing line
* is scrolled into viewportMargin, causing it to appear at the bottom of the
* DOM upon loading. Fix by hiding the widget until it is first scrolled into
* view (when CodeMirror fires a "redraw" event on the widget).
*/
private LineWidget addLineWidget(CodeMirror cm, int line,
final Widget widget, Configuration options) {
widget.setVisible(false);
LineWidget lineWidget = cm.addLineWidget(line, widget.getElement(), options);
lineWidget.onFirstRedraw(new Runnable() {
@Override
public void run() {
widget.setVisible(true);
}
});
return lineWidget;
}
private void clearActiveLine(CodeMirror cm) {