Revision: 8562
Author: jlaba...@google.com
Date: Wed Aug 18 06:22:52 2010
Log: Improving performance of CellTable. CellTable called StringBuilder#length() before and after rendering each cell to determine if we need to add a blank space to force the cell to render, but length() does an array join on the underlying array in IE, killing performance. Instead, we now put a DIV between the TD and the cell contents, which forces the cell to render. This patch also optimizes some Cells.

Review at http://gwt-code-reviews.appspot.com/775802

Review by: r...@google.com
http://code.google.com/p/google-web-toolkit/source/detail?r=8562

Modified:
 /trunk/user/src/com/google/gwt/cell/client/ActionCell.java
 /trunk/user/src/com/google/gwt/cell/client/CheckboxCell.java
 /trunk/user/src/com/google/gwt/cell/client/TextInputCell.java
 /trunk/user/src/com/google/gwt/user/cellview/client/CellTable.java

=======================================
--- /trunk/user/src/com/google/gwt/cell/client/ActionCell.java Tue Jul 27 08:47:38 2010 +++ /trunk/user/src/com/google/gwt/cell/client/ActionCell.java Wed Aug 18 06:22:52 2010
@@ -39,7 +39,7 @@
     void execute(T object);
   }

-  private final String message;
+  private final String html;
   private final Delegate<C> delegate;

   /**
@@ -50,8 +50,8 @@
    */
   public ActionCell(String message, Delegate<C> delegate) {
     super("click");
-    this.message = message;
     this.delegate = delegate;
+    this.html = "<button>" + message + "</button>";
   }

   @Override
@@ -64,6 +64,6 @@

   @Override
   public void render(C value, Object key, StringBuilder sb) {
-    sb.append("<button>" + message + "</button>");
+    sb.append(html);
   }
 }
