LGTM

But lots of formatting errors.


http://gwt-code-reviews.appspot.com/864801/diff/3001/4009
File user/src/com/google/gwt/user/client/ui/AbsolutePanel.java (right):

http://gwt-code-reviews.appspot.com/864801/diff/3001/4009#newcode99
user/src/com/google/gwt/user/client/ui/AbsolutePanel.java:99:
extra spaces

http://gwt-code-reviews.appspot.com/864801/diff/3001/4009#newcode154
user/src/com/google/gwt/user/client/ui/AbsolutePanel.java:154:
extra spaces

http://gwt-code-reviews.appspot.com/864801/diff/3001/4010
File user/src/com/google/gwt/user/client/ui/ComplexPanel.java (right):

http://gwt-code-reviews.appspot.com/864801/diff/3001/4010#newcode46
user/src/com/google/gwt/user/client/ui/ComplexPanel.java:46:
extra spaces

http://gwt-code-reviews.appspot.com/864801/diff/3001/4012
File user/src/com/google/gwt/user/client/ui/FlowPanel.java (right):

http://gwt-code-reviews.appspot.com/864801/diff/3001/4012#newcode41
user/src/com/google/gwt/user/client/ui/FlowPanel.java:41:
extra newline

http://gwt-code-reviews.appspot.com/864801/diff/3001/4013
File user/src/com/google/gwt/user/client/ui/HasOneWidget.java (right):

http://gwt-code-reviews.appspot.com/864801/diff/3001/4013#newcode21
user/src/com/google/gwt/user/client/ui/HasOneWidget.java:21: public
interface HasOneWidget {
Would HasWidget be a better name?  The singularity of Widget implicitly
assumes it has one.

http://gwt-code-reviews.appspot.com/864801/diff/3001/4015
File user/src/com/google/gwt/user/client/ui/HorizontalPanel.java
(right):

http://gwt-code-reviews.appspot.com/864801/diff/3001/4015#newcode34
user/src/com/google/gwt/user/client/ui/HorizontalPanel.java:34:
extra newline

http://gwt-code-reviews.appspot.com/864801/diff/3001/4015#newcode35
user/src/com/google/gwt/user/client/ui/HorizontalPanel.java:35: private
VerticalAlignmentConstant vertAlign = ALIGN_TOP;
missing newline

http://gwt-code-reviews.appspot.com/864801/diff/3001/4015#newcode46
user/src/com/google/gwt/user/client/ui/HorizontalPanel.java:46: public
void add(IsWidget w) {
missing newline

http://gwt-code-reviews.appspot.com/864801/diff/3001/4016
File user/src/com/google/gwt/user/client/ui/IndexedPanel.java (right):

http://gwt-code-reviews.appspot.com/864801/diff/3001/4016#newcode30
user/src/com/google/gwt/user/client/ui/IndexedPanel.java:30: * /** Gets
the child widget at the specified index.
syntax

http://gwt-code-reviews.appspot.com/864801/diff/3001/4019
File user/src/com/google/gwt/user/client/ui/Panel.java (right):

http://gwt-code-reviews.appspot.com/864801/diff/3001/4019#newcode69
user/src/com/google/gwt/user/client/ui/Panel.java:69:
extra spaces

http://gwt-code-reviews.appspot.com/864801/diff/3001/4019#newcode110
user/src/com/google/gwt/user/client/ui/Panel.java:110:
extra spaces

http://gwt-code-reviews.appspot.com/864801/diff/3001/4020
File user/src/com/google/gwt/user/client/ui/PopupPanel.java (right):

http://gwt-code-reviews.appspot.com/864801/diff/3001/4020#newcode971
user/src/com/google/gwt/user/client/ui/PopupPanel.java:971: public void
setWidget(IsWidget w) {
+1 users may override setWidget(Widget) and expect it to be called.  I
think this method should delegate to setWidget(widget)

http://gwt-code-reviews.appspot.com/864801/diff/3001/4021
File user/src/com/google/gwt/user/client/ui/SimplePanel.java (right):

http://gwt-code-reviews.appspot.com/864801/diff/3001/4021#newcode47
user/src/com/google/gwt/user/client/ui/SimplePanel.java:47:
extra spaces

http://gwt-code-reviews.appspot.com/864801/diff/3001/4021#newcode117
user/src/com/google/gwt/user/client/ui/SimplePanel.java:117:
extra spaces

http://gwt-code-reviews.appspot.com/864801/diff/3001/4023
File user/src/com/google/gwt/user/client/ui/StackPanel.java (right):

http://gwt-code-reviews.appspot.com/864801/diff/3001/4023#newcode72
user/src/com/google/gwt/user/client/ui/StackPanel.java:72: }
missing newline

http://gwt-code-reviews.appspot.com/864801/diff/3001/4024
File user/src/com/google/gwt/user/client/ui/TabLayoutPanel.java (right):

http://gwt-code-reviews.appspot.com/864801/diff/3001/4024#newcode264
user/src/com/google/gwt/user/client/ui/TabLayoutPanel.java:264:
extra spaces

http://gwt-code-reviews.appspot.com/864801/diff/3001/4024#newcode277
user/src/com/google/gwt/user/client/ui/TabLayoutPanel.java:277:
extra spaces

http://gwt-code-reviews.appspot.com/864801/diff/3001/4025
File user/src/com/google/gwt/user/client/ui/TabPanel.java (right):

http://gwt-code-reviews.appspot.com/864801/diff/3001/4025#newcode323
user/src/com/google/gwt/user/client/ui/TabPanel.java:323:
extra spaces

http://gwt-code-reviews.appspot.com/864801/diff/3001/4025#newcode327
user/src/com/google/gwt/user/client/ui/TabPanel.java:327:
extra spaces

http://gwt-code-reviews.appspot.com/864801/diff/3001/4026
File user/src/com/google/gwt/user/client/ui/VerticalPanel.java (right):

http://gwt-code-reviews.appspot.com/864801/diff/3001/4026#newcode33
user/src/com/google/gwt/user/client/ui/VerticalPanel.java:33:
extra newline

http://gwt-code-reviews.appspot.com/864801/diff/3001/4026#newcode41
user/src/com/google/gwt/user/client/ui/VerticalPanel.java:41: }
missing newline

http://gwt-code-reviews.appspot.com/864801/diff/3001/4034
File user/test/com/google/gwt/user/client/ui/SimplePanelTest.java
(right):

http://gwt-code-reviews.appspot.com/864801/diff/3001/4034#newcode35
user/test/com/google/gwt/user/client/ui/SimplePanelTest.java:35:
p.setWidget(null);
Is that because Widget extends IsWidget?  I assume it uses the IsWidget
version then, and in general does a runtime check?

http://gwt-code-reviews.appspot.com/864801/diff/3001/4036
File user/test/com/google/gwt/user/client/ui/WidgetTest.java (right):

http://gwt-code-reviews.appspot.com/864801/diff/3001/4036#newcode49
user/test/com/google/gwt/user/client/ui/WidgetTest.java:49: final
com.google.gwt.event.shared.HandlerManager manager = new
com.google.gwt.event.shared.HandlerManager(
Why did you inline the path for HandlerManager?

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

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

Reply via email to