Defer showing the patch set table until it is fully built

This way we don't show a partial table to the user, where the rows
are present but lack row items.  The user might try to use their
keyboard to navigate within the table while its still being built,
resulting in unexpected behavior.

We now wrap the table into a <div> that contains either the table,
or the progress meter.  The table is only put into place once the
entire thing is prepared, and all row items have been assigned.

Bug: GERRIT-94
Signed-off-by: Shawn O. Pearce <sop@google.com>

1 files changed, 167 insertions, 135 deletions

diff --git a/src/main/java/com/google/gerrit/client/changes/PatchTable.java b/src/main/java/com/google/gerrit/client/changes/PatchTable.java
index d41569daa4..b0cf08f75c 100644
--- a/src/main/java/com/google/gerrit/client/changes/PatchTable.java
+++ b/src/main/java/com/google/gerrit/client/changes/PatchTable.java
@@ -21,24 +21,24 @@ import com.google.gerrit.client.ui.FancyFlexTable;
 import com.google.gwt.user.client.DeferredCommand;
 import com.google.gwt.user.client.History;
 import com.google.gwt.user.client.IncrementalCommand;
+import com.google.gwt.user.client.ui.Composite;
+import com.google.gwt.user.client.ui.FlowPanel;
 import com.google.gwt.user.client.ui.SourcesTableEvents;
 import com.google.gwt.user.client.ui.TableListener;
 import com.google.gwtexpui.progress.client.ProgressBar;
+import com.google.gwtexpui.safehtml.client.SafeHtml;
 import com.google.gwtexpui.safehtml.client.SafeHtmlBuilder;
 
 import java.util.List;
 