=======================================
--- /trunk/user/src/com/google/gwt/cell/client/CheckboxCell.java Thu Aug 5 10:37:27 2010 +++ /trunk/user/src/com/google/gwt/cell/client/CheckboxCell.java Wed Aug 18 06:22:52 2010
@@ -21,8 +21,8 @@
 import com.google.gwt.event.dom.client.KeyCodes;

 /**
- * A {...@link Cell} used to render a checkbox.  The value of the checkbox
- * may be toggled using the ENTER key as well as via mouse click.
+ * A {...@link Cell} used to render a checkbox. The value of the checkbox may be
+ * toggled using the ENTER key as well as via mouse click.
  *
  * <p>
  * Note: This class is new and its interface subject to change.
@@ -30,6 +30,17 @@
  */
 public class CheckboxCell extends AbstractEditableCell<Boolean, Boolean> {

+  /**
+   * An html string representation of a checked input box.
+   */
+  private static final String INPUT_CHECKED =
+      "<input type=\"checkbox\" checked/>";
+
+  /**
+   * An html string representation of an unchecked input box.
+   */
+ private static final String INPUT_UNCHECKED = "<input type=\"checkbox\"/>";
+
   private final boolean isSelectBox;

   /**
@@ -63,19 +74,19 @@
   public void onBrowserEvent(Element parent, Boolean value, Object key,
       NativeEvent event, ValueUpdater<Boolean> valueUpdater) {
     String type = event.getType();
-
-    boolean enterPressed = "keyup".equals(type) &&
-        event.getKeyCode() == KeyCodes.KEY_ENTER;
+
+    boolean enterPressed = "keyup".equals(type)
+        && event.getKeyCode() == KeyCodes.KEY_ENTER;
     if ("change".equals(type) || enterPressed) {
       InputElement input = parent.getFirstChild().cast();
       Boolean isChecked = input.isChecked();
-
+
       // If the enter key was pressed, toggle the value
       if (enterPressed) {
         isChecked = !isChecked;
         input.setChecked(isChecked);
       }
-
+
       setViewData(key, isChecked);
       if (valueUpdater != null) {
         valueUpdater.update(isChecked);
@@ -92,13 +103,10 @@
       viewData = null;
     }

-    sb.append("<input type=\"checkbox\"");
-    if (value != null) {
-      boolean isChecked = (viewData != null) ? viewData : value;
-      if (isChecked) {
-        sb.append(" checked");
-      }
-    }
-    sb.append("/>");
+    if (value != null && ((viewData != null) ? viewData : value)) {
+      sb.append(INPUT_CHECKED);
+    } else {
+      sb.append(INPUT_UNCHECKED);
+    }
   }
 }
=======================================
--- /trunk/user/src/com/google/gwt/cell/client/TextInputCell.java Tue Jul 27 08:47:38 2010 +++ /trunk/user/src/com/google/gwt/cell/client/TextInputCell.java Wed Aug 18 06:22:52 2010
@@ -59,12 +59,11 @@
       viewData = null;
     }

-    sb.append("<input type='text'");
-    if (viewData != null) {
-      sb.append(" value='" + viewData + "'");
-    } else if (value != null) {
-      sb.append(" value='" + value + "'");
-    }
-    sb.append("></input>");
+ String s = (viewData != null) ? viewData : (value != null ? value : null);
+    if (s != null) {
+ sb.append("<input type='text' value='").append(s).append("'></input>");
+    } else {
+      sb.append("<input type='text'></input>");
+    }
   }
 }
=======================================
--- /trunk/user/src/com/google/gwt/user/cellview/client/CellTable.java Tue Aug 10 07:37:40 2010 +++ /trunk/user/src/com/google/gwt/user/cellview/client/CellTable.java Wed Aug 18 06:22:52 2010
@@ -619,17 +619,6 @@
       fireEventToCell(event, eventType, tableCell, value, col, row);
     }
   }
-
-  /**
-   * Redraw the table using the existing data.
-   */
-  @Override
-  public void redraw() {
-    if (redrawScheduled) {
-      redrawCancelled = true;
-    }
-    super.redraw();
-  }

   public void redrawFooters() {
     createHeaders(true);
@@ -693,8 +682,12 @@
     createHeadersAndFooters();

     ProvidesKey<T> keyProvider = getKeyProvider();
-    String firstColumnStyle = style.firstColumn();
-    String lastColumnStyle = style.lastColumn();
+    String evenRowStyle = style.evenRow();
+    String oddRowStyle = style.oddRow();
+    String cellStyle = style.cell();
+    String firstColumnStyle = " " + style.firstColumn();
+    String lastColumnStyle = " " + style.lastColumn();
+    String selectedRowStyle = " " + style.selectedRow();
     int columnCount = columns.size();
     int length = values.size();
     int end = start + length;
@@ -702,35 +695,27 @@
       T value = values.get(i - start);
       boolean isSelected = (selectionModel == null || value == null)
           ? false : selectionModel.isSelected(value);
-      sb.append("<tr onclick=''");
-      sb.append(" class='");
-      sb.append(i % 2 == 0 ? style.evenRow() : style.oddRow());
+      sb.append("<tr onclick='' class='");
+      sb.append(i % 2 == 0 ? evenRowStyle : oddRowStyle);
       if (isSelected) {
-        sb.append(" ").append(style.selectedRow());
+        sb.append(selectedRowStyle);
       }
       sb.append("'>");
       int curColumn = 0;
       for (Column<T, ?> column : columns) {
-        // TODO(jlabanca): How do we sink ONFOCUS and ONBLUR?
-        sb.append("<td class='").append(style.cell());
+        sb.append("<td class='").append(cellStyle);
         if (curColumn == 0) {
-          sb.append(" ").append(firstColumnStyle);
+          sb.append(firstColumnStyle);
         }
         // The first and last column could be the same column.
         if (curColumn == columnCount - 1) {
-          sb.append(" ").append(lastColumnStyle);
-        }
-        sb.append("'>");
-        int bufferLength = sb.length();
+          sb.append(lastColumnStyle);
+        }
+        sb.append("'><div>");
         if (value != null) {
           column.render(value, keyProvider, sb);
         }
-
-        // Add blank space to ensure empty rows aren't squished.
-        if (bufferLength == sb.length()) {
-          sb.append("&nbsp");
-        }
-        sb.append("</td>");
+        sb.append("</div></td>");
         curColumn++;
       }
       sb.append("</tr>");
@@ -773,6 +758,10 @@

   @Override
   void replaceAllChildren(List<T> values, String html) {
+    // Cancel any pending redraw.
+    if (redrawScheduled) {
+      redrawCancelled = true;
+    }
     TABLE_IMPL.replaceAllRows(
CellTable.this, tbody, CellBasedWidgetImpl.get().processHtml(html));
   }
@@ -884,6 +873,13 @@
     return colgroup.getChild(index).cast();
   }

+  /**
+ * Find the cell that contains the element. Note that the TD element is not
+   * the parent. The parent is the div inside the TD cell.
+   *
+   * @param elem the element
+   * @return the parent cell
+   */
   private TableCellElement findNearestParentCell(Element elem) {
     while ((elem != null) && (elem != table)) {
       // TODO: We need is() implementations in all Element subclasses.
@@ -897,6 +893,9 @@
     return null;
   }

+  /**
+ * Fire an event to the Cell within the specified {...@link TableCellElement}.
+   */
   @SuppressWarnings("unchecked")
   private <C> void fireEventToCell(Event event, String eventType,
       TableCellElement tableCell, T value, int col, int row) {
@@ -906,10 +905,11 @@
       C cellValue = column.getValue(value);
       ProvidesKey<T> providesKey = getKeyProvider();
       Object key = providesKey == null ? value : providesKey.getKey(value);
-      boolean cellWasEditing = cell.isEditing(tableCell, cellValue, key);
+      Element parentElem = tableCell.getFirstChildElement();
+      boolean cellWasEditing = cell.isEditing(parentElem, cellValue, key);
       column.onBrowserEvent(
-          tableCell, getPageStart() + row, value, event, providesKey);
-      cellIsEditing = cell.isEditing(tableCell, cellValue, key);
+          parentElem, getPageStart() + row, value, event, providesKey);
+      cellIsEditing = cell.isEditing(parentElem, cellValue, key);
       if (cellWasEditing && !cellIsEditing) {
         resetFocus();
       }

--
http://groups.google.com/group/Google-Web-Toolkit-Contributors

Reply via email to