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>
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 d41569d..b0cf08f 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.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 @@
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();
-
- 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();
-
- 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 {
- m.setAttribute("href", "#" + Link.toPatchUnified(p.getKey()));
+ 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);
}
- 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());
-
- } 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();
+ @Override
+ public void resetHtml(final SafeHtml html) {
+ super.resetHtml(html);
}
- m.closeTd();
- m.openTd();
- m.addStyleName(S_DATA_CELL);
- m.addStyleName("CommentCell");
- if (p.getCommentCount() > 0) {
- m.append(Util.M.patchTableComments(p.getCommentCount()));
+ @Override
+ public void setRowItem(final int row, final Patch p) {
+ super.setRowItem(row, p);
}
- 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 {
+ 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();
}
- 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();
+ void appendRow(final SafeHtmlBuilder m, final Patch p) {
+ m.openTr();
- m.closeTr();
- }
+ m.openTd();
+ m.addStyleName(S_ICON_CELL);
+ m.addStyleName("LeftMostCell");
+ m.nbsp();
+ m.closeTd();
- @Override
- protected Object getRowItemKey(final Patch item) {
- return item.getKey();
- }
+ m.openTd();
+ m.setStyleName("ChangeTypeCell");
+ m.append(p.getChangeType().getCode());
+ m.closeTd();
- @Override
- protected void onOpenItem(final Patch item) {
- History.newItem(Link.toPatchSideBySide(item.getKey()));
+ 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 {
+ m.setAttribute("href", "#" + Link.toPatchUnified(p.getKey()));
+ }
+ 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());
+
+ } 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(Util.M.patchTableComments(p.getCommentCount()));
+ }
+ 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");
+ 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 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 @@
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)));
}
}
