Some comments:

 - NIT: DefaultColumnWidths#getColumnWidth implemnt could be simplified to a 
simple map.get(), given that Map#get returns null if an entry is not present 
for the given key.

 - at least when using fixed layout, cells should have overflow:hidden 
(maybe it's the case already, I haven't checked), preferably only made the 
default in CellTable.BasicStyle (and possibly pointed out in the javadoc for 
CellTable.Style.cellTableCell for those who won't use the DefaultStyle, and 
will then have to provide the appropriate 'overflow' value, where 
overflow:visible is probably not the expected behavior).

 - looking around for more doc on fixed layout (it's been a while that I 
haven't looked at it), I stumbled on that sentence in the MSDN: "If no width 
is specified for the table, it renders by default with width=100%." 
http://msdn.microsoft.com/en-us/library/ms531161(v=vs.85).aspx (not to 
mention what we all already know: "Setting the property to fixed 
significantly improves table rendering speed, particularly for longer 
tables") I don't know how other browsers behave when no width is specified 
for the table (you said width=0 ?)

 - scrolling would only be possible with fixed layout (at least the way you 
explain it with independent tables; but there seems to be –realy hackish!– 
alternatives: http://www.imaputz.com/cssStuff/bigFourVersion.html )

 - how about colgroups? (for styling: e.g. add a border around 2 columns) 
Those would be used when constructing the <col> for setting their widths. At 
a minimum, rename your proposed refreshColumnsWidth to refreshColumns, 
refreshColumnStyles or refreshColumnsMeta (including style names and widths 
on columns) to allow colgroup support to be added later.

Otherwise, sounds great, and I like Stephen's proposal about the overloaded 
setWidth instead of overloaded setColumnWidths. You didn't mention it but 
there might be a need for a getTableLayout (with enum FIXED vs. AUTO) or 
isFixedLayout or similar (if an enum is used, it should be possible to pass 
an enum in the overloaded setWidth, and an additional setTableLayout could 
still be useful, e.g. for use in UiBinder: e.g. <c:CellTable 
ui:field="table" tableLayout="FIXED" width="100%" />)

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

Reply via email to