Ray and I hashed out the names a bit. I don't mind shortening
CellTableHeaderCreator to HeaderCreator. I don't think it'll cause any
conflicts down the road.
@rjrjr - any objections to the shorter name?
http://gwt-code-reviews.appspot.com/1499808/diff/1/samples/showcase/src/com/google/gwt/sample/showcase/client/content/cell/CwCustomDataGrid.java
File
samples/showcase/src/com/google/gwt/sample/showcase/client/content/cell/CwCustomDataGrid.java
(right):
http://gwt-code-reviews.appspot.com/1499808/diff/1/samples/showcase/src/com/google/gwt/sample/showcase/client/content/cell/CwCustomDataGrid.java#newcode82
samples/showcase/src/com/google/gwt/sample/showcase/client/content/cell/CwCustomDataGrid.java:82:
public class CwCustomDataGrid extends ContentWidget {
On 2011/08/02 03:47:57, skybrian wrote:
This example doesn't really hang together. On the one hand it's
showing friends
and birthdays (like a social app or a contacts database) and on the
other it's
showing who's eligible for retirement benefits (like an HR app). It
seems like
we should pick one or the other? Perhaps take out "retirement
benefits" and
replace that with something that makes more sense for a contacts
database?
I removed the "retirement benefits" row completely. We only need one
example of a spanning row, so the "birthday this month" is fine.
I also wonder if we should simplify it or split into two or more
examples where
each demonstrates one thing. For someone reading documentation, 800
lines of
sample code seems really big and I'd much prefer to read a smaller
example.
This is a sample that is meant to illustrate what you can do with GWT.
We show source code for people who want to delve into it, but the
primary purpose of Showcase is to show off GWT, not to provide example
code.
http://gwt-code-reviews.appspot.com/1499808/diff/6001/user/src/com/google/gwt/user/cellview/client/AbstractCellTable.java
File user/src/com/google/gwt/user/cellview/client/AbstractCellTable.java
(right):
http://gwt-code-reviews.appspot.com/1499808/diff/6001/user/src/com/google/gwt/user/cellview/client/AbstractCellTable.java#newcode493
user/src/com/google/gwt/user/cellview/client/AbstractCellTable.java:493:
rawHtml = rawHtml.substring(7, rawHtml.length() - 8);
On 2011/08/02 03:47:57, skybrian wrote:
Perhaps add: assert tag.size() == 5
Done.
http://gwt-code-reviews.appspot.com/1499808/diff/6001/user/src/com/google/gwt/user/cellview/client/CellTableHeaderBuilder.java
File
user/src/com/google/gwt/user/cellview/client/CellTableHeaderBuilder.java
(right):
http://gwt-code-reviews.appspot.com/1499808/diff/6001/user/src/com/google/gwt/user/cellview/client/CellTableHeaderBuilder.java#newcode32
user/src/com/google/gwt/user/cellview/client/CellTableHeaderBuilder.java:32:
public interface CellTableHeaderBuilder<T> {
On 2011/08/02 03:47:57, skybrian wrote:
How about just HeaderBuilder?
Its specific to CellTable/DataGrid. Other CellWidgets might have a
different header builder. Ray and I came up with
CellTableHeaderCreator.
http://gwt-code-reviews.appspot.com/1499808/diff/6001/user/src/com/google/gwt/user/cellview/client/CellTableHeaderBuilder.java#newcode35
user/src/com/google/gwt/user/cellview/client/CellTableHeaderBuilder.java:35:
* A utility for building the header.
On 2011/08/02 03:47:57, skybrian wrote:
"(or footer)"
Done.
http://gwt-code-reviews.appspot.com/1499808/diff/6001/user/src/com/google/gwt/user/cellview/client/CellTableHeaderBuilder.java#newcode44
user/src/com/google/gwt/user/cellview/client/CellTableHeaderBuilder.java:44:
public abstract class Utility<T> {
On 2011/08/02 03:47:57, skybrian wrote:
Hmm, why isn't this an interface?
It seems like "Callbacks" would be a better name, although it looks
like you're
using "Utility" elsewhere.
Ray and I spent some time hashing out names, and we settled on
CellTableHeaderCreator and Helper (instead of Utility). I don't like
Callback because it isn't used in response to an event or an action, its
just a helper class used to build the table.
http://gwt-code-reviews.appspot.com/1499808/diff/6001/user/src/com/google/gwt/user/cellview/client/CellTableHeaderBuilder.java#newcode47
user/src/com/google/gwt/user/cellview/client/CellTableHeaderBuilder.java:47:
* Associate a {@link Column} with the specified element. If a column is
On 2011/08/02 03:47:57, skybrian wrote:
"Enables column-specific event handling for the specified element."
Done.
http://gwt-code-reviews.appspot.com/1499808/diff/6001/user/src/com/google/gwt/user/cellview/client/CellTableHeaderBuilder.java#newcode52
user/src/com/google/gwt/user/cellview/client/CellTableHeaderBuilder.java:52:
* The element builder must be in a state where an attribute can be
added.
On 2011/08/02 03:47:57, skybrian wrote:
Could be moved to the "builder" parameter.
Also, we could say something about where to get the builder. "The
builder should
be a child element of a row returned by {@link #startRow}."
Done.
http://gwt-code-reviews.appspot.com/1499808/diff/6001/user/src/com/google/gwt/user/cellview/client/CellTableHeaderBuilder.java#newcode58
user/src/com/google/gwt/user/cellview/client/CellTableHeaderBuilder.java:58:
public abstract void associateColumn(ElementBuilderBase<?> builder,
Column<T, ?> column);
On 2011/08/02 03:47:57, skybrian wrote:
Perhaps this should be named enableColumnHandlers?
Done.
I like that better.
http://gwt-code-reviews.appspot.com/1499808/diff/6001/user/src/com/google/gwt/user/cellview/client/CellTableHeaderBuilder.java#newcode72
user/src/com/google/gwt/user/cellview/client/CellTableHeaderBuilder.java:72:
* @param builder the {@link ElementBuilderBase} to render into
On 2011/08/02 03:47:57, skybrian wrote:
Is this true?
"This will normally a cell in a row returned by startRow()."
Done.
http://gwt-code-reviews.appspot.com/1499808/diff/6001/user/src/com/google/gwt/user/cellview/client/CellTableHeaderBuilder.java#newcode75
user/src/com/google/gwt/user/cellview/client/CellTableHeaderBuilder.java:75:
public abstract <H> void renderHeader(ElementBuilderBase<?> builder,
Context context,
On 2011/08/02 03:47:57, skybrian wrote:
I don't know how this fits into your existing conventions, but perhaps
rename
"builder" to "out"? (For render methods I like to put the thing being
rendered
into first, and name it "out".)
Done.
http://gwt-code-reviews.appspot.com/1499808/diff/6001/user/src/com/google/gwt/user/cellview/client/CellTableHeaderBuilder.java#newcode79
user/src/com/google/gwt/user/cellview/client/CellTableHeaderBuilder.java:79:
* Add a row to the table.
On 2011/08/02 03:47:57, skybrian wrote:
Should this be something like:
"Adds a header (or footer) row to the table, below any rows previously
added."
Done.
http://gwt-code-reviews.appspot.com/1499808/diff/6001/user/src/com/google/gwt/user/cellview/client/DefaultCellTableBuilder.java
File
user/src/com/google/gwt/user/cellview/client/DefaultCellTableBuilder.java
(right):
http://gwt-code-reviews.appspot.com/1499808/diff/6001/user/src/com/google/gwt/user/cellview/client/DefaultCellTableBuilder.java#newcode114
user/src/com/google/gwt/user/cellview/client/DefaultCellTableBuilder.java:114:
// Builder the cell.
On 2011/08/02 03:47:57, skybrian wrote:
"Build"
Done.
http://gwt-code-reviews.appspot.com/1499808/diff/6001/user/src/com/google/gwt/user/cellview/client/DefaultCellTableHeaderBuilder.java
File
user/src/com/google/gwt/user/cellview/client/DefaultCellTableHeaderBuilder.java
(right):
http://gwt-code-reviews.appspot.com/1499808/diff/6001/user/src/com/google/gwt/user/cellview/client/DefaultCellTableHeaderBuilder.java#newcode214
user/src/com/google/gwt/user/cellview/client/DefaultCellTableHeaderBuilder.java:214:
* @param index the index
On 2011/08/02 03:47:57, skybrian wrote:
What is this an index into?
Done.
Specified that the index is the column index of the header
http://gwt-code-reviews.appspot.com/1499808/diff/6001/user/src/com/google/gwt/user/cellview/client/DefaultCellTableHeaderBuilder.java#newcode235
user/src/com/google/gwt/user/cellview/client/DefaultCellTableHeaderBuilder.java:235:
protected void renderHeader(Utility<T> utility, ElementBuilderBase<?>
builder, Header<?> header,
On 2011/08/02 03:47:57, skybrian wrote:
Could we make the builder the first argument?
Done.
http://gwt-code-reviews.appspot.com/1499808/
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
