Set viewportMargin to Infinity on CodeMirror initialization

Because CodeMirror doesn't render lines until scrolled into view,
line widgets that are also manually added to diffTable will not be
put in the right spot. Fixed by setting viewportMargin to Infinity
on initialization and resetting to default later.

Fixed minor styling issues with CommentBox, SkipBar and DiffTable.

Change-Id: I188841f569c1b61ca200c8ba946584d26142c070

gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/CodeMirrorDemo.java

@@ -40,8 +40,9 @@ import com.google.gerrit.reviewdb.client.Project;
 import com.google.gwt.core.client.JavaScriptObject;
 import com.google.gwt.core.client.JsArray;
 import com.google.gwt.core.client.JsArrayString;
+import com.google.gwt.core.client.Scheduler;
+import com.google.gwt.core.client.Scheduler.ScheduledCommand;
 import com.google.gwt.dom.client.Element;
-import com.google.gwt.dom.client.NativeEvent;
 import com.google.gwt.dom.client.Style.Unit;
 import com.google.gwt.event.dom.client.KeyCodes;
 import com.google.gwt.event.dom.client.KeyPressEvent;
@@ -184,6 +185,7 @@ public class CodeMirrorDemo extends Screen {
   @Override
   public void onShowView() {
     super.onShowView();
+
     if (cmA != null) {
       cmA.refresh();
     }
@@ -194,6 +196,17 @@ public class CodeMirrorDemo extends Screen {
     for (CommentBox box : initialBoxes) {
       box.resizePaddingWidget();
     }
+    Scheduler.get().scheduleDeferred(new ScheduledCommand() {
+      @Override
+      public void execute() {
+        if (cmA != null) {
+          cmA.setOption("viewportMargin", 10);
+        }
+        if (cmB != null) {
+          cmB.setOption("viewportMargin", 10);
+        }
+      }
+    });
     (cmB != null ? cmB : cmA).focus();
   }
 
@@ -227,26 +240,18 @@ public class CodeMirrorDemo extends Screen {
   private void registerCmEvents(CodeMirror cm) {
     cm.on("cursorActivity", updateActiveLine(cm));
     cm.on("scroll", doScroll(cm));
-    /**
-     * Trying to prevent right click from updating the cursor.
-     *
-     * TODO: Change to listen on "contextmenu" instead. Latest CM has
-     * provided a patch that will hopefully make this work.
-     */
-    cm.on("mousedown", ignoreRightClick());
+    // TODO: Prevent right click from updating the cursor.
     cm.addKeyMap(KeyMap.create().on("'u'", upToChange()));
     cm.addKeyMap(KeyMap.create().on("'o'", toggleOpenBox(cm)));
     cm.addKeyMap(KeyMap.create().on("Enter", toggleOpenBox(cm)));
+    CodeMirror.defineVimEx("up", "u", upToChange());
     if (Gerrit.isSignedIn()) {
       cm.addKeyMap(KeyMap.create().on("'c'", insertNewDraft(cm)));
     }
-    /**
-     * TODO: Maybe remove this after updating CM to HEAD. The latest VIM mode
-     * doesn't enter INSERT mode when document is read only.
-     */
-    for (String c : new String[]{"A", "C", "D", "I", "O", "P", "R", "S", "U",
-        "X", "Y", "~"}) {
-      CodeMirror.disableUnwantedKey("vim", c);
+    // TODO: Work on a better way for customizing keybindings.
+    for (String s : new String[]{"C", "O", "R", "U", "Ctrl-C", "Ctrl-F",
+        "Enter"}) {
+      CodeMirror.disableUnwantedKey("vim", s);
     }
   }
 
@@ -337,7 +342,13 @@ public class CodeMirrorDemo extends Screen {
       .set("styleSelectedText", true)
       .set("showTrailingSpace", true)
       .set("keyMap", "vim")
-      .set("value", contents);
+      .set("value", contents)
+      /**
+       * Without this, CM won't 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.
+       */
+      .setInfinity("viewportMargin");
     final CodeMirror cm = CodeMirror.create(ele, cfg);
     cm.setHeight(Window.getClientHeight() - HEADER_FOOTER);
     return cm;
@@ -662,9 +673,13 @@ public class CodeMirrorDemo extends Screen {
     final CodeMirror other = otherCm(cm);
     return new Runnable() {
       public void run() {
-        // Prevent feedback loop, Chrome seems fine but Firefox chokes.
+        /**
+         * Prevent feedback loop, Chrome seems fine but Firefox chokes.
+         * However on Chrome this may cause scrolling to be out of sync
+         * if scrolled too fast.
+         */
         double now = (double) System.currentTimeMillis();
-        if (cm.getScrollSetBy() == other && cm.getScrollSetAt() + 50 > now) {
+        if (cm.getScrollSetBy() == other && cm.getScrollSetAt() + 30 > now) {
           return;
         }
         other.scrollToY(cm.getScrollInfo().getTop());
@@ -750,16 +765,6 @@ public class CodeMirrorDemo extends Screen {
     };
   }
 
-  private CodeMirror.EventHandler ignoreRightClick() {
-    return new CodeMirror.EventHandler() {
-      public void handle(CodeMirror instance, NativeEvent event) {
-        if (event.getButton() == NativeEvent.BUTTON_RIGHT) {
-          event.preventDefault();
-        }
-      }
-    };
-  }
-
   private static String getContentType(DiffInfo.FileMeta meta) {
     return meta != null && meta.content_type() != null
         ? ModeInjector.getContentType(meta.content_type())

gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/CommentBoxHeader.java

@@ -24,10 +24,12 @@ import com.google.gwt.event.dom.client.ClickEvent;
 import com.google.gwt.event.dom.client.ClickHandler;
 import com.google.gwt.event.dom.client.HasClickHandlers;
 import com.google.gwt.event.shared.HandlerRegistration;
+import com.google.gwt.resources.client.CssResource;
 import com.google.gwt.uibinder.client.UiBinder;
 import com.google.gwt.uibinder.client.UiField;
 import com.google.gwt.user.client.ui.Composite;
 import com.google.gwt.user.client.ui.HTMLPanel;
+import com.google.gwt.user.client.ui.UIObject;
 
 import java.sql.Timestamp;
 
@@ -40,8 +42,17 @@ class CommentBoxHeader extends Composite implements HasClickHandlers {
   interface Binder extends UiBinder<HTMLPanel, CommentBoxHeader> {}
   private static Binder uiBinder = GWT.create(Binder.class);
 
+  interface CommentBoxHeaderStyle extends CssResource {
+    String name();
+    String summary();
+    String date();
+  }
+
   private boolean draft;
 
+  @UiField
+  Element avatarCell;
+
   @UiField(provided=true)
   AvatarImage avatar;
 
@@ -54,6 +65,9 @@ class CommentBoxHeader extends Composite implements HasClickHandlers {
   @UiField
   Element date;
 
+  @UiField
+  CommentBoxHeaderStyle headerStyle;
+
   CommentBoxHeader(AccountInfo author, Timestamp when, boolean isDraft) {
     if (author != null) {
       avatar = new AvatarImage(author, 26);
@@ -62,6 +76,9 @@ class CommentBoxHeader extends Composite implements HasClickHandlers {
       avatar = new AvatarImage();
     }
     initWidget(uiBinder.createAndBindUi(this));
+    if (author == null) {
+      UIObject.setVisible(avatarCell, false);
+    }
     draft = isDraft;
     if (when != null) {
       setDate(when);

gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/CommentBoxHeader.ui.xml

@@ -18,15 +18,37 @@ limitations under the License.
     xmlns:g='urn:import:com.google.gwt.user.client.ui'
     xmlns:c='urn:import:com.google.gerrit.client'>
   <ui:with field='res' type='com.google.gerrit.client.diff.CommentBoxResources' />
+  <ui:style field='headerStyle'
+    type='com.google.gerrit.client.diff.CommentBoxHeader.CommentBoxHeaderStyle'>
+    .avatarCell {
+      width: 26px;
+      padding-right: 5px;
+    }
+    .name {
+      width: 15%;
+      font-weight: bold;
+      text-overflow: ellipsis;
+      white-space: nowrap;
+    }
+    .summary {
+      width: 60%;
+    }
+    .date {
+      width: 15%;
+      text-align: right;
+    }
+  </ui:style>
     <g:HTMLPanel>
     <table class='{res.style.table}'>
       <tr>
-        <td><c:AvatarImage ui:field='avatar' /></td>
-        <td ui:field='name' class='{res.style.name}'></td>
-        <td class='{res.style.summary}'>
+        <td ui:field='avatarCell' class='{headerStyle.avatarCell}'>
+          <c:AvatarImage ui:field='avatar' />
+        </td>
+        <td ui:field='name' class='{headerStyle.name}'></td>
+        <td class='{headerStyle.summary}'>
           <div ui:field='summary' class='{res.style.summaryText}'></div>
         </td>
-        <td ui:field='date' class='{res.style.date}'></td>
+        <td ui:field='date' class='{headerStyle.date}'></td>
       </tr>
     </table>
     </g:HTMLPanel>

gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/CommentBoxResources.java

@@ -29,10 +29,7 @@ interface CommentBoxResources extends ClientBundle {
     String close();
     String commentBox();
     String table();
-    String name();
-    String summary();
     String summaryText();
-    String date();
     String contentPanel();
     String message();
     String button();

gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/CommentBoxUi.css

@@ -17,19 +17,9 @@
   table-layout: fixed;
 }
 
-.name {
-  width: 20%;
-  font-weight: bold;
-}
-
-.summary {
-  width: 60%;
-}
-
 .summaryText {
   color: #777;
   height: 1em;
-  max-width: 80%;
   overflow: hidden;
   padding-bottom: 2px;
   text-overflow: ellipsis;
@@ -40,11 +30,6 @@
   display: none;
 }
 
-.date {
-  width: 25%;
-  text-align: right;
-}
-
 .close .contentPanel {
   display: none;
 }

gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/DiffTable.ui.xml

@@ -27,6 +27,9 @@ limitations under the License.
       padding-bottom: 0.1em;
       overflow: hidden;
     }
+    .difftable .CodeMirror pre span {
+      padding-bottom: 0.1em;
+    }
     .table {
       width: 100%;
       table-layout: fixed;
@@ -81,7 +84,7 @@ limitations under the License.
     .hideNumber .CodeMirror-linenumber {
       color: transparent;
       background-color: #def;
-      width: 23px !important;
+      padding-right: 20px;
       height: 1.3em;
     }
     .difftable .CodeMirror.cm-keymap-fat-cursor div.CodeMirror-cursor {

gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/DraftBox.java

@@ -54,9 +54,6 @@ class DraftBox extends CommentBox {
   @UiField
   NpTextArea editArea;
 
-  @UiField
-  DraftBoxStyle draftStyle;
-
   @UiField
   Button edit;
 
@@ -69,6 +66,9 @@ class DraftBox extends CommentBox {
   @UiField
   Button discard;
 
+  @UiField
+  DraftBoxStyle draftStyle;
+
   private static final int INITIAL_COLS = 60;
   private static final int INITIAL_LINES = 5;
   private static final int MAX_LINES = 30;
@@ -92,18 +92,18 @@ class DraftBox extends CommentBox {
     editArea.setCharacterWidth(INITIAL_COLS);
     editArea.setVisibleLines(INITIAL_LINES);
     editArea.setSpellCheck(true);
-    if (saveOnInit) {
-      onSave(null);
-    }
-    if (isNew) {
-      addStyleName(draftStyle.newDraft());
-    }
     expandTimer = new Timer() {
       @Override
       public void run() {
         expandText();
       }
     };
+    if (saveOnInit) {
+      onSave(null);
+    }
+    if (isNew) {
+      addStyleName(draftStyle.newDraft());
+    }
     addDomHandler(new MouseMoveHandler() {
       @Override
       public void onMouseMove(MouseMoveEvent event) {
@@ -132,10 +132,12 @@ class DraftBox extends CommentBox {
       addStyleName(draftStyle.edit());
       editArea.setText(getOriginal().message());
       expandText();
+      editArea.setReadOnly(false);
       editArea.setFocus(true);
       disableClickFocusHandler();
     } else {
       expandTimer.cancel();
+      editArea.setReadOnly(true);
       removeStyleName(draftStyle.edit());
       addStyleName(draftStyle.view());
       enableClickFocusHandler();
@@ -148,6 +150,7 @@ class DraftBox extends CommentBox {
   }
 
   private void removeUI() {
+    setEdit(false);
     expandTimer.cancel();
     if (replyToBox != null) {
       replyToBox.unregisterReplyBox();
@@ -181,13 +184,13 @@ class DraftBox extends CommentBox {
     CommentInfo original = getOriginal();
     CommentInput input = CommentInput.create(original);
     input.setMessage(message);
+    setEdit(false);
     GerritCallback<CommentInfo> cb = new GerritCallback<CommentInfo>() {
       @Override
       public void onSuccess(CommentInfo result) {
         updateOriginal(result);
         setMessageText(message);
         setDate(result.updated());
-        setEdit(false);
         if (isNew) {
           removeStyleName(draftStyle.newDraft());
           isNew = false;
@@ -213,6 +216,7 @@ class DraftBox extends CommentBox {
     if (isNew) {
       removeUI();
     } else {
+      setEdit(false);
       CommentApi.deleteDraft(getPatchSetId(), getOriginal().id(),
           new GerritCallback<JavaScriptObject>() {
         @Override

gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/SkipBar.java

@@ -16,6 +16,9 @@ package com.google.gerrit.client.diff;
 
 import com.google.gerrit.client.patches.PatchUtil;
 import com.google.gwt.core.client.GWT;
+import com.google.gwt.core.client.Scheduler;
+import com.google.gwt.core.client.Scheduler.ScheduledCommand;
+import com.google.gwt.dom.client.Style.Unit;
 import com.google.gwt.event.dom.client.ClickEvent;
 import com.google.gwt.event.dom.client.ClickHandler;
 import com.google.gwt.resources.client.CssResource;
@@ -45,7 +48,6 @@ class SkipBar extends Composite {
   private LineWidget widget;
 
   interface SkipBarStyle extends CssResource {
-    String isLineWidget();
     String noExpand();
   }
 
@@ -80,9 +82,15 @@ class SkipBar extends Composite {
     }, ClickEvent.getType());
   }
 
-  void setWidget(LineWidget widget) {
-    this.widget = widget;
-    addStyleName(style.isLineWidget());
+  void setWidget(LineWidget lineWidget) {
+    widget = lineWidget;
+    Scheduler.get().scheduleDeferred(new ScheduledCommand(){
+      @Override
+      public void execute() {
+        getElement().getStyle().setPaddingLeft(
+            cm.getGutterElement().getOffsetWidth(), Unit.PX);
+      }
+    });
   }
 
   void setMarker(TextMarker marker, int length) {
@@ -108,8 +116,7 @@ class SkipBar extends Composite {
 
   private boolean checkAndUpdateArrows() {
     if (numSkipLines <= UP_DOWN_THRESHOLD) {
-      upArrow.addStyleName(style.noExpand());
-      downArrow.addStyleName(style.noExpand());
+      addStyleName(style.noExpand());
       return false;
     }
     return true;

gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/SkipBar.ui.xml

@@ -29,27 +29,24 @@ limitations under the License.
       font-style: italic;
       overflow: hidden;
     }
-    .isLineWidget .text {
-      padding-left: 31px;
-    }
     .anchor {
       color: inherit;
       text-decoration: none;
     }
-    .noExpand {
+    .noExpand .arrow {
       display: none;
     }
   </ui:style>
   <g:HTMLPanel addStyleNames='{style.skipBar}'>
   <div class='{style.text}'>
     <ui:msg>
-      <g:Anchor ui:field='upArrow' addStyleNames='{style.anchor}'>
+      <g:Anchor ui:field='upArrow' addStyleNames='{style.arrow} {style.anchor}'>
       </g:Anchor>
       <span><ui:msg>... skipped </ui:msg></span>
       <g:Anchor ui:field='skipNum' addStyleNames='{style.anchor}'>
       </g:Anchor>
       <span><ui:msg> common lines ...</ui:msg></span>
-      <g:Anchor ui:field='downArrow' addStyleNames='{style.anchor}'>
+      <g:Anchor ui:field='downArrow' addStyleNames=' {style.arrow} {style.anchor}'>
       </g:Anchor>
     </ui:msg>
   </div>

gerrit-gwtui/src/main/java/net/codemirror/lib/CodeMirror.java

@@ -38,6 +38,18 @@ public class CodeMirror extends JavaScriptObject {
     this.setOption(option, value);
   }-*/;
 
+  public final native void setOption(String option, double value) /*-{
+    this.setOption(option, value);
+  }-*/;
+
+  public final native void setOption(String option, JavaScriptObject val) /*-{
+    this.setOption(option, val);
+  }-*/;
+
+  public final native void setOptionToInfinity(String option) /*-{
+    this.setOption(option, Infinity);
+  }-*/;
+
   public final native void setValue(String v) /*-{ this.setValue(v); }-*/;
 
   public final native void setWidth(int w) /*-{ this.setSize(w, null); }-*/;
@@ -221,6 +233,16 @@ public class CodeMirror extends JavaScriptObject {
     $wnd.CodeMirror.keyMap[category][name] = undefined;
   }-*/;
 
+  public static native void defineVimEx(String name, String prefix, Runnable thunk) /*-{
+    $wnd.CodeMirror.Vim.defineEx(name, prefix, $entry(function() {
+      thunk.@java.lang.Runnable::run()();
+    }));
+  }-*/;
+
+  public final native Element getGutterElement() /*-{
+    return this.getGutterElement();
+  }-*/;
+
   protected CodeMirror() {
   }
 

gerrit-gwtui/src/main/java/net/codemirror/lib/Configuration.java

@@ -39,6 +39,9 @@ public class Configuration extends JavaScriptObject {
   public final native Configuration set(String name, JavaScriptObject val)
   /*-{ this[name] = val; return this; }-*/;
 
+  public final native Configuration setInfinity(String name)
+  /*-{ this[name] = Infinity; return this; }-*/;
+
   protected Configuration() {
   }
 }