LGTM

You can submit for M3, but I'm curious if this works in IE because blur
and focus don't bubble.  Also, add a TODO to add tests after M3.


http://gwt-code-reviews.appspot.com/758802/diff/1/3
File user/src/com/google/gwt/user/cellview/client/CellTree.java (right):

http://gwt-code-reviews.appspot.com/758802/diff/1/3#newcode544
user/src/com/google/gwt/user/cellview/client/CellTree.java:544:
Extra spaces.  Do an auto-format before submitting.

http://gwt-code-reviews.appspot.com/758802/diff/1/3#newcode548
user/src/com/google/gwt/user/cellview/client/CellTree.java:548: if
("blur".equals(eventType)) {
Are you always expecting to receive blur and focus?  If so, you need to
sink them through CellBasedWidgetImpl#sinkEvents.

http://gwt-code-reviews.appspot.com/758802/diff/1/3#newcode751
user/src/com/google/gwt/user/cellview/client/CellTree.java:751: private
boolean handleKey(Event event) {
JavaDoc the return value

http://gwt-code-reviews.appspot.com/758802/diff/1/3#newcode806
user/src/com/google/gwt/user/cellview/client/CellTree.java:806: case
KeyCodes.KEY_RIGHT:
I think left left should close (or move to parent if closed), right
should open (or move to first child is open), and enter should toggle.
Can you add a TODO to try it out on some people to see what is the most
naturally?

http://gwt-code-reviews.appspot.com/758802/diff/1/5
File user/src/com/google/gwt/user/cellview/client/CellTreeNodeView.java
(right):

http://gwt-code-reviews.appspot.com/758802/diff/1/5#newcode684
user/src/com/google/gwt/user/cellview/client/CellTreeNodeView.java:684:
extra spaces

http://gwt-code-reviews.appspot.com/758802/diff/1/5#newcode942
user/src/com/google/gwt/user/cellview/client/CellTreeNodeView.java:942:
for (int i = 0; i < index; i++) {
You can also just use:
ensureChildContainer().getChild(index).cast()

http://gwt-code-reviews.appspot.com/758802/diff/1/5#newcode956
user/src/com/google/gwt/user/cellview/client/CellTreeNodeView.java:956:
& ~(Event.ONBLUR | Event.ONFOCUS));
Can you add a comment explaining what this does?  It looks like its
unsinking ONBLUR and ONFOCUS.

http://gwt-code-reviews.appspot.com/758802/diff/1/5#newcode1013
user/src/com/google/gwt/user/cellview/client/CellTreeNodeView.java:1013:
Event.sinkEvents(child, Event.ONBLUR | Event.ONFOCUS);
blur and focus don't propagate in all browsers.  You'll need to so
something fancy.  Either use CellBasedWidgetImpl, or set the CellTree as
the event listener of this node (but make sure to unset it before
detaching CellTree).

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

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

Reply via email to