From afb8e25bd5a357ab0ab19d834722eb3c231a1f12 Mon Sep 17 00:00:00 2001 From: "Shawn O. Pearce" Date: Sat, 16 May 2009 14:32:10 -0700 Subject: [PATCH] Implement n/p keys to jump to next/previous diff chunk or comment In a large file displayed with context = Whole File these keyboard keys can be very useful to locate the next hunk or prior hunk and jump immediately to them. Bug: GERRIT-136 Signed-off-by: Shawn O. Pearce --- .../patches/AbstractPatchContentTable.java | 91 +++++++++++++++++++ .../gerrit/client/patches/PatchConstants.java | 2 + .../client/patches/PatchConstants.properties | 2 + .../gerrit/client/ui/FancyFlexTable.java | 42 +++++++++ .../gerrit/client/ui/NavigationTable.java | 8 +- 5 files changed, 144 insertions(+), 1 deletion(-) diff --git a/src/main/java/com/google/gerrit/client/patches/AbstractPatchContentTable.java b/src/main/java/com/google/gerrit/client/patches/AbstractPatchContentTable.java index d692488754..f26d775dec 100644 --- a/src/main/java/com/google/gerrit/client/patches/AbstractPatchContentTable.java +++ b/src/main/java/com/google/gerrit/client/patches/AbstractPatchContentTable.java @@ -61,6 +61,8 @@ public abstract class AbstractPatchContentTable extends NavigationTable keysNavigation.add(new UpToChangeCommand(0, 'u', PatchUtil.C.upToChange())); keysNavigation.add(new PrevKeyCommand(0, 'k', PatchUtil.C.linePrev())); keysNavigation.add(new NextKeyCommand(0, 'j', PatchUtil.C.lineNext())); + keysNavigation.add(new PrevChunkKeyCmd(0, 'p', PatchUtil.C.chunkPrev())); + keysNavigation.add(new NextChunkKeyCmd(0, 'n', PatchUtil.C.chunkNext())); if (Gerrit.isSignedIn()) { keysAction.add(new InsertCommentCommand(0, 'c', PatchUtil.C @@ -142,6 +144,73 @@ public abstract class AbstractPatchContentTable extends NavigationTable return null; } + private boolean isChunk(final int row) { + final Object o = getRowItem(row); + if (o instanceof PatchLine) { + final PatchLine pl = (PatchLine) o; + switch (pl.getType()) { + case DELETE: + case INSERT: + case REPLACE: + return true; + } + } else if (o instanceof CommentList) { + return true; + } + return false; + } + + private int findChunkStart(int row) { + while (0 <= row && isChunk(row)) { + row--; + } + return row + 1; + } + + private int findChunkEnd(int row) { + final int max = table.getRowCount(); + while (row < max && isChunk(row)) { + row++; + } + return row - 1; + } + + private static int oneBefore(final int begin) { + return 1 <= begin ? begin - 1 : begin; + } + + private int oneAfter(final int end) { + return end + 1 < table.getRowCount() ? end + 1 : end; + } + + private void moveToPrevChunk(int row) { + while (0 <= row && isChunk(row)) { + row--; + } + for (; 0 <= row; row--) { + if (isChunk(row)) { + final int start = findChunkStart(row); + movePointerTo(start, false); + scrollIntoView(oneBefore(start), oneAfter(row)); + break; + } + } + } + + private void moveToNextChunk(int row) { + final int max = table.getRowCount(); + while (row < max && isChunk(row)) { + row++; + } + for (; row < max; row++) { + if (isChunk(row)) { + movePointerTo(row, false); + scrollIntoView(oneBefore(row), oneAfter(findChunkEnd(row))); + break; + } + } + } + /** Invoked when the user clicks on a table cell. */ protected abstract void onCellDoubleClick(int row, int column); @@ -451,4 +520,26 @@ public abstract class AbstractPatchContentTable extends NavigationTable new PublishCommentScreen(id)); } } + + public class PrevChunkKeyCmd extends KeyCommand { + public PrevChunkKeyCmd(int mask, int key, String help) { + super(mask, key, help); + } + + @Override + public void onKeyPress(final KeyPressEvent event) { + moveToPrevChunk(getCurrentRow()); + } + } + + public class NextChunkKeyCmd extends KeyCommand { + public NextChunkKeyCmd(int mask, int key, String help) { + super(mask, key, help); + } + + @Override + public void onKeyPress(final KeyPressEvent event) { + moveToNextChunk(getCurrentRow()); + } + } } diff --git a/src/main/java/com/google/gerrit/client/patches/PatchConstants.java b/src/main/java/com/google/gerrit/client/patches/PatchConstants.java index ae4a4790cb..11c6e0a9b1 100644 --- a/src/main/java/com/google/gerrit/client/patches/PatchConstants.java +++ b/src/main/java/com/google/gerrit/client/patches/PatchConstants.java @@ -34,6 +34,8 @@ public interface PatchConstants extends Constants { String upToChange(); String linePrev(); String lineNext(); + String chunkPrev(); + String chunkNext(); String commentInsert(); String commentSaveDraft(); String commentDiscard(); diff --git a/src/main/java/com/google/gerrit/client/patches/PatchConstants.properties b/src/main/java/com/google/gerrit/client/patches/PatchConstants.properties index f56aa3c0fd..a61fd53fab 100644 --- a/src/main/java/com/google/gerrit/client/patches/PatchConstants.properties +++ b/src/main/java/com/google/gerrit/client/patches/PatchConstants.properties @@ -15,6 +15,8 @@ patchHistoryTitle = Patch History upToChange = Up to change linePrev = Previous line lineNext = Next line +chunkPrev = Previous diff chunk or comment +chunkNext = Next diff chunk or comment commentInsert = Create a new inline comment commentSaveDraft = Save draft comment commentDiscard = Discard draft comment diff --git a/src/main/java/com/google/gerrit/client/ui/FancyFlexTable.java b/src/main/java/com/google/gerrit/client/ui/FancyFlexTable.java index aac7b7810f..2dd808f139 100644 --- a/src/main/java/com/google/gerrit/client/ui/FancyFlexTable.java +++ b/src/main/java/com/google/gerrit/client/ui/FancyFlexTable.java @@ -15,10 +15,13 @@ package com.google.gerrit.client.ui; import com.google.gwt.core.client.GWT; +import com.google.gwt.dom.client.Document; +import com.google.gwt.user.client.DOM; import com.google.gwt.user.client.Element; import com.google.gwt.user.client.ui.Composite; import com.google.gwt.user.client.ui.FlexTable; import com.google.gwt.user.client.ui.Widget; +import com.google.gwt.user.client.ui.HTMLTable.CellFormatter; import com.google.gwtexpui.safehtml.client.SafeHtml; import java.util.Iterator; @@ -69,6 +72,45 @@ public abstract class FancyFlexTable extends Composite { impl.resetHtml(table, body); } + protected void scrollIntoView(final int topRow, final int endRow) { + final CellFormatter fmt = table.getCellFormatter(); + final Element top = DOM.getParent(fmt.getElement(topRow, C_ARROW)); + final Element end = DOM.getParent(fmt.getElement(endRow, C_ARROW)); + + final int rTop = top.getAbsoluteTop(); + final int rEnd = end.getAbsoluteTop() + end.getOffsetHeight(); + final int rHeight = rEnd - rTop; + + final int sTop = Document.get().getScrollTop(); + final int sHeight = Document.get().getClientHeight(); + final int sEnd = sTop + sHeight; + + final int nTop; + if (sHeight <= rHeight) { + // The region is larger than the visible area, make the top + // exactly the top of the region, its the most visible area. + // + nTop = rTop; + } else if (sTop <= rTop && rTop <= sEnd) { + // At least part of the region is already visible. + // + if (rEnd <= sEnd) { + // ... actually its all visible. Don't scroll. + // + return; + } + + // Move only enough to make the end visible. + // + nTop = sTop + (rHeight - (sEnd - rTop)); + } else { + // None of the region is visible. Make it visible. + // + nTop = rTop; + } + Document.get().setScrollTop(nTop); + } + protected void applyDataRowStyle(final int newRow) { table.getCellFormatter().addStyleName(newRow, C_ARROW, S_ICON_CELL); table.getCellFormatter().addStyleName(newRow, C_ARROW, S_LEFT_MOST_CELL); diff --git a/src/main/java/com/google/gerrit/client/ui/NavigationTable.java b/src/main/java/com/google/gerrit/client/ui/NavigationTable.java index d8c4301c1f..5a2efc8f9b 100644 --- a/src/main/java/com/google/gerrit/client/ui/NavigationTable.java +++ b/src/main/java/com/google/gerrit/client/ui/NavigationTable.java @@ -90,6 +90,10 @@ public abstract class NavigationTable extends FancyFlexTable { } protected void movePointerTo(final int newRow) { + movePointerTo(newRow, true); + } + + protected void movePointerTo(final int newRow, final boolean scroll) { final CellFormatter fmt = table.getCellFormatter(); final boolean clear = 0 <= currentRow && currentRow < table.getRowCount(); if (clear) { @@ -100,7 +104,9 @@ public abstract class NavigationTable extends FancyFlexTable { table.setWidget(newRow, C_ARROW, pointer); final Element tr = DOM.getParent(fmt.getElement(newRow, C_ARROW)); UIObject.setStyleName(tr, S_ACTIVE_ROW, true); - tr.scrollIntoView(); + if (scroll) { + tr.scrollIntoView(); + } } else if (clear) { table.setWidget(currentRow, C_ARROW, null); }