LGTM On Wed, Aug 17, 2011 at 7:32 AM, <[email protected]> wrote:
> A patch is coming that modifies the TableBuilder API to make it more > flexible. TableBuilder.Utility is going away completely, and the event > handling implementation is moved from AbstractCellTable into > AbstractTableBuilder. This will allow users to implement a simple grid > based TableBuilder with faster rendering and event handling logic. > > The new API is also much more builder-like, and simpler to boot.Once > those changes are in, I'll revisit HeaderCreator and apply similar > changes, hopefully eliminating the Helper class. Either way, there will > be follow-up changes that > unify the terminology used in TableBuilder/HeaderCreator. > > > > http://gwt-code-reviews.**appspot.com/1499808/diff/** > 14004/samples/showcase/src/**com/google/gwt/sample/** > showcase/client/content/cell/**CwCustomDataGrid.java<http://gwt-code-reviews.appspot.com/1499808/diff/14004/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/** > 14004/samples/showcase/src/**com/google/gwt/sample/** > showcase/client/content/cell/**CwCustomDataGrid.java#**newcode79<http://gwt-code-reviews.appspot.com/1499808/diff/14004/samples/showcase/src/com/google/gwt/sample/showcase/client/content/cell/CwCustomDataGrid.java#newcode79> > samples/showcase/src/com/**google/gwt/sample/showcase/** > client/content/cell/**CwCustomDataGrid.java:79: > * Example file. > On 2011/08/16 02:16:25, skybrian wrote: > >> "Defines a custom table that displays a contact in each row. >> This is an example that shows how to completely customize the >> > appearance of the > >> headers, data rows, and footers in a CellTable." >> > > Done. > > > http://gwt-code-reviews.**appspot.com/1499808/diff/** > 14004/samples/showcase/src/**com/google/gwt/sample/** > showcase/client/content/cell/**CwCustomDataGrid.java#**newcode143<http://gwt-code-reviews.appspot.com/1499808/diff/14004/samples/showcase/src/com/google/gwt/sample/showcase/client/content/cell/CwCustomDataGrid.java#newcode143> > samples/showcase/src/com/**google/gwt/sample/showcase/** > client/content/cell/**CwCustomDataGrid.java:143: > * A custom header builder. > On 2011/08/16 02:16:25, skybrian wrote: > >> "Renders custom headers that ..." >> > > Done. > > > http://gwt-code-reviews.**appspot.com/1499808/diff/** > 14004/samples/showcase/src/**com/google/gwt/sample/** > showcase/client/content/cell/**CwCustomDataGrid.java#**newcode164<http://gwt-code-reviews.appspot.com/1499808/diff/14004/samples/showcase/src/com/google/gwt/sample/showcase/client/content/cell/CwCustomDataGrid.java#newcode164> > samples/showcase/src/com/**google/gwt/sample/showcase/** > client/content/cell/**CwCustomDataGrid.java:164: > public void buildHeader(Helper<**ContactInfo> utility) { > On 2011/08/16 02:16:25, skybrian wrote: > >> s/utility/helper/ >> > > Done. > > > http://gwt-code-reviews.**appspot.com/1499808/diff/** > 14004/samples/showcase/src/**com/google/gwt/sample/** > showcase/client/content/cell/**CwCustomDataGrid.java#**newcode209<http://gwt-code-reviews.appspot.com/1499808/diff/14004/samples/showcase/src/com/google/gwt/sample/showcase/client/content/cell/CwCustomDataGrid.java#newcode209> > samples/showcase/src/com/**google/gwt/sample/showcase/** > client/content/cell/**CwCustomDataGrid.java:209: > * Build a single header. > On 2011/08/16 02:16:25, skybrian wrote: > >> "Renders the header of one column, with the given options." >> > > Done. > > > http://gwt-code-reviews.**appspot.com/1499808/diff/** > 14004/samples/showcase/src/**com/google/gwt/sample/** > showcase/client/content/cell/**CwCustomDataGrid.java#**newcode211<http://gwt-code-reviews.appspot.com/1499808/diff/14004/samples/showcase/src/com/google/gwt/sample/showcase/client/content/cell/CwCustomDataGrid.java#newcode211> > samples/showcase/src/com/**google/gwt/sample/showcase/** > client/content/cell/**CwCustomDataGrid.java:211: > * @param utility the utility used to builder the header > On 2011/08/16 02:16:25, skybrian wrote: > >> "used to build" >> > > Done. > > > http://gwt-code-reviews.**appspot.com/1499808/diff/** > 14004/samples/showcase/src/**com/google/gwt/sample/** > showcase/client/content/cell/**CwCustomDataGrid.java#**newcode213<http://gwt-code-reviews.appspot.com/1499808/diff/14004/samples/showcase/src/com/google/gwt/sample/showcase/client/content/cell/CwCustomDataGrid.java#newcode213> > samples/showcase/src/com/**google/gwt/sample/showcase/** > client/content/cell/**CwCustomDataGrid.java:213: > * @param header the header to buil > On 2011/08/16 02:16:25, skybrian wrote: > >> "the Header to render" >> > > Done. > > > http://gwt-code-reviews.**appspot.com/1499808/diff/** > 14004/samples/showcase/src/**com/google/gwt/sample/** > showcase/client/content/cell/**CwCustomDataGrid.java#**newcode220<http://gwt-code-reviews.appspot.com/1499808/diff/14004/samples/showcase/src/com/google/gwt/sample/showcase/client/content/cell/CwCustomDataGrid.java#newcode220> > samples/showcase/src/com/**google/gwt/sample/showcase/** > client/content/cell/**CwCustomDataGrid.java:220: > private void buildHeader(Helper<**ContactInfo> utility, TableRowBuilder > tr, Header<?> header, > On 2011/08/16 02:16:25, skybrian wrote: > >> rename utility -> helper >> rename tr -> out. >> Maybe reorder first arguments to match renderHeader() >> > > maybe rename to renderOneHeader()? >> > > Done. > > > http://gwt-code-reviews.**appspot.com/1499808/diff/** > 14004/samples/showcase/src/**com/google/gwt/sample/** > showcase/client/content/cell/**CwCustomDataGrid.java#**newcode295<http://gwt-code-reviews.appspot.com/1499808/diff/14004/samples/showcase/src/com/google/gwt/sample/showcase/client/content/cell/CwCustomDataGrid.java#newcode295> > samples/showcase/src/com/**google/gwt/sample/showcase/** > client/content/cell/**CwCustomDataGrid.java:295: > * A custom version of {@link CellTableBuilder}. > On 2011/08/16 02:16:25, skybrian wrote: > >> "Renders the data rows that display each contact in the table." >> > > Done. > > > http://gwt-code-reviews.**appspot.com/1499808/diff/** > 14004/samples/showcase/src/**com/google/gwt/sample/** > showcase/client/content/cell/**CwCustomDataGrid.java#**newcode489<http://gwt-code-reviews.appspot.com/1499808/diff/14004/samples/showcase/src/com/google/gwt/sample/showcase/client/content/cell/CwCustomDataGrid.java#newcode489> > samples/showcase/src/com/**google/gwt/sample/showcase/** > client/content/cell/**CwCustomDataGrid.java:489: > * A map containing the IDs of {@link ContactInfo} who's friends list is > On 2011/08/16 02:16:25, skybrian wrote: > >> "Contains the contact id for each row in the table where the friends >> > list is > >> currently expanded." >> > > Done. > > > http://gwt-code-reviews.**appspot.com/1499808/diff/** > 14004/samples/showcase/src/**com/google/gwt/sample/** > showcase/client/content/cell/**CwCustomDataGrid.java#**newcode636<http://gwt-code-reviews.appspot.com/1499808/diff/14004/samples/showcase/src/com/google/gwt/sample/showcase/client/content/cell/CwCustomDataGrid.java#newcode636> > samples/showcase/src/com/**google/gwt/sample/showcase/** > client/content/cell/**CwCustomDataGrid.java:636: > * Initialize the column. > On 2011/08/16 02:16:25, skybrian wrote: > >> "Defines the columns in the custom table. Maps the data in the >> > ContactInfo for > >> each row into the appropriate column in the table, and defines >> > handlers for each > >> column." >> > > Done. > > > http://gwt-code-reviews.**appspot.com/1499808/diff/** > 14004/user/src/com/google/gwt/**user/cellview/client/**HeaderCreator.java<http://gwt-code-reviews.appspot.com/1499808/diff/14004/user/src/com/google/gwt/user/cellview/client/HeaderCreator.java> > File user/src/com/google/gwt/user/**cellview/client/HeaderCreator.**java > (right): > > http://gwt-code-reviews.**appspot.com/1499808/diff/** > 14004/user/src/com/google/gwt/**user/cellview/client/** > HeaderCreator.java#newcode23<http://gwt-code-reviews.appspot.com/1499808/diff/14004/user/src/com/google/gwt/user/cellview/client/HeaderCreator.java#newcode23> > user/src/com/google/gwt/user/**cellview/client/HeaderCreator.**java:23: * > Creates the header or footer section of a CellTable. > On 2011/08/16 02:16:25, skybrian wrote: > >> "Creates the DOM elements for the header"... >> > > Done. > > > http://gwt-code-reviews.**appspot.com/1499808/diff/** > 14004/user/src/com/google/gwt/**user/cellview/client/** > HeaderCreator.java#newcode32<http://gwt-code-reviews.appspot.com/1499808/diff/14004/user/src/com/google/gwt/user/cellview/client/HeaderCreator.java#newcode32> > user/src/com/google/gwt/user/**cellview/client/HeaderCreator.**java:32: > public interface HeaderCreator<T> { > On 2011/08/16 02:16:25, skybrian wrote: > >> Hmm, it seems like it should be HeaderBuilder to be consistent with >> TableBuilder? >> > > On the other hand, it doesn't actually create Header objects. Perhaps >> "HeaderRenderer" and "RowRenderer" might make more sense. >> > > I'll rename TableBuilder to TableCreator in another CL. I just don't > want to bloat this CL, which is already big enough. > > > http://gwt-code-reviews.**appspot.com/1499808/diff/** > 14004/user/src/com/google/gwt/**user/cellview/client/** > HeaderCreator.java#newcode35<http://gwt-code-reviews.appspot.com/1499808/diff/14004/user/src/com/google/gwt/user/cellview/client/HeaderCreator.java#newcode35> > user/src/com/google/gwt/user/**cellview/client/HeaderCreator.**java:35: * > A > utility for building the header or footer. > On 2011/08/16 02:16:25, skybrian wrote: > >> "Contains methods that {@link #buildHeader} can call while building a >> > header or > >> footer." >> > > Done. > > > http://gwt-code-reviews.**appspot.com/1499808/diff/** > 14004/user/src/com/google/gwt/**user/cellview/client/** > HeaderCreator.java#newcode38<http://gwt-code-reviews.appspot.com/1499808/diff/14004/user/src/com/google/gwt/user/cellview/client/HeaderCreator.java#newcode38> > user/src/com/google/gwt/user/**cellview/client/HeaderCreator.**java:38: * > The cell table being rendered will define the Utility implementation and > On 2011/08/16 02:16:25, skybrian wrote: > >> s/Utility/Helper/ >> > > Done. > > > http://gwt-code-reviews.**appspot.com/1499808/diff/** > 14004/user/src/com/google/gwt/**user/cellview/client/** > HeaderCreator.java#newcode73<http://gwt-code-reviews.appspot.com/1499808/diff/14004/user/src/com/google/gwt/user/cellview/client/HeaderCreator.java#newcode73> > user/src/com/google/gwt/user/**cellview/client/HeaderCreator.**java:73: > public abstract void enabledColumnHandlers(**ElementBuilderBase<?> > builder, Column<T, ?> column); > On 2011/08/16 02:16:25, skybrian wrote: > >> s/enabled/enable/ >> > > Done. > > > http://gwt-code-reviews.**appspot.com/1499808/diff/** > 14004/user/src/com/google/gwt/**user/cellview/client/** > HeaderCreator.java#newcode78<http://gwt-code-reviews.appspot.com/1499808/diff/14004/user/src/com/google/gwt/user/cellview/client/HeaderCreator.java#newcode78> > user/src/com/google/gwt/user/**cellview/client/HeaderCreator.**java:78: > public AbstractCellTable<T> getCellTable() { > On 2011/08/16 02:16:25, skybrian wrote: > >> Could be just "getTable" >> > > Done. > > > http://gwt-code-reviews.**appspot.com/1499808/diff/** > 14004/user/src/com/google/gwt/**user/cellview/client/** > HeaderCreator.java#newcode83<http://gwt-code-reviews.appspot.com/1499808/diff/14004/user/src/com/google/gwt/user/cellview/client/HeaderCreator.java#newcode83> > user/src/com/google/gwt/user/**cellview/client/HeaderCreator.**java:83: * > Render a Header into the specified {@link ElementBuilderBase}. Use this > On 2011/08/16 02:16:25, skybrian wrote: > >> "Renders a given Header into a given ElementBuilderBase. This method >> > ensures > >> that the CellTable widget will handle events " ... >> > > Done. > > > http://gwt-code-reviews.**appspot.com/1499808/diff/** > 14004/user/src/com/google/gwt/**user/cellview/client/** > HeaderCreator.java#newcode90<http://gwt-code-reviews.appspot.com/1499808/diff/14004/user/src/com/google/gwt/user/cellview/client/HeaderCreator.java#newcode90> > user/src/com/google/gwt/user/**cellview/client/HeaderCreator.**java:90: * > and must be in a state where a attributes and html can be added. > On 2011/08/16 02:16:25, skybrian wrote: > >> "a state that allows both attributes and elements to be added." >> > > Done. > > > http://gwt-code-reviews.**appspot.com/1499808/diff/** > 14004/user/src/com/google/gwt/**user/cellview/client/** > HeaderCreator.java#newcode91<http://gwt-code-reviews.appspot.com/1499808/diff/14004/user/src/com/google/gwt/user/cellview/client/HeaderCreator.java#newcode91> > user/src/com/google/gwt/user/**cellview/client/HeaderCreator.**java:91: * > @param context the {@link Context} of the cell > On 2011/08/16 02:16:25, skybrian wrote: > >> which cell? >> > > Done. > > > http://gwt-code-reviews.**appspot.com/1499808/diff/** > 14004/user/src/com/google/gwt/**user/cellview/client/** > HeaderCreator.java#newcode116<http://gwt-code-reviews.appspot.com/1499808/diff/14004/user/src/com/google/gwt/user/cellview/client/HeaderCreator.java#newcode116> > user/src/com/google/gwt/user/**cellview/client/HeaderCreator.**java:116: > void buildHeader(Helper<T> utility); > On 2011/08/16 02:16:25, skybrian wrote: > >> s/utility/helper/ >> > > Maybe this should be named renderHeaders()? (I see in the example that >> > it > >> doesn't actually build the Header objects.) >> > > The API is still in flux. There is a pending change that makes > TableBuilder a true builder API and completely eliminates the > Helper/Utility class. We'll make similar changes to HeaderBuilder. > > If you can look the other way for a couple more changes, the API will > soon be improved based on real use. > > > http://gwt-code-reviews.**appspot.com/1499808/<http://gwt-code-reviews.appspot.com/1499808/> > -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