-public class PatchTable extends FancyFlexTable<Patch> {
+public class PatchTable extends Composite {
+  private final FlowPanel myBody;
   private PatchSet.Id psid;
+  private String savePointerId;
 
   public PatchTable() {
-    table.addTableListener(new TableListener() {
-      public void onCellClicked(SourcesTableEvents sender, int row, int cell) {
-        if (row > 0) {
-          movePointerTo(row);
-        }
-      }
-    });
+    myBody = new FlowPanel();
+    initWidget(myBody);
   }
 
   public void display(final PatchSet.Id id, final List<Patch> list) {
@@ -47,150 +47,181 @@ public class PatchTable extends FancyFlexTable<Patch> {
     if (cmd.execute()) {
       cmd.initMeter();
       DeferredCommand.addCommand(cmd);
+    } else {
+      cmd.showTable();
     }
   }
 
-  private void appendHeader(final SafeHtmlBuilder m) {
-    m.openTr();
-
-    m.openTd();
-    m.addStyleName(S_ICON_HEADER);
-    m.addStyleName("LeftMostCell");
-    m.nbsp();
-    m.closeTd();
-
-    m.openTd();
-    m.setStyleName(S_ICON_HEADER);
-    m.nbsp();
-    m.closeTd();
-
-    m.openTd();
-    m.setStyleName(S_DATA_HEADER);
-    m.append(Util.C.patchTableColumnName());
-    m.closeTd();
-
-    m.openTd();
-    m.setStyleName(S_DATA_HEADER);
-    m.append(Util.C.patchTableColumnComments());
-    m.closeTd();
-
-    m.openTd();
-    m.setStyleName(S_DATA_HEADER);
-    m.setAttribute("colspan", 2);
-    m.append(Util.C.patchTableColumnDiff());
-    m.closeTd();
-
-    m.closeTr();
+  public void setSavePointerId(final String id) {
+    savePointerId = id;
   }
 
-  private void appendRow(final SafeHtmlBuilder m, final Patch p) {
-    m.openTr();
+  private class MyTable extends FancyFlexTable<Patch> {
+    MyTable() {
+      table.addTableListener(new TableListener() {
+        public void onCellClicked(SourcesTableEvents sender, int row, int cell) {
+          if (row > 0) {
+            movePointerTo(row);
+          }
+        }
+      });
+      setSavePointerId(PatchTable.this.savePointerId);
+    }
+
+    @Override
+    public void resetHtml(final SafeHtml html) {
+      super.resetHtml(html);
+    }
 
-    m.openTd();
-    m.addStyleName(S_ICON_CELL);
-    m.addStyleName("LeftMostCell");
-    m.nbsp();
-    m.closeTd();
+    @Override
+    public void setRowItem(final int row, final Patch p) {
+      super.setRowItem(row, p);
+    }
 
-    m.openTd();
-    m.setStyleName("ChangeTypeCell");
-    m.append(p.getChangeType().getCode());
-    m.closeTd();
+    void appendHeader(final SafeHtmlBuilder m) {
+      m.openTr();
 
-    m.openTd();
-    m.addStyleName(S_DATA_CELL);
-    m.addStyleName("FilePathCell");
+      m.openTd();
+      m.addStyleName(S_ICON_HEADER);
+      m.addStyleName("LeftMostCell");
+      m.nbsp();
+      m.closeTd();
 
-    m.openAnchor();
-    if (p.getPatchType() == Patch.PatchType.UNIFIED) {
-      m.setAttribute("href", "#" + Link.toPatchSideBySide(p.getKey()));
-    } else {
-      m.setAttribute("href", "#" + Link.toPatchUnified(p.getKey()));
+      m.openTd();
+      m.setStyleName(S_ICON_HEADER);
+      m.nbsp();
+      m.closeTd();
+
+      m.openTd();
+      m.setStyleName(S_DATA_HEADER);
+      m.append(Util.C.patchTableColumnName());
+      m.closeTd();
+
+      m.openTd();
+      m.setStyleName(S_DATA_HEADER);
+      m.append(Util.C.patchTableColumnComments());
+      m.closeTd();
+
+      m.openTd();
+      m.setStyleName(S_DATA_HEADER);
+      m.setAttribute("colspan", 2);
+      m.append(Util.C.patchTableColumnDiff());
+      m.closeTd();
+
+      m.closeTr();
     }
-    m.append(p.getFileName());
-    m.closeAnchor();
 
-    if (p.getSourceFileName() != null) {
-      final String secondLine;
-      if (p.getChangeType() == Patch.ChangeType.RENAMED) {
-        secondLine = Util.M.renamedFrom(p.getSourceFileName());
+    void appendRow(final SafeHtmlBuilder m, final Patch p) {
+      m.openTr();
+
+      m.openTd();
+      m.addStyleName(S_ICON_CELL);
+      m.addStyleName("LeftMostCell");
+      m.nbsp();
+      m.closeTd();
+
+      m.openTd();
+      m.setStyleName("ChangeTypeCell");
+      m.append(p.getChangeType().getCode());
+      m.closeTd();
 
-      } else if (p.getChangeType() == Patch.ChangeType.COPIED) {
-        secondLine = Util.M.copiedFrom(p.getSourceFileName());
+      m.openTd();
+      m.addStyleName(S_DATA_CELL);
+      m.addStyleName("FilePathCell");
 
+      m.openAnchor();
+      if (p.getPatchType() == Patch.PatchType.UNIFIED) {
+        m.setAttribute("href", "#" + Link.toPatchSideBySide(p.getKey()));
       } else {
-        secondLine = Util.M.otherFrom(p.getSourceFileName());
+        m.setAttribute("href", "#" + Link.toPatchUnified(p.getKey()));
       }
-      m.br();
-      m.openSpan();
-      m.setStyleName("SourceFilePath");
-      m.append(secondLine);
-      m.closeSpan();
-    }
-    m.closeTd();
+      m.append(p.getFileName());
+      m.closeAnchor();
 
-    m.openTd();
-    m.addStyleName(S_DATA_CELL);
-    m.addStyleName("CommentCell");
-    if (p.getCommentCount() > 0) {
-      m.append(Util.M.patchTableComments(p.getCommentCount()));
-    }
-    if (p.getDraftCount() > 0) {
+      if (p.getSourceFileName() != null) {
+        final String secondLine;
+        if (p.getChangeType() == Patch.ChangeType.RENAMED) {
+          secondLine = Util.M.renamedFrom(p.getSourceFileName());
+
+        } else if (p.getChangeType() == Patch.ChangeType.COPIED) {
+          secondLine = Util.M.copiedFrom(p.getSourceFileName());
+
+        } else {
+          secondLine = Util.M.otherFrom(p.getSourceFileName());
+        }
+        m.br();
+        m.openSpan();
+        m.setStyleName("SourceFilePath");
+        m.append(secondLine);
+        m.closeSpan();
+      }
+      m.closeTd();
+
+      m.openTd();
+      m.addStyleName(S_DATA_CELL);
+      m.addStyleName("CommentCell");
       if (p.getCommentCount() > 0) {
-        m.append(", ");
+        m.append(Util.M.patchTableComments(p.getCommentCount()));
       }
-      m.openSpan();
-      m.setStyleName("Drafts");
-      m.append(Util.M.patchTableDrafts(p.getDraftCount()));
-      m.closeSpan();
-    }
-    m.closeTd();
+      if (p.getDraftCount() > 0) {
+        if (p.getCommentCount() > 0) {
+          m.append(", ");
+        }
+        m.openSpan();
+        m.setStyleName("Drafts");
+        m.append(Util.M.patchTableDrafts(p.getDraftCount()));
+        m.closeSpan();
+      }
+      m.closeTd();
+
+      m.openTd();
+      m.addStyleName(S_DATA_CELL);
+      m.addStyleName("DiffLinkCell");
+      if (p.getPatchType() == Patch.PatchType.UNIFIED) {
+        m.openAnchor();
+        m.setAttribute("href", "#" + Link.toPatchSideBySide(p.getKey()));
+        m.append(Util.C.patchTableDiffSideBySide());
+        m.closeAnchor();
+      } else {
+        m.nbsp();
+      }
+      m.closeTd();
 
-    m.openTd();
-    m.addStyleName(S_DATA_CELL);
-    m.addStyleName("DiffLinkCell");
-    if (p.getPatchType() == Patch.PatchType.UNIFIED) {
+      m.openTd();
+      m.addStyleName(S_DATA_CELL);
+      m.addStyleName("DiffLinkCell");
       m.openAnchor();
-      m.setAttribute("href", "#" + Link.toPatchSideBySide(p.getKey()));
-      m.append(Util.C.patchTableDiffSideBySide());
+      m.setAttribute("href", "#" + Link.toPatchUnified(p.getKey()));
+      m.append(Util.C.patchTableDiffUnified());
       m.closeAnchor();
-    } else {
-      m.nbsp();
+      m.closeTd();
+
+      m.closeTr();
     }
-    m.closeTd();
-
-    m.openTd();
-    m.addStyleName(S_DATA_CELL);
-    m.addStyleName("DiffLinkCell");
-    m.openAnchor();
-    m.setAttribute("href", "#" + Link.toPatchUnified(p.getKey()));
-    m.append(Util.C.patchTableDiffUnified());
-    m.closeAnchor();
-    m.closeTd();
-
-    m.closeTr();
-  }
 
-  @Override
-  protected Object getRowItemKey(final Patch item) {
-    return item.getKey();
-  }
+    @Override
+    protected Object getRowItemKey(final Patch item) {
+      return item.getKey();
+    }
 
-  @Override
-  protected void onOpenItem(final Patch item) {
-    History.newItem(Link.toPatchSideBySide(item.getKey()));
+    @Override
+    protected void onOpenItem(final Patch item) {
+      History.newItem(Link.toPatchSideBySide(item.getKey()));
+    }
   }
 
   private final class DisplayCommand implements IncrementalCommand {
+    private final MyTable table;
     private final List<Patch> list;
     private boolean attached;
     private SafeHtmlBuilder nc = new SafeHtmlBuilder();
-    private int stage;
+    private int stage = 0;
     private int row;
     private double start;
     private ProgressBar meter;
 
     private DisplayCommand(final List<Patch> list) {
+      this.table = new MyTable();
       this.list = list;
     }
 
@@ -213,52 +244,53 @@ public class PatchTable extends FancyFlexTable<Patch> {
       switch (stage) {
         case 0:
           if (row == 0) {
-            appendHeader(nc);
+            table.appendHeader(nc);
           }
           while (row < list.size()) {
-            appendRow(nc, list.get(row));
+            table.appendRow(nc, list.get(row));
             if ((++row % 10) == 0 && longRunning()) {
               updateMeter();
               return true;
             }
           }
-          resetHtml(nc);
+          table.resetHtml(nc);
           nc = null;
-          meter = null;
-
           stage = 1;
           row = 0;
 
         case 1:
           while (row < list.size()) {
-            setRowItem(row + 1, list.get(row));
-            if ((++row % 10) == 0 && longRunning()) {
+            table.setRowItem(row + 1, list.get(row));
+            if ((++row % 50) == 0 && longRunning()) {
+              updateMeter();
               return true;
             }
           }
-          finishDisplay(false);
+          updateMeter();
+          showTable();
       }
       return false;
     }
 
-    void initMeter() {
-      if (stage == 0 && meter == null) {
-        final SafeHtmlBuilder b = new SafeHtmlBuilder();
-        b.openTr();
-        b.openTd();
-        b.closeTd();
-        b.closeTr();
-        resetHtml(b);
+    void showTable() {
+      myBody.clear();
+      myBody.add(table);
+      table.finishDisplay(false);
+    }
 
+    void initMeter() {
+      if (meter == null) {
         meter = new ProgressBar(Util.M.loadingPatchSet(psid.get()));
-        table.setWidget(0, 0, meter);
+        myBody.clear();
+        myBody.add(meter);
       }
       updateMeter();
     }
 
     void updateMeter() {
       if (meter != null) {
-        meter.setValue((100 * row / list.size()));
+        final int n = list.size();
+        meter.setValue(((100 * (stage * n + row)) / (2 * n)));
       }
     }