LGTM

A few suggestions and nits.


http://gwt-code-reviews.appspot.com/776804/diff/34001/35002
File
samples/showcase/src/com/google/gwt/sample/showcase/client/content/cell/CwCellValidation.java
(right):

http://gwt-code-reviews.appspot.com/776804/diff/34001/35002#newcode70
samples/showcase/src/com/google/gwt/sample/showcase/client/content/cell/CwCellValidation.java:70:

extra spaces

http://gwt-code-reviews.appspot.com/776804/diff/34001/35002#newcode113
samples/showcase/src/com/google/gwt/sample/showcase/client/content/cell/CwCellValidation.java:113:

extra spaces

http://gwt-code-reviews.appspot.com/776804/diff/34001/35005
File user/src/com/google/gwt/cell/client/ActionCell.java (right):

http://gwt-code-reviews.appspot.com/776804/diff/34001/35005#newcode3
user/src/com/google/gwt/cell/client/ActionCell.java:3: *
Auto formatter has been removing these extra spaces in JavaDoc.  Any
idea with this file adds them back in?  I think its an Eclipse 3.4
versus 3.5 thing.  I'm not sure which is "correct", but its annoying to
go back and forth.

There a bunch of files that add the spaces to JavaDoc.

http://gwt-code-reviews.appspot.com/776804/diff/34001/35006
File user/src/com/google/gwt/cell/client/ButtonCell.java (right):

http://gwt-code-reviews.appspot.com/776804/diff/34001/35006#newcode37
user/src/com/google/gwt/cell/client/ButtonCell.java:37:
extra spaces

http://gwt-code-reviews.appspot.com/776804/diff/34001/35009
File user/src/com/google/gwt/cell/client/ClickableTextCell.java (right):

http://gwt-code-reviews.appspot.com/776804/diff/34001/35009#newcode38
user/src/com/google/gwt/cell/client/ClickableTextCell.java:38:
extra spaces

http://gwt-code-reviews.appspot.com/776804/diff/34001/35009#newcode53
user/src/com/google/gwt/cell/client/ClickableTextCell.java:53: public
void render(SafeHtml value, Object key, SafeHtmlBuilder sb) {
protected

http://gwt-code-reviews.appspot.com/776804/diff/34001/35013
File user/src/com/google/gwt/cell/client/EditTextCell.java (right):

http://gwt-code-reviews.appspot.com/776804/diff/34001/35013#newcode43
user/src/com/google/gwt/cell/client/EditTextCell.java:43:
extra spaces

http://gwt-code-reviews.appspot.com/776804/diff/34001/35017
File user/src/com/google/gwt/cell/client/ImageResourceCell.java (right):

http://gwt-code-reviews.appspot.com/776804/diff/34001/35017#newcode30
user/src/com/google/gwt/cell/client/ImageResourceCell.java:30: // This
is not actually a constant, but we have no choice but to trust it
Add this note to the class JavaDoc as well.

This class assumes that the URL returned from ImageResource is safe from
script attacks.  If you do not generate the ImageResource from a
ClientBundle, you should use UriUtils to sanitize the URL before
returning it from ImageResource#getURL();

http://gwt-code-reviews.appspot.com/776804/diff/34001/35019
File user/src/com/google/gwt/cell/client/SafeHtmlCell.java (right):

http://gwt-code-reviews.appspot.com/776804/diff/34001/35019#newcode34
user/src/com/google/gwt/cell/client/SafeHtmlCell.java:34: public
abstract class SafeHtmlCell<C> extends AbstractCell<C> {
Rename to AbstractSafeHtmlCell

http://gwt-code-reviews.appspot.com/776804/diff/34001/35019#newcode69
user/src/com/google/gwt/cell/client/SafeHtmlCell.java:69: public
abstract void render(SafeHtml data, Object key, SafeHtmlBuilder sb);
I think this method should be protected since its intended to be called
only by render(C, Object, SafeHtmlBuilder)

http://gwt-code-reviews.appspot.com/776804/diff/34001/35037
File user/src/com/google/gwt/user/cellview/client/CellTreeNodeView.java
(right):

http://gwt-code-reviews.appspot.com/776804/diff/34001/35037#newcode66
user/src/com/google/gwt/user/cellview/client/CellTreeNodeView.java:66:
@Template("<div onclick=\"\" style=\"position:relative;{0}:{1}px;\"
class=\"{2}\">{3}<div class=\"{4}\">{5}</div></div>")
You can use padding-{0}, and then th direction parameter can just be
"right" or "left".  That makes it more obvious that the first value is
setting the padding, and its what you did in other classes.

http://gwt-code-reviews.appspot.com/776804/diff/34001/35037#newcode70
user/src/com/google/gwt/user/cellview/client/CellTreeNodeView.java:70:
@Template("<div><div style=\"{0}:{1}px;\"
class=\"{2}\">{3}</div></div>")
Ditto

http://gwt-code-reviews.appspot.com/776804/diff/34001/35037#newcode134
user/src/com/google/gwt/user/cellview/client/CellTreeNodeView.java:134:
? "padding-right" : "padding-left";
"right"/"left" instead of "padding-right"/"padding-left"

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

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

Reply via email to