Are all these ForIsWidget interfaces (and method overloads) really
needed? (and actually useful?)

For completeness, TabLayoutPanel and TabPanel don't have IsWidget
overloads for their add, getTabWidget and selectTab methods.

(if you ask me, I'd only keep HasOneWidget, without the getWidget
method, i.e. just a move of Activity.Display into c.g.g.user.client.ui,
and have it implemented by SimplePanel; and also of course the move of
IsWidget, implemented by Widget; and the Widget#getWidgetOrNull, as a
public method; and that's it)


http://gwt-code-reviews.appspot.com/864801/diff/3001/4003
File user/src/com/google/gwt/app/place/Activity.java (right):

http://gwt-code-reviews.appspot.com/864801/diff/3001/4003#newcode58
user/src/com/google/gwt/app/place/Activity.java:58: * {...@link
Display#setWidget(IsWidget)}.
s/Display/HasOneWidget/

http://gwt-code-reviews.appspot.com/864801/diff/3001/4004
File user/src/com/google/gwt/app/place/ActivityManager.java (right):

http://gwt-code-reviews.appspot.com/864801/diff/3001/4004#newcode60
user/src/com/google/gwt/app/place/ActivityManager.java:60: public Widget
getWidget() {
Which makes me wonder: should HasOneWidget have this getWidget method?

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#newcode28
user/src/com/google/gwt/user/client/ui/HasOneWidget.java:28: public
Widget getWidget();
The asymmetry between setWidget taking an IsWidget, and getWidget
returning a Widget looks weird.

Maybe rename the interface to TakesOneWidget and only keep its
setWidget(IsWdiget) method?
...or maybe add a setWidget(Widget) method.

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) {
Shouldn't this be a new method overload, and keep the setWidget(Widget)
override to also maybeUpdateSize?

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

http://gwt-code-reviews.appspot.com/864801/diff/3001/4027#newcode42
user/src/com/google/gwt/user/client/ui/Widget.java:42: protected static
Widget asWidgetOrNull(IsWidget w) {
Making it public would be harmless and could prove useful ;-)

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

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

Reply via email to