commit bee7425dc93ab47fcd0f616b7f7c30363df609a7
author Shawn O. Pearce <sop@google.com> Tue May 19 11:30:31 2009 -0700
committer Shawn O. Pearce <sop@google.com> Tue May 19 11:30:31 2009 -0700
tree 9106ff7b768ae5a10e09901e7bcef040e8aacdfa
parent a95a6447b2935c021c2656d5db0c039593c7abdb

Fix file browser in patch that is taller than the window

If the patch screen is taller than the window the file browser popup
didn't position correctly because the movePointerTo method asked the
entire document window to scroll to the table row position.  That is
more often than not causing the document height to extend even longer,
and then the popup goes far below the bottom of the window.

Instead, we only want to scroll the ScrollPanel if the table happens
to be embedded inside of one.  This leaves the browser window alone,
and causes only the ScrollPanel to advance.

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

src/main/java/com/google/gerrit/client/patches/PatchBrowserPopup.java

diff --git a/src/main/java/com/google/gerrit/client/patches/PatchBrowserPopup.java b/src/main/java/com/google/gerrit/client/patches/PatchBrowserPopup.java
index 214eb9a..09ef9f5 100644
--- a/src/main/java/com/google/gerrit/client/patches/PatchBrowserPopup.java
+++ b/src/main/java/com/google/gerrit/client/patches/PatchBrowserPopup.java
@@ -90,15 +90,14 @@
 
     GlobalKey.dialog(this);
 
-    if (fileList.isLoaded()) {
-      installFileList();
-    } else {
+    if (!fileList.isLoaded()) {
       fileList.onTableLoaded(new Command() {
         @Override
         public void execute() {
           sp.setHeight("");
           setPosition(getOffsetWidth(), getOffsetHeight());
-          installFileList();
+          fileList.setRegisterKeys(true);
+          fileList.movePointerTo(callerKey);
         }
       });
     }
@@ -110,10 +109,9 @@
     }
     sp.setWidth((Window.getClientWidth() - 60) + "px");
     setPopupPositionAndShow(this);
-  }
-
-  private void installFileList() {
-    fileList.setRegisterKeys(true);
-    fileList.movePointerTo(callerKey);
+    if (fileList.isLoaded()) {
+      fileList.setRegisterKeys(true);
+      fileList.movePointerTo(callerKey);
+    }
   }
 }

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

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 724e1c7..c9da9bf 100644
--- a/src/main/java/com/google/gerrit/client/ui/NavigationTable.java
+++ b/src/main/java/com/google/gerrit/client/ui/NavigationTable.java
@@ -21,7 +21,9 @@
 import com.google.gwt.user.client.DOM;
 import com.google.gwt.user.client.Element;
 import com.google.gwt.user.client.ui.Image;
+import com.google.gwt.user.client.ui.ScrollPanel;
 import com.google.gwt.user.client.ui.UIObject;
+import com.google.gwt.user.client.ui.Widget;
 import com.google.gwt.user.client.ui.HTMLTable.CellFormatter;
 import com.google.gwtexpui.globalkey.client.GlobalKey;
 import com.google.gwtexpui.globalkey.client.KeyCommand;
@@ -48,6 +50,9 @@
   private int currentRow = -1;
   private String saveId;
 
+  private boolean computedScrollType;
+  private ScrollPanel parentScrollPanel;
+
   protected NavigationTable() {
     pointer = Gerrit.ICONS.arrowRight().createImage();
     keysNavigation = new KeyCommandSet(Gerrit.C.sectionNavigation());
@@ -138,7 +143,7 @@
       final Element tr = DOM.getParent(fmt.getElement(newRow, C_ARROW));
       UIObject.setStyleName(tr, S_ACTIVE_ROW, true);
       if (scroll) {
-        tr.scrollIntoView();
+        scrollIntoView(tr);
       }
     } else if (clear) {
       table.setWidget(currentRow, C_ARROW, null);
@@ -146,6 +151,31 @@
     currentRow = newRow;
   }
 
+  protected void scrollIntoView(final Element tr) {
+    if (!computedScrollType) {
+      parentScrollPanel = null;
+      Widget w = getParent();
+      while (w != null) {
+        if (w instanceof ScrollPanel) {
+          parentScrollPanel = (ScrollPanel) w;
+          break;
+        }
+        w = w.getParent();
+      }
+      computedScrollType = true;
+    }
+
+    if (parentScrollPanel != null) {
+      parentScrollPanel.ensureVisible(new UIObject() {
+        {
+          setElement(tr);
+        }
+      });
+    } else {
+      tr.scrollIntoView();
+    }
+  }
+
   protected void movePointerTo(final Object oldId) {
     final int row = findRow(oldId);
     if (0 <= row) {
@@ -200,6 +230,12 @@
   }
 
   @Override
+  protected void onLoad() {
+    computedScrollType = false;
+    parentScrollPanel = null;
+  }
+
+  @Override
   protected void onUnload() {
     setRegisterKeys(false);
 
@@ -210,6 +246,8 @@
       }
     }
 
+    computedScrollType = false;
+    parentScrollPanel = null;
     super.onUnload();
   }