Just a few nits.

http://gwt-code-reviews.appspot.com/421802/diff/1/2
File /bikeshed/src/com/google/gwt/bikeshed/tree/client/CellBrowser.java
(right):

http://gwt-code-reviews.appspot.com/421802/diff/1/2#newcode126
/bikeshed/src/com/google/gwt/bikeshed/tree/client/CellBrowser.java:126:
*
space

http://gwt-code-reviews.appspot.com/421802/diff/1/2#newcode153
/bikeshed/src/com/google/gwt/bikeshed/tree/client/CellBrowser.java:153:
*
formatting pass do this? Check your eclipse settings.

http://gwt-code-reviews.appspot.com/421802/diff/1/5
File
/bikeshed/src/com/google/gwt/sample/expenses/gwt/customized/CustomizedShell.ui.xml
(right):

http://gwt-code-reviews.appspot.com/421802/diff/1/5#newcode25
/bikeshed/src/com/google/gwt/sample/expenses/gwt/customized/CustomizedShell.ui.xml:25:
background: white url(verticalShadow.png) left top repeat-y;
We should use an implicit ImageResource here. I dont remember the exact
tag attributes but something along the lines of:

@sprite .shadowLeft {
  gwt-image: 'myimage';
  ...
}

<ui:image field='myimage' repeatStyle="vertical"
resource="verticalShadow.png" />

http://gwt-code-reviews.appspot.com/421802/diff/1/5#newcode41
/bikeshed/src/com/google/gwt/sample/expenses/gwt/customized/CustomizedShell.ui.xml:41:
height='100%'>
would an image (for the logo) and a span (for the title text) not also
work here instead of a table?

http://gwt-code-reviews.appspot.com/421802/diff/1/7
File
/bikeshed/src/com/google/gwt/sample/expenses/gwt/customized/ExpenseList.java
(right):

http://gwt-code-reviews.appspot.com/421802/diff/1/7#newcode172
/bikeshed/src/com/google/gwt/sample/expenses/gwt/customized/ExpenseList.java:172:
return new Image("searchIcon.png");
Seems like we can make this an @sprite in the binder instead. What are
the benefits of using @UiFactory in code for an image?

http://gwt-code-reviews.appspot.com/421802/diff/1/8
File
/bikeshed/src/com/google/gwt/sample/expenses/gwt/customized/ExpenseList.ui.xml
(right):

http://gwt-code-reviews.appspot.com/421802/diff/1/8#newcode43
/bikeshed/src/com/google/gwt/sample/expenses/gwt/customized/ExpenseList.ui.xml:43:
ui:field='searchImage'>
Since we don't really need a widget here (no listeners on this) just
make it a sprite.

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

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

Reply via email to