Fix render logic for RelatedChangesTab.NavigationList + Re-render the NavigationList whenever the visible rows change. This fixes a bug where calling setMaxHeight() after the NavigationList has been rendered, does not actually add rows to fill the new visible range. Also, guard against executing the render logic until the rowHeight has been measured. + Correctly remember the selected row across iterations of DisplayCommand. Change-Id: Ifb17f1c2199eac67e166b862833ce7221fea6732
diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/RelatedChangesTab.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/RelatedChangesTab.java index ff89f36..64e013e 100644 --- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/RelatedChangesTab.java +++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/RelatedChangesTab.java
@@ -112,6 +112,7 @@ maxHeight = height; if (view != null) { view.setHeight(height + "px"); + view.ensureRowMeasurements(); view.movePointerTo(view.selectedRow, true); } } @@ -155,6 +156,7 @@ private double start; private int row; private int connectedPos; + private int selected; private DisplayCommand(String revision, JsArray<ChangeAndCommit> changes, NavigationList navList) { @@ -211,14 +213,13 @@ return true; } - int select = 0; while (row < changes.length()) { ChangeAndCommit info = changes.get(row); String commit = info.commit().commit(); rows.add(new RowSafeHtml( info, connected != null && !connected.contains(commit))); if (revision.equals(commit)) { - select = row; + selected = row; } if (longRunning(++row)) { return true; @@ -226,7 +227,8 @@ } navList.rows = rows; - navList.movePointerTo(select, true); + navList.ensureRowMeasurements(); + navList.movePointerTo(selected, true); return false; } @@ -322,19 +324,17 @@ private class NavigationList extends ScrollPanel implements ClickHandler, DoubleClickHandler, ScrollHandler { - List<SafeHtml> rows; private final KeyCommandSet keysNavigation; private final Element body; private final Element surrogate; private final Node fragment = createDocumentFragment(); + List<SafeHtml> rows; private HandlerRegistration regNavigation; private int selectedRow; private int startRow; private int rowHeight; private int rowWidth; - private int top; - private int bottom; NavigationList() { addDomHandler(this, ClickEvent.getType()); @@ -376,9 +376,8 @@ } private void ensureRowMeasurements() { - if (rowHeight == 0) { + if (rowHeight == 0 && rows != null) { surrogate.setInnerSafeHtml(rows.get(0)); - getContainerElement().appendChild(surrogate); rowHeight = surrogate.getOffsetHeight(); rowWidth = surrogate.getOffsetWidth(); @@ -393,12 +392,10 @@ renderSelected(selectedRow, false); selectedRow = row; - if (scroll) { + if (scroll && rowHeight != 0) { // Position the selected row in the middle. - ensureRowMeasurements(); - int pos = Math.max(rowHeight * selectedRow - maxHeight / 2, 0); - setVerticalScrollPosition(pos); - + setVerticalScrollPosition( + Math.max(rowHeight * selectedRow - maxHeight / 2, 0)); render(); } renderSelected(selectedRow, true); @@ -417,23 +414,19 @@ } private void render() { - if (rows == null) { - return; - } - - int currChildren = body.getChildCount(); - int vpos = getVerticalScrollPosition(); - if (currChildren > 0 && top <= vpos && vpos <= bottom) { + if (rows == null || rowHeight == 0) { return; } int currStart = startRow; - int currEnd = startRow + currChildren; + int currEnd = startRow + body.getChildCount(); - ensureRowMeasurements(); - int page = maxHeight / rowHeight; + int vpos = getVerticalScrollPosition(); int start = Math.max(vpos / rowHeight - 5, 0); - int end = Math.min(vpos / rowHeight + page + 5, rows.size()); + int end = Math.min((vpos + maxHeight) / rowHeight + 5, rows.size()); + if (currStart <= start && end <= currEnd) { + return; // All of the required nodes are already in the DOM. + } if (end <= currStart) { renderRange(start, end, true, true); @@ -459,14 +452,6 @@ } private void renderRange(int start, int end, boolean removeAll, boolean insertFirst) { - if (insertFirst || removeAll) { - startRow = start; - top = start * rowHeight; - } - if (!insertFirst || removeAll) { - bottom = (end - 2) * rowHeight - maxHeight; - } - SafeHtmlBuilder sb = new SafeHtmlBuilder(); for (int i = start; i < end; i++) { sb.append(rows.get(i)); @@ -474,7 +459,6 @@ if (removeAll) { body.setInnerSafeHtml(sb); - body.getStyle().setTop(top, Style.Unit.PX); } else { surrogate.setInnerSafeHtml(sb); for (int cnt = surrogate.getChildCount(); cnt > 0; cnt--) { @@ -482,11 +466,15 @@ } if (insertFirst) { body.insertFirst(fragment); - body.getStyle().setTop(top, Style.Unit.PX); } else { body.appendChild(fragment); } } + + if (insertFirst || removeAll) { + startRow = start; + body.getStyle().setTop(start * rowHeight, Style.Unit.PX); + } } @Override