Keyboard navigation from change screen into diffs

The support for keyboard navigation in the change screen allows
the user to start reviewing the files that were changed within the
different patch sets by just using keys.

Bug: issue 498
Change-Id: I577f0a2d5b6da20d48b5cf80035c2571ee903ebb
Signed-off-by: Edwin Kempin <edwin.kempin@gmail.com>

gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/ChangeConstants.java

@@ -47,6 +47,9 @@ public interface ChangeConstants extends Constants {
   String changeTablePagePrev();
   String changeTablePageNext();
   String upToDashboard();
+  String expandCollapseDependencies();
+  String previousPatchSet();
+  String nextPatchSet();
   String keyPublishComments();
 
   String patchTableColumnName();
@@ -59,7 +62,8 @@ public interface ChangeConstants extends Constants {
 
   String patchTablePrev();
   String patchTableNext();
-  String patchTableOpen();
+  String patchTableOpenDiff();
+  String patchTableOpenUnifiedDiff();
   String upToChangeIconLink();
   String prevPatchLinkIcon();
   String nextPatchLinkIcon();

gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/ChangeConstants.properties

@@ -27,6 +27,9 @@ changeTableStar = Star (or unstar) change
 changeTablePagePrev = Previous page of changes
 changeTablePageNext = Next page of changes
 upToDashboard = Up to dashboard
+expandCollapseDependencies = Expands / Collapses dependencies section
+previousPatchSet = Previous patch set
+nextPatchSet = Next patch set
 keyPublishComments = Review and publish comments
 
 patchTableColumnName = File Path
@@ -39,7 +42,8 @@ patchTableDownloadPostImage = new
 
 patchTablePrev = Previous file
 patchTableNext = Next file
-patchTableOpen = Open file
+patchTableOpenDiff = Open diff
+patchTableOpenUnifiedDiff = Open unified diff
 
 changeScreenIncludedIn =  Included in
 changeScreenDependencies =  Dependencies

gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/ChangeScreen.java

@@ -18,7 +18,6 @@ import com.google.gerrit.client.Gerrit;
 import com.google.gerrit.client.rpc.GerritCallback;
 import com.google.gerrit.client.rpc.ScreenLoadCallback;
 import com.google.gerrit.client.ui.CommentPanel;
-import com.google.gerrit.client.ui.ComplexDisclosurePanel;
 import com.google.gerrit.client.ui.ExpandAllCommand;
 import com.google.gerrit.client.ui.LinkMenuBar;
 import com.google.gerrit.client.ui.NeedsSignInKeyCommand;
@@ -28,7 +27,6 @@ import com.google.gerrit.common.data.AccountInfo;
 import com.google.gerrit.common.data.AccountInfoCache;
 import com.google.gerrit.common.data.ChangeDetail;
 import com.google.gerrit.common.data.ChangeInfo;
-import com.google.gerrit.common.data.GitwebLink;
 import com.google.gerrit.common.data.ToggleStarRequest;
 import com.google.gerrit.reviewdb.Account;
 import com.google.gerrit.reviewdb.Change;
@@ -40,11 +38,9 @@ import com.google.gwt.event.dom.client.ClickHandler;
 import com.google.gwt.event.dom.client.KeyPressEvent;
 import com.google.gwt.event.shared.HandlerRegistration;
 import com.google.gwt.i18n.client.LocaleInfo;
-import com.google.gwt.user.client.ui.Anchor;
 import com.google.gwt.user.client.ui.DisclosurePanel;
 import com.google.gwt.user.client.ui.FlowPanel;
 import com.google.gwt.user.client.ui.Image;
-import com.google.gwt.user.client.ui.InlineLabel;
 import com.google.gwt.user.client.ui.Label;
 import com.google.gwt.user.client.ui.Panel;
 import com.google.gwtexpui.globalkey.client.GlobalKey;
@@ -61,7 +57,6 @@ public class ChangeScreen extends Screen {
 
   private Image starChange;
   private boolean starred;
-  private PatchSet.Id currentPatchSet;
   private ChangeDescriptionBlock descriptionBlock;
   private ApprovalTable approvals;
 
@@ -72,7 +67,7 @@ public class ChangeScreen extends Screen {
   private ChangeTable.Section dependsOn;
   private ChangeTable.Section neededBy;
 
-  private FlowPanel patchSetPanels;
+  private PatchSetsBlock patchSetsBlock;
 
   private Panel comments;
 
@@ -121,6 +116,7 @@ public class ChangeScreen extends Screen {
     super.registerKeys();
     regNavigation = GlobalKey.add(this, keysNavigation);
     regAction = GlobalKey.add(this, keysAction);
+    patchSetsBlock.setRegisterKeys(true);
   }
 
   public void refresh() {
@@ -150,6 +146,7 @@ public class ChangeScreen extends Screen {
     keysNavigation = new KeyCommandSet(Gerrit.C.sectionNavigation());
     keysAction = new KeyCommandSet(Gerrit.C.sectionActions());
     keysNavigation.add(new DashboardKeyCommand(0, 'u', Util.C.upToDashboard()));
+    keysNavigation.add(new ExpandCollapseDependencySectionKeyCommand(0, 'd', Util.C.expandCollapseDependencies()));
 
     if (Gerrit.isSignedIn()) {
       keysAction.add(new StarKeyCommand(0, 's', Util.C.changeTableStar()));
@@ -195,8 +192,8 @@ public class ChangeScreen extends Screen {
     dependenciesPanel.setWidth("95%");
     add(dependenciesPanel);
 
-    patchSetPanels = new FlowPanel();
-    add(patchSetPanels);
+    patchSetsBlock = new PatchSetsBlock(this);
+    add(patchSetsBlock);
 
     comments = new FlowPanel();
     comments.setStyleName(Gerrit.RESOURCES.css().changeComments());
@@ -245,7 +242,7 @@ public class ChangeScreen extends Screen {
     approvals.display(detail.getChange(), detail.getMissingApprovals(), detail
         .getApprovals());
 
-    addPatchSets(detail);
+    patchSetsBlock.display(detail);
     addComments(detail);
 
     // If any dependency change is still open, show our dependency list.
@@ -264,40 +261,6 @@ public class ChangeScreen extends Screen {
     dependenciesPanel.setOpen(depsOpen);
   }
 
-  private void addPatchSets(final ChangeDetail detail) {
-    patchSetPanels.clear();
-
-    final PatchSet currps = detail.getCurrentPatchSet();
-    final GitwebLink gw = Gerrit.getConfig().getGitwebLink();
-    for (final PatchSet ps : detail.getPatchSets()) {
-      final ComplexDisclosurePanel panel =
-          new ComplexDisclosurePanel(Util.M.patchSetHeader(ps.getPatchSetId()),
-              ps == currps);
-      final PatchSetPanel psp = new PatchSetPanel(this, detail, ps);
-      panel.setContent(psp);
-
-      final InlineLabel revtxt = new InlineLabel(ps.getRevision().get() + " ");
-      revtxt.addStyleName(Gerrit.RESOURCES.css().patchSetRevision());
-      panel.getHeader().add(revtxt);
-      if (gw != null) {
-        final Anchor revlink =
-            new Anchor("(gitweb)", false, gw.toRevision(detail.getChange()
-                .getProject(), ps));
-        revlink.addStyleName(Gerrit.RESOURCES.css().patchSetLink());
-        panel.getHeader().add(revlink);
-      }
-
-      if (ps == currps) {
-        psp.ensureLoaded(detail.getCurrentPatchSetDetail());
-      } else {
-        panel.addOpenHandler(psp);
-      }
-      add(panel);
-      patchSetPanels.add(panel);
-    }
-    currentPatchSet = currps.getId();
-  }
-
   private void addComments(final ChangeDetail detail) {
     comments.clear();
 
@@ -398,6 +361,17 @@ public class ChangeScreen extends Screen {
     }
   }
 
+  public class ExpandCollapseDependencySectionKeyCommand extends KeyCommand {
+    public ExpandCollapseDependencySectionKeyCommand(int mask, char key, String help) {
+      super(mask, key, help);
+    }
+
+    @Override
+    public void onKeyPress(KeyPressEvent event) {
+      dependenciesPanel.setOpen(!dependenciesPanel.isOpen());
+    }
+  }
+
   public class StarKeyCommand extends NeedsSignInKeyCommand {
     public StarKeyCommand(int mask, char key, String help) {
       super(mask, key, help);
@@ -416,8 +390,9 @@ public class ChangeScreen extends Screen {
 
     @Override
     public void onKeyPress(final KeyPressEvent event) {
-      Gerrit.display("change,publish," + currentPatchSet.toString(),
-          new PublishCommentScreen(currentPatchSet));
+      PatchSet.Id currentPatchSetId = patchSetsBlock.getCurrentPatchSet().getId();
+      Gerrit.display("change,publish," + currentPatchSetId.toString(),
+          new PublishCommentScreen(currentPatchSetId));
     }
   }
 }

gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/PatchSetComplexDisclosurePanel.java

@@ -18,10 +18,12 @@ import com.google.gerrit.client.FormatUtil;
 import com.google.gerrit.client.Gerrit;
 import com.google.gerrit.client.rpc.GerritCallback;
 import com.google.gerrit.client.ui.AccountDashboardLink;
+import com.google.gerrit.client.ui.ComplexDisclosurePanel;
 import com.google.gerrit.client.ui.PatchLink;
 import com.google.gerrit.client.ui.PatchLink.SideBySide;
 import com.google.gerrit.client.ui.PatchLink.Unified;
 import com.google.gerrit.common.data.ChangeDetail;
+import com.google.gerrit.common.data.GitwebLink;
 import com.google.gerrit.common.data.PatchSetDetail;
 import com.google.gerrit.reviewdb.Account;
 import com.google.gerrit.reviewdb.AccountGeneralPreferences;
@@ -43,8 +45,8 @@ import com.google.gwt.event.logical.shared.OpenEvent;
 import com.google.gwt.event.logical.shared.OpenHandler;
 import com.google.gwt.user.client.Window;
 import com.google.gwt.user.client.rpc.AsyncCallback;
+import com.google.gwt.user.client.ui.Anchor;
 import com.google.gwt.user.client.ui.Button;
-import com.google.gwt.user.client.ui.Composite;
 import com.google.gwt.user.client.ui.DisclosurePanel;
 import com.google.gwt.user.client.ui.FlowPanel;
 import com.google.gwt.user.client.ui.Grid;
@@ -54,9 +56,10 @@ import com.google.gwt.user.client.ui.HTMLTable.CellFormatter;
 import com.google.gwtexpui.clippy.client.CopyableLabel;
 
 import java.util.Collections;
+import java.util.HashSet;
 import java.util.Set;
 
-class PatchSetPanel extends Composite implements OpenHandler<DisclosurePanel> {
+class PatchSetComplexDisclosurePanel extends ComplexDisclosurePanel implements OpenHandler<DisclosurePanel> {
   private static final int R_AUTHOR = 0;
   private static final int R_COMMITTER = 1;
   private static final int R_DOWNLOAD = 2;
@@ -70,14 +73,48 @@ class PatchSetPanel extends Composite implements OpenHandler<DisclosurePanel> {
   private Grid infoTable;
   private Panel actionsPanel;
   private PatchTable patchTable;
+  private final Set<ClickHandler> registeredClickHandler =  new HashSet<ClickHandler>();
 
-  PatchSetPanel(final ChangeScreen parent, final ChangeDetail detail,
+  /**
+   * Creates a closed complex disclosure panel for a patch set.
+   * The patch set details are loaded when the complex disclosure panel is opened.
+   */
+  PatchSetComplexDisclosurePanel(final ChangeScreen parent, final ChangeDetail detail,
       final PatchSet ps) {
+    this(parent, detail, ps, false);
+    addOpenHandler(this);
+  }
+
+  /**
+   * Creates an open complex disclosure panel for a patch set.
+   */
+  PatchSetComplexDisclosurePanel(final ChangeScreen parent, final ChangeDetail detail,
+      final PatchSetDetail psd) {
+    this(parent, detail, psd.getPatchSet(), true);
+    ensureLoaded(psd);
+  }
+
+  private PatchSetComplexDisclosurePanel(final ChangeScreen parent, final ChangeDetail detail,
+      final PatchSet ps, boolean isOpen) {
+    super(Util.M.patchSetHeader(ps.getPatchSetId()), isOpen);
     changeScreen = parent;
     changeDetail = detail;
     patchSet = ps;
     body = new FlowPanel();
-    initWidget(body);
+    setContent(body);
+
+    final GitwebLink gw = Gerrit.getConfig().getGitwebLink();
+
+    final InlineLabel revtxt = new InlineLabel(ps.getRevision().get() + " ");
+    revtxt.addStyleName(Gerrit.RESOURCES.css().patchSetRevision());
+    getHeader().add(revtxt);
+    if (gw != null) {
+      final Anchor revlink =
+          new Anchor("(gitweb)", false, gw.toRevision(detail.getChange()
+              .getProject(), ps));
+      revlink.addStyleName(Gerrit.RESOURCES.css().patchSetLink());
+      getHeader().add(revlink);
+    }
   }
 
   /**
@@ -124,6 +161,10 @@ class PatchSetPanel extends Composite implements OpenHandler<DisclosurePanel> {
       }
     }
     body.add(patchTable);
+
+    for(ClickHandler clickHandler : registeredClickHandler) {
+      patchTable.addClickHandler(clickHandler);
+    }
   }
 
   private void displayDownload() {
@@ -415,6 +456,7 @@ class PatchSetPanel extends Composite implements OpenHandler<DisclosurePanel> {
           new GerritCallback<PatchSetDetail>() {
             public void onSuccess(final PatchSetDetail result) {
               ensureLoaded(result);
+              patchTable.setRegisterKeys(true);
             }
           });
     }
@@ -447,4 +489,27 @@ class PatchSetPanel extends Composite implements OpenHandler<DisclosurePanel> {
     }
     changeScreen.display(result);
   }
+
+  public PatchSet getPatchSet() {
+    return patchSet;
+  }
+
+  /**
+   * Adds a click handler to the patch table.
+   * If the patch table is not yet initialized it is guaranteed that the click handler
+   * is added to the patch table after initialization.
+   */
+  public void addClickHandler(final ClickHandler clickHandler) {
+    registeredClickHandler.add(clickHandler);
+    if (patchTable != null) {
+      patchTable.addClickHandler(clickHandler);
+    }
+  }
+
+  /** Activates / Deactivates the key navigation and the highlighting of the current row for the patch table */
+  public void setActive(boolean active) {
+    if (patchTable != null) {
+      patchTable.setActive(active);
+    }
+  }
 }

gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/PatchSetsBlock.java

@@ -0,0 +1,218 @@
+// Copyright (C) 2010 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.client.changes;
+
+import com.google.gerrit.client.Gerrit;
+import com.google.gerrit.common.data.ChangeDetail;
+import com.google.gerrit.reviewdb.PatchSet;
+import com.google.gwt.event.dom.client.ClickEvent;
+import com.google.gwt.event.dom.client.ClickHandler;
+import com.google.gwt.event.dom.client.KeyPressEvent;
+import com.google.gwt.event.logical.shared.OpenEvent;
+import com.google.gwt.event.logical.shared.OpenHandler;
+import com.google.gwt.event.shared.HandlerRegistration;
+import com.google.gwt.user.client.ui.Composite;
+import com.google.gwt.user.client.ui.DisclosurePanel;
+import com.google.gwt.user.client.ui.FlowPanel;
+import com.google.gwtexpui.globalkey.client.GlobalKey;
+import com.google.gwtexpui.globalkey.client.KeyCommand;
+import com.google.gwtexpui.globalkey.client.KeyCommandSet;
+
+import java.util.HashMap;
+
+/**
+ * Composite that displays the patch sets of a change. This composite ensures
+ * that keyboard navigation to each changed file in all patch sets is possible.
+ */
+public class PatchSetsBlock extends Composite {
+
+  private final HashMap<Integer, PatchSetComplexDisclosurePanel> patchSetPanels =
+      new HashMap<Integer, PatchSetComplexDisclosurePanel>();
+
+  private final ChangeScreen parent;
+  private final FlowPanel body;
+  private HandlerRegistration regNavigation;
+
+  /**
+   * the patch set id of the patch set for which is the keyboard navigation is
+   * currently enabled
+   */
+  private int activePatchSetId = -1;
+
+  /** the patch set id of the current (latest) patch set */
+  private int currentPatchSetId = -1;
+
+  PatchSetsBlock(final ChangeScreen parent) {
+    this.parent = parent;
+    body = new FlowPanel();
+    initWidget(body);
+  }
+
+  /** Adds UI elements for each patch set of the given change to this composite. */
+  public void display(final ChangeDetail detail) {
+    clear();
+
+    final PatchSet currps = detail.getCurrentPatchSet();
+    currentPatchSetId = currps.getPatchSetId();
+    for (final PatchSet ps : detail.getPatchSets()) {
+      if (ps == currps) {
+        add(new PatchSetComplexDisclosurePanel(parent, detail, detail
+            .getCurrentPatchSetDetail()));
+      } else {
+        add(new PatchSetComplexDisclosurePanel(parent, detail, ps));
+      }
+    }
+  }
+
+  private void clear() {
+    body.clear();
+    patchSetPanels.clear();
+    setRegisterKeys(false);
+  }
+
+  /**
+   * Adds the given patch set panel to this composite and ensures that handler
+   * to activate / deactivate keyboard navigation for the patch set panel are
+   * registered.
+   */
+  private void add(final PatchSetComplexDisclosurePanel patchSetPanel) {
+    body.add(patchSetPanel);
+
+    int patchSetId = patchSetPanel.getPatchSet().getPatchSetId();
+    ActivationHandler activationHandler = new ActivationHandler(patchSetId);
+    patchSetPanel.addOpenHandler(activationHandler);
+    patchSetPanel.addClickHandler(activationHandler);
+    patchSetPanels.put(patchSetId, patchSetPanel);
+  }
+
+  public void setRegisterKeys(final boolean on) {
+    if (on) {
+      KeyCommandSet keysNavigation =
+          new KeyCommandSet(Gerrit.C.sectionNavigation());
+      keysNavigation.add(new PreviousPatchSetKeyCommand(0, 'p', Util.C
+          .previousPatchSet()));
+      keysNavigation.add(new NextPatchSetKeyCommand(0, 'n', Util.C
+          .nextPatchSet()));
+      regNavigation = GlobalKey.add(this, keysNavigation);
+      if (activePatchSetId != -1) {
+        activate(activePatchSetId);
+      } else {
+        activate(currentPatchSetId);
+      }
+    } else {
+      if (regNavigation != null) {
+        regNavigation.removeHandler();
+        regNavigation = null;
+      }
+      deactivate();
+    }
+  }
+
+  @Override
+  protected void onUnload() {
+    setRegisterKeys(false);
+    super.onUnload();
+  }
+
+  /**
+   * Activates keyboard navigation for the patch set panel that displays the
+   * patch set with the given patch set id.
+   * The keyboard navigation for the previously active patch set panel is
+   * automatically deactivated.
+   * This method also ensures that the current row is only highlighted in the
+   * table of the active patch set panel.
+   */
+  private void activate(final int patchSetId) {
+    if (activePatchSetId != patchSetId) {
+      deactivate();
+      PatchSetComplexDisclosurePanel patchSetPanel =
+          patchSetPanels.get(patchSetId);
+      patchSetPanel.setOpen(true);
+      patchSetPanel.setActive(true);
+      activePatchSetId = patchSetId;
+    }
+  }
+
+  /** Deactivates the keyboard navigation for the currently active patch set panel. */
+  private void deactivate() {
+    if (activePatchSetId != -1) {
+      PatchSetComplexDisclosurePanel patchSetPanel =
+          patchSetPanels.get(activePatchSetId);
+      patchSetPanel.setActive(false);
+      activePatchSetId = -1;
+    }
+  }
+
+  public PatchSet getCurrentPatchSet() {
+    PatchSetComplexDisclosurePanel patchSetPanel =
+        patchSetPanels.get(currentPatchSetId);
+    if (patchSetPanel != null) {
+      return patchSetPanel.getPatchSet();
+    } else {
+      return null;
+    }
+  }
+
+  private class ActivationHandler implements OpenHandler<DisclosurePanel>,
+      ClickHandler {
+
+    private final int patchSetId;
+
+    ActivationHandler(int patchSetId) {
+      this.patchSetId = patchSetId;
+    }
+
+    @Override
+    public void onOpen(OpenEvent<DisclosurePanel> event) {
+      // when a patch set panel is opened by the user
+      // it should automatically become active
+      activate(patchSetId);
+    }
+
+    @Override
+    public void onClick(ClickEvent event) {
+      // when a user clicks on a patch table the corresponding
+      // patch set panel should automatically become active
+      activate(patchSetId);
+    }
+
+  }
+
+  public class PreviousPatchSetKeyCommand extends KeyCommand {
+    public PreviousPatchSetKeyCommand(int mask, char key, String help) {
+      super(mask, key, help);
+    }
+
+    @Override
+    public void onKeyPress(final KeyPressEvent event) {
+      if (activePatchSetId > 1) {
+        activate(activePatchSetId - 1);
+      }
+    }
+  }
+
+  public class NextPatchSetKeyCommand extends KeyCommand {
+    public NextPatchSetKeyCommand(int mask, char key, String help) {
+      super(mask, key, help);
+    }
+
+    @Override
+    public void onKeyPress(final KeyPressEvent event) {
+      if (activePatchSetId > 0 && activePatchSetId < body.getWidgetCount()) {
+        activate(activePatchSetId + 1);
+      }
+    }
+  }
+}

gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/PatchTable.java

@@ -26,6 +26,7 @@ import com.google.gwt.core.client.GWT;
 import com.google.gwt.event.dom.client.ClickEvent;
 import com.google.gwt.event.dom.client.ClickHandler;
 import com.google.gwt.event.dom.client.KeyCodes;
+import com.google.gwt.event.dom.client.KeyPressEvent;
 import com.google.gwt.user.client.Command;
 import com.google.gwt.user.client.DeferredCommand;
 import com.google.gwt.user.client.IncrementalCommand;
@@ -35,6 +36,7 @@ import com.google.gwt.user.client.ui.Image;
 import com.google.gwt.user.client.ui.Label;
 import com.google.gwt.user.client.ui.Widget;
 import com.google.gwt.user.client.ui.HTMLTable.Cell;
+import com.google.gwtexpui.globalkey.client.KeyCommand;
 import com.google.gwtexpui.progress.client.ProgressBar;
 import com.google.gwtexpui.safehtml.client.SafeHtml;
 import com.google.gwtexpui.safehtml.client.SafeHtmlBuilder;
@@ -85,6 +87,11 @@ public class PatchTable extends Composite {
     }
   }
 
+  public void addClickHandler(final ClickHandler clickHandler) {
+    myTable.addClickHandler(clickHandler);
+
+  }
+
   public void setRegisterKeys(final boolean on) {
     myTable.setRegisterKeys(on);
   }
@@ -93,6 +100,10 @@ public class PatchTable extends Composite {
     myTable.movePointerTo(k);
   }
 
+  public void setActive(boolean active) {
+    myTable.setActive(active);
+  }
+
   public void notifyDraftDelta(final Patch.Key k, final int delta) {
     if (myTable != null) {
       myTable.notifyDraftDelta(k, delta);
@@ -170,13 +181,16 @@ public class PatchTable extends Composite {
     private static final int C_PATH = 2;
     private static final int C_DRAFT = 3;
     private static final int C_SIDEBYSIDE = 4;
+    private int activeRow = -1;
 
     MyTable() {
       keysNavigation.add(new PrevKeyCommand(0, 'k', Util.C.patchTablePrev()));
       keysNavigation.add(new NextKeyCommand(0, 'j', Util.C.patchTableNext()));
-      keysNavigation.add(new OpenKeyCommand(0, 'o', Util.C.patchTableOpen()));
+      keysNavigation.add(new OpenKeyCommand(0, 'o', Util.C.patchTableOpenDiff()));
       keysNavigation.add(new OpenKeyCommand(0, KeyCodes.KEY_ENTER, Util.C
-          .patchTableOpen()));
+          .patchTableOpenDiff()));
+      keysNavigation.add(new OpenUnifiedDiffKeyCommand(0, 'O', Util.C
+          .patchTableOpenUnifiedDiff()));
 
       table.addClickHandler(new ClickHandler() {
         @Override
@@ -190,6 +204,10 @@ public class PatchTable extends Composite {
       setSavePointerId(PatchTable.this.savePointerId);
     }
 
+    public void addClickHandler(final ClickHandler clickHandler) {
+      table.addClickHandler(clickHandler);
+    }
+
     void updateReviewedStatus(final Patch.Key patchKey, boolean reviewed) {
       final int row = findRow(patchKey);
       if (0 <= row) {
@@ -234,6 +252,22 @@ public class PatchTable extends Composite {
       super.movePointerTo(oldId);
     }
 
+    /** Activates / Deactivates the key navigation and the highlighting of the current row for this table */
+    public void setActive(boolean active) {
+      if (active) {
+        if(activeRow > 0 && getCurrentRow() != activeRow) {
+          super.movePointerTo(activeRow);
+          activeRow = -1;
+        }
+      } else {
+        if(getCurrentRow() > 0) {
+          activeRow = getCurrentRow();
+          super.movePointerTo(-1);
+        }
+      }
+      setRegisterKeys(active);
+    }
+
     void initializeRow(int row) {
       Patch patch = PatchTable.this.patchList.get(row - 1);
       setRowItem(row, patch);
@@ -462,6 +496,29 @@ public class PatchTable extends Composite {
         ((InlineHyperlink) link).go();
       }
     }
+
+    private final class OpenUnifiedDiffKeyCommand extends KeyCommand {
+
+      public OpenUnifiedDiffKeyCommand(int mask, char key, String help) {
+        super(mask, key, help);
+      }
+
+      @Override
+      public void onKeyPress(KeyPressEvent event) {
+        Widget link = table.getWidget(getCurrentRow(), C_PATH);
+        if (link instanceof FlowPanel) {
+          link = ((FlowPanel) link).getWidget(0);
+        }
+        if (link instanceof PatchLink.Unified) {
+          ((InlineHyperlink) link).go();
+        } else {
+          link = table.getWidget(getCurrentRow(), C_SIDEBYSIDE + 1);
+          if (link instanceof PatchLink.Unified) {
+            ((InlineHyperlink) link).go();
+          }
+        }
+      }
+    }
   }
 
   private final class DisplayCommand implements IncrementalCommand {

gerrit-gwtui/src/main/java/com/google/gerrit/client/ui/NavigationTable.java

@@ -148,6 +148,7 @@ public abstract class NavigationTable<RowItem> extends FancyFlexTable<RowItem> {
       }
     } else if (clear) {
       table.setWidget(currentRow, C_ARROW, null);
+      pointer.removeFromParent();
     }
     currentRow = newRow;
   }