http://gwt-code-reviews.appspot.com/390801/diff/1/3
File
bikeshed/src/com/google/gwt/bikeshed/cells/client/CompositeCell.java
(right):

http://gwt-code-reviews.appspot.com/390801/diff/1/3#newcode32
bikeshed/src/com/google/gwt/bikeshed/cells/client/CompositeCell.java:32:
public class CompositeCell<C, V> extends Cell<C, V> {
One thing we should be very clear about is that CompositeCell always
concatenates its child cells, and makes no guarantees about how they're
likely to lay out. IOW, if a child cell uses a block-level element,
they'll stack up. This might be reasonable, but we should make it clear.

http://gwt-code-reviews.appspot.com/390801/diff/1/3#newcode49
bikeshed/src/com/google/gwt/bikeshed/cells/client/CompositeCell.java:49:
public boolean consumesEvents() {
We've never been clear on whether these methods could return different
values at different times -- I'm leaning towards *not*, because that
would allow a CompositeCell to cache these results (there's no
particular reason to expect a container to cache this value on behalf of
its Cells, so it might be called very frequently).

http://gwt-code-reviews.appspot.com/390801/diff/1/3#newcode59
bikeshed/src/com/google/gwt/bikeshed/cells/client/CompositeCell.java:59:
public boolean dependsOnSelection() {
Ditto above about caching results.

http://gwt-code-reviews.appspot.com/390801/diff/1/5
File bikeshed/src/com/google/gwt/bikeshed/list/client/ListView.java
(right):

http://gwt-code-reviews.appspot.com/390801/diff/1/5#newcode59
bikeshed/src/com/google/gwt/bikeshed/list/client/ListView.java:59: void
setSelectionModel(SelectionModel<? super T> selectionModel);
It's debatable whether all lists should be required to implement
selection. We can go ahead and check it in this way, but I'd like to
think about it before fully committing to this idea.

http://gwt-code-reviews.appspot.com/390801/diff/1/7
File
bikeshed/src/com/google/gwt/bikeshed/list/client/SimpleCellList.java
(right):

http://gwt-code-reviews.appspot.com/390801/diff/1/7#newcode51
bikeshed/src/com/google/gwt/bikeshed/list/client/SimpleCellList.java:51:
private static final String STYLENNAME_SELECTED =
"gwt-sstree-selectedItem";
I assume that these classnames are temporary since this widget is being
used in the sstree. We can revisit the way we handle styling later, but
a TODO might be a good idea.

http://gwt-code-reviews.appspot.com/390801/diff/1/7#newcode122
bikeshed/src/com/google/gwt/bikeshed/list/client/SimpleCellList.java:122:
int type = event.getTypeInt();
When Dan gets done factoring out the "pager" concept, I suppose we can
eliminate all this built-in button stuff.

http://gwt-code-reviews.appspot.com/390801/diff/1/8
File
bikeshed/src/com/google/gwt/bikeshed/list/client/impl/SimpleCellListImpl.java
(right):

http://gwt-code-reviews.appspot.com/390801/diff/1/8#newcode42
bikeshed/src/com/google/gwt/bikeshed/list/client/impl/SimpleCellListImpl.java:42:
public abstract class SimpleCellListImpl<T> {
If this shouldn't be used, consider making it package protected if
possible.

http://gwt-code-reviews.appspot.com/390801/diff/1/8#newcode297
bikeshed/src/com/google/gwt/bikeshed/list/client/impl/SimpleCellListImpl.java:297:
+ "'><i>loading...</i></div>");
At some point we'll either have to make this string overridable or turn
it into something more i18n-friendly.

http://gwt-code-reviews.appspot.com/390801/diff/1/18
File bikeshed/src/com/google/gwt/bikeshed/tree/client/TreeView.java
(right):

http://gwt-code-reviews.appspot.com/390801/diff/1/18#newcode28
bikeshed/src/com/google/gwt/bikeshed/tree/client/TreeView.java:28:
public abstract class TreeView extends Composite {
Sure is starting to smell like this class can be nuked...

http://gwt-code-reviews.appspot.com/390801/diff/1/19
File bikeshed/src/com/google/gwt/bikeshed/tree/client/TreeViewModel.java
(right):

http://gwt-code-reviews.appspot.com/390801/diff/1/19#newcode34
bikeshed/src/com/google/gwt/bikeshed/tree/client/TreeViewModel.java:34:
class DefaultNodeInfo<T> implements NodeInfo<T> {
I really like the fact that much of the code is gone from this class. It
sure seemed like a symptom that the NodeInfo contract was too complex
before.

http://gwt-code-reviews.appspot.com/390801/show

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

Reply via email to