Some general Grid maintenance (JavaDocs mostly) (#13334)

Change-Id: I6724ae2433b742b620d43cb564798d59be805545

client/src/com/vaadin/client/ui/grid/Cell.java

@@ -19,10 +19,12 @@ import com.google.gwt.dom.client.Element;
 
 /**
  * Describes a cell
- * 
- * TODO The description is still very vague since the content and naming of this
- * class is still under debate and the API is not final. Improve the description
- * when API has been finalized.
+ * <p>
+ * It's a representation of the element in a grid cell, and its row and column
+ * indices.
+ * <p>
+ * Unlike the {@link FlyweightRow}, an instance of {@link Cell} can be stored in
+ * a field.
  * 
  * @since
  * @author Vaadin Ltd
@@ -36,7 +38,7 @@ public class Cell {
     private final Element element;
 
     /**
-     * Constructs a new {@link Cell}
+     * Constructs a new {@link Cell}.
      * 
      * @param row
      *            The index of the row
@@ -53,7 +55,7 @@ public class Cell {
     }
 
     /**
-     * Returns the index of the row the cell resides on
+     * Returns the index of the row the cell resides in.
      * 
      * @return the row index
      * 
@@ -63,7 +65,7 @@ public class Cell {
     }
 
     /**
-     * Returns the index of the column the cell resides on
+     * Returns the index of the column the cell resides in.
      * 
      * @return the column index
      */
@@ -72,9 +74,9 @@ public class Cell {
     }
 
     /**
-     * Returns the element of the cell
+     * Returns the element of the cell.
      * 
-     * @return the element
+     * @return the cell element
      */
     public Element getElement() {
         return element;

client/src/com/vaadin/client/ui/grid/ColumnConfiguration.java

@@ -26,7 +26,7 @@ package com.vaadin.client.ui.grid;
 public interface ColumnConfiguration {
 
     /**
-     * Removes columns at a certain index.
+     * Removes columns at certain indices.
      * <p>
      * If any of the removed columns were frozen, the number of frozen columns
      * will be reduced by the number of the removed columns that were frozen.
@@ -34,11 +34,10 @@ public interface ColumnConfiguration {
      * @param index
      *            the index of the first column to be removed
      * @param numberOfColumns
-     *            the number of rows to remove, starting from the index
+     *            the number of rows to remove, starting from {@code index}
      * @throws IndexOutOfBoundsException
-     *             if any integer in the range
-     *             <code>[index..(index+numberOfColumns)]</code> is not an
-     *             existing column index.
+     *             if the entire range of removed columns is not currently
+     *             present in the escalator
      * @throws IllegalArgumentException
      *             if <code>numberOfColumns</code> is less than 1.
      */

client/src/com/vaadin/client/ui/grid/ColumnGroup.java

@@ -24,15 +24,15 @@ import java.util.List;
 import com.vaadin.client.ui.grid.renderers.TextRenderer;
 
 /**
- * Column groups are used to group columns together for adding common auxiliary
- * headers and footers. Columns groups are added to {@link ColumnGroupRow
- * ColumnGroupRows}.
+ * A column group is a horizontal grouping of columns in a header or footer.
+ * Columns groups are added to {@link ColumnGroupRow ColumnGroupRows}.
  * 
  * @param <T>
  *            The row type of the grid. The row type is the POJO type from where
  *            the data is retrieved into the column cells.
  * @since
  * @author Vaadin Ltd
+ * @see ColumnGroupRow#addGroup(ColumnGroup...)
  */
 public class ColumnGroup<T> {
 
@@ -81,7 +81,7 @@ public class ColumnGroup<T> {
     }
 
     /**
-     * Gets the header text.
+     * Gets the text shown in the header.
      * 
      * @return the header text
      */
@@ -92,61 +92,60 @@ public class ColumnGroup<T> {
     /**
      * Sets the text shown in the header.
      * 
-     * @param header
-     *            the header to set
+     * @param caption
+     *            the caption to set
      */
-    public void setHeaderCaption(String header) {
-        this.header = header;
+    public void setHeaderCaption(String caption) {
+        this.header = caption;
         grid.refreshHeader();
     }
 
     /**
      * Gets the text shown in the footer.
      * 
-     * @return the text in the footer
+     * @return the footer text
      */
     public String getFooterCaption() {
         return footer;
     }
 
     /**
-     * Sets the text displayed in the footer.
+     * Sets the text shown in the footer.
      * 
-     * @param footer
-     *            the footer to set
+     * @param caption
+     *            the caption to set
      */
-    public void setFooterCaption(String footer) {
-        this.footer = footer;
+    public void setFooterCaption(String caption) {
+        this.footer = caption;
         grid.refreshFooter();
     }
 
     /**
-     * Returns all column in this group. It includes the subgroups columns as
-     * well.
+     * Returns all columns in this group. This includes columns in all the
+     * sub-groups as well.
      * 
-     * @return unmodifiable list of columns
+     * @return an unmodifiable list of columns
      */
     public List<GridColumn<?, T>> getColumns() {
         return columns;
     }
 
     /**
-     * Returns the renderer used for rendering the header cells
+     * Returns the renderer for the header cells.
      * 
-     * @return a renderer that renders header cells
+     * @return the renderer for the header cells
      */
     public Renderer<String> getHeaderRenderer() {
         return headerRenderer;
     }
 
     /**
-     * Sets the renderer that renders header cells.
+     * Sets the renderer for the header cells.
      * 
      * @param renderer
-     *            The renderer to use for rendering header cells. Must not be
-     *            null.
+     *            a non-{@code null} renderer to use for the header cells.
      * @throws IllegalArgumentException
-     *             thrown when renderer is null
+     *             if {@code renderer} is {@code null}
      */
     public void setHeaderRenderer(Renderer<String> renderer) {
         if (renderer == null) {
@@ -157,9 +156,9 @@ public class ColumnGroup<T> {
     }
 
     /**
-     * Returns the renderer used for rendering the footer cells
+     * Returns the renderer for the footer cells.
      * 
-     * @return a renderer that renders footer cells
+     * @return the renderer for the footer cells
      */
     public Renderer<String> getFooterRenderer() {
         return footerRenderer;
@@ -169,10 +168,9 @@ public class ColumnGroup<T> {
      * Sets the renderer that renders footer cells.
      * 
      * @param renderer
-     *            The renderer to use for rendering footer cells. Must not be
-     *            null.
+     *            a non-{@code null} renderer for footer cells.
      * @throws IllegalArgumentException
-     *             thrown when renderer is null
+     *             if {@code renderer} is {@code null}
      */
     public void setFooterRenderer(Renderer<String> renderer) {
         if (renderer == null) {

client/src/com/vaadin/client/ui/grid/ColumnGroupRow.java

@@ -25,8 +25,8 @@ import java.util.List;
 import java.util.Set;
 
 /**
- * A column group row represents an auxiliary header or footer row added to the
- * grid. A column group row includes column groups that group columns together.
+ * A column group row represents an additional header or footer row in a
+ * {@link Grid}. A column group row contains {@link ColumnGroup ColumnGroups}.
  * 
  * @param <T>
  *            Row type
@@ -67,17 +67,28 @@ public class ColumnGroupRow<T> {
     }
 
     /**
-     * Add a new group to the row by using column instances.
+     * Adds a new group of columns to the column group row.
      * 
      * @param columns
-     *            The columns that should belong to the group
-     * @return a column group representing the collection of columns added to
-     *         the group.
+     *            The columns that should be added to the group
+     * @return the added columns as a column group
+     * @throws IllegalArgumentException
+     *             if any of {@code columns} already belongs to a group, or if
+     *             {@code columns} or any of its elements are {@code null}
      */
     public ColumnGroup<T> addGroup(GridColumn<?, T>... columns)
             throws IllegalArgumentException {
 
+        if (columns == null) {
+            throw new IllegalArgumentException("columns may not be null");
+        }
+
         for (GridColumn<?, T> column : columns) {
+            if (column == null) {
+                throw new IllegalArgumentException(
+                        "none of the given columns may be null");
+            }
+
             if (isColumnGrouped(column)) {
                 throw new IllegalArgumentException("Column "
                         + String.valueOf(column.getHeaderCaption())
@@ -141,20 +152,29 @@ public class ColumnGroupRow<T> {
     }
 
     /**
-     * Add a new group to the row by using other already greated groups
+     * Add a new group to the row by using other already created groups.
      * 
      * @param groups
-     *            The subgroups of the group.
-     * @return a column group representing the collection of columns added to
-     *         the group.
-     * 
+     *            the column groups to be further grouped together
+     * @return the added column groups as a new single group
+     * @throws IllegalArgumentException
+     *             if any column group already belongs to another group, or if
+     *             {@code groups} or any of its elements are null
      */
     public ColumnGroup<T> addGroup(ColumnGroup<T>... groups)
             throws IllegalArgumentException {
-        assert groups != null : "groups cannot be null";
+
+        if (groups == null) {
+            throw new IllegalArgumentException("groups may not be null");
+        }
 
         Set<GridColumn<?, T>> columns = new HashSet<GridColumn<?, T>>();
         for (ColumnGroup<T> group : groups) {
+            if (group == null) {
+                throw new IllegalArgumentException(
+                        "none of the given group may be null");
+            }
+
             columns.addAll(group.getColumns());
         }
 
@@ -169,39 +189,46 @@ public class ColumnGroupRow<T> {
 
     /**
      * Removes a group from the row.
+     * <p>
+     * <em>Note:</em> this removes only a group in the immediate hierarchy
+     * level, and does not search recursively.
      * 
      * @param group
      *            The group to remove
+     * @return {@code true} iff the group was successfully removed
      */
-    public void removeGroup(ColumnGroup<T> group) {
-        groups.remove(group);
-        grid.refreshHeader();
-        grid.refreshFooter();
+    public boolean removeGroup(ColumnGroup<T> group) {
+        boolean removed = groups.remove(group);
+        if (removed) {
+            grid.refreshHeader();
+            grid.refreshFooter();
+        }
+        return removed;
     }
 
     /**
-     * Get the groups in the row
+     * Gets the groups in this row.
      * 
-     * @return unmodifiable list of groups in this row
+     * @return an unmodifiable list of groups in this row
      */
     public List<ColumnGroup<T>> getGroups() {
         return Collections.unmodifiableList(groups);
     }
 
     /**
-     * Is the header visible for the row.
+     * Checks whether the header is visible for the row group or not.
      * 
-     * @return <code>true</code> if header is visible
+     * @return <code>true</code> iff the header is visible
      */
     public boolean isHeaderVisible() {
         return headerVisible;
     }
 
     /**
-     * Sets the header visible for the row.
+     * Sets the visibility of the row group's header.
      * 
      * @param visible
-     *            should the header be shown
+     *            {@code true} iff the header should be shown
      */
     public void setHeaderVisible(boolean visible) {
         headerVisible = visible;
@@ -209,19 +236,19 @@ public class ColumnGroupRow<T> {
     }
 
     /**
-     * Is the footer visible for the row.
+     * Checks whether the footer is visible for the row or not.
      * 
-     * @return <code>true</code> if footer is visible
+     * @return <code>true</code> iff footer is visible
      */
     public boolean isFooterVisible() {
         return footerVisible;
     }
 
     /**
-     * Sets the footer visible for the row.
+     * Sets the visibility of the row group's footer.
      * 
      * @param visible
-     *            should the footer be shown
+     *            {@code true} iff the footer should be shown
      */
     public void setFooterVisible(boolean visible) {
         footerVisible = visible;
@@ -229,8 +256,8 @@ public class ColumnGroupRow<T> {
     }
 
     /**
-     * Iterates all the column groups and checks if the columns alread has been
-     * added to a group.
+     * Iterates through all the column groups and checks if the columns already
+     * has been added to a group.
      */
     private boolean isColumnGrouped(GridColumn<?, T> column) {
         for (ColumnGroup<T> group : groups) {

client/src/com/vaadin/client/ui/grid/Escalator.java

@@ -63,6 +63,8 @@ import com.vaadin.shared.util.SharedUtil;
 /*-
 
  Maintenance Notes! Reading these might save your day.
+ (note for editors: line width is 80 chars, including the
+ one-space indentation)
 
 
  == Row Container Structure
@@ -71,7 +73,7 @@ import com.vaadin.shared.util.SharedUtil;
  |-- AbstractStaticRowContainer
  | |-- HeaderRowContainer
  | `-- FooterContainer
- `-- BodyRowContainer
+ `---- BodyRowContainer
 
  AbstractRowContainer is intended to contain all common logic
  between RowContainers. It manages the bookkeeping of row
@@ -134,15 +136,23 @@ import com.vaadin.shared.util.SharedUtil;
  element's visual index via the field
  BodyRowContainer.visualRowOrder.
 
+ Currently, the physical and visual indices are kept in sync
+ _most of the time_ by a deferred rearrangement of rows.
+ They become desynced when scrolling. This is to help screen
+ readers to read the contents from the DOM in a natural
+ order. See BodyRowContainer.DeferredDomSorter for more
+ about that.
+
  */
 
 /**
  * A workaround-class for GWT and JSNI.
  * <p>
  * GWT is unable to handle some method calls to Java methods in inner-classes
- * from within JSNI blocks. Having that inner class implement a non-inner-class
- * (or interface), makes it possible for JSNI to indirectly refer to the inner
- * class, by invoking methods and fields in the non-inner-class.
+ * from within JSNI blocks. Having that inner class extend a non-inner-class (or
+ * implement such an interface), makes it possible for JSNI to indirectly refer
+ * to the inner class, by invoking methods and fields in the non-inner-class
+ * API.
  * 
  * @see Escalator.Scroller
  */
@@ -247,10 +257,6 @@ public class Escalator extends Widget {
      * often also be identified by searching for code reading the ROW_HEIGHT_PX
      * constant.
      */
-    /*
-     * [[API]]: Implementing this suggestion would require a change in the
-     * public API. These suggestions usually don't come lightly.
-     */
     /*
      * [[mpixscroll]]: This code will require alterations that are relevant for
      * supporting the scrolling through more pixels than some browsers normally
@@ -4070,7 +4076,7 @@ public class Escalator extends Widget {
     }
 
     /**
-     * Returns the representation of this Escalator header.
+     * Returns the row container for the header in this Escalator.
      * 
      * @return the header. Never <code>null</code>
      */
@@ -4079,7 +4085,7 @@ public class Escalator extends Widget {
     }
 
     /**
-     * Returns the representation of this Escalator body.
+     * Returns the row container for the body in this Escalator.
      * 
      * @return the body. Never <code>null</code>
      */
@@ -4088,7 +4094,7 @@ public class Escalator extends Widget {
     }
 
     /**
-     * Returns the representation of this Escalator footer.
+     * Returns the row container for the footer in this Escalator.
      * 
      * @return the footer. Never <code>null</code>
      */
@@ -4148,7 +4154,7 @@ public class Escalator extends Widget {
 
     /**
      * Returns the vertical scroll offset. Note that this is not necessarily the
-     * same as the scroll top in the DOM
+     * same as the {@code scrollTop} attribute in the DOM.
      * 
      * @return the logical vertical scroll offset
      */
@@ -4157,8 +4163,8 @@ public class Escalator extends Widget {
     }
 
     /**
-     * Sets the vertical scroll offset. Note that this is not necessarily the
-     * same as the scroll top in the DOM
+     * Sets the vertical scroll offset. Note that this will not necessarily
+     * become the same as the {@code scrollTop} attribute in the DOM.
      * 
      * @param scrollTop
      *            the number of pixels to scroll vertically
@@ -4169,7 +4175,7 @@ public class Escalator extends Widget {
 
     /**
      * Returns the logical horizontal scroll offset. Note that this is not
-     * necessarily the same as the scroll left in the DOM.
+     * necessarily the same as the {@code scrollLeft} attribute in the DOM.
      * 
      * @return the logical horizontal scroll offset
      */
@@ -4178,8 +4184,8 @@ public class Escalator extends Widget {
     }
 
     /**
-     * Sets the logical horizontal scroll offset. Note that this is not
-     * necessarily the same as the scroll left in the DOM.
+     * Sets the logical horizontal scroll offset. Note that will not necessarily
+     * become the same as the {@code scrollLeft} attribute in the DOM.
      * 
      * @param scrollLeft
      *            the number of pixels to scroll horizontally
@@ -4190,8 +4196,8 @@ public class Escalator extends Widget {
 
     /**
      * Scrolls the body horizontally so that the column at the given index is
-     * visible and there is at least {@code padding} pixels to the given scroll
-     * destination.
+     * visible and there is at least {@code padding} pixels in the direction of
+     * the given scroll destination.
      * 
      * @param columnIndex
      *            the index of the column to scroll to
@@ -4205,9 +4211,7 @@ public class Escalator extends Widget {
      *             column
      * @throws IllegalArgumentException
      *             if {@code destination} is {@link ScrollDestination#MIDDLE}
-     *             and padding is nonzero, because having a padding on a
-     *             centered column is undefined behavior, or if the column is
-     *             frozen
+     *             and padding is nonzero, or if the indicated column is frozen
      */
     public void scrollToColumn(final int columnIndex,
             final ScrollDestination destination, final int padding)
@@ -4251,8 +4255,7 @@ public class Escalator extends Widget {
      *             if {@code rowIndex} is not a valid index for an existing row
      * @throws IllegalArgumentException
      *             if {@code destination} is {@link ScrollDestination#MIDDLE}
-     *             and padding is nonzero, because having a padding on a
-     *             centered row is undefined behavior
+     *             and padding is nonzero
      */
     public void scrollToRow(final int rowIndex,
             final ScrollDestination destination, final int padding)
@@ -4348,7 +4351,7 @@ public class Escalator extends Widget {
 
     /**
      * Adds an event handler that gets notified when the range of visible rows
-     * changes e.g. because of scrolling.
+     * changes e.g. because of scrolling or row resizing.
      * 
      * @param rowVisibilityChangeHandler
      *            the event handler
@@ -4376,14 +4379,14 @@ public class Escalator extends Widget {
     }
 
     /**
-     * Gets the range of currently visible rows
+     * Gets the range of currently visible rows.
      * 
      * @return range of visible rows
      */
     public Range getVisibleRowRange() {
-        return Range.between(
+        return Range.withLength(
                 body.getLogicalRowIndex(body.visualRowOrder.getFirst()),
-                body.getLogicalRowIndex(body.visualRowOrder.getLast()) + 1);
+                body.visualRowOrder.size());
     }
 
     /**
@@ -4473,12 +4476,9 @@ public class Escalator extends Widget {
      * @param rows
      *            the number of rows that should be visible in Escalator's body
      * @throws IllegalArgumentException
-     *             if {@code rows} is zero or less
-     * @throws IllegalArgumentException
-     *             if {@code rows} is {@link Double#isInifinite(double)
-     *             infinite}
-     * @throws IllegalArgumentException
-     *             if {@code rows} is {@link Double#isNaN(double) NaN}.
+     *             if {@code rows} is &leq; 0,
+     *             {@link Double#isInifinite(double) infinite} or
+     *             {@link Double#isNaN(double) NaN}.
      * @see #setHeightMode(HeightMode)
      */
     public void setHeightByRows(double rows) throws IllegalArgumentException {