cc'ing John L, who actually understands this code

http://gwt-code-reviews.appspot.com/1149803/diff/1/3
File user/src/com/google/gwt/user/client/ui/DialogBox.java (right):

http://gwt-code-reviews.appspot.com/1149803/diff/1/3#newcode116
user/src/com/google/gwt/user/client/ui/DialogBox.java:116: * your own
risk.
Now that it's actually useful to implement the interface, I don't think
we can keep this disclaimer.

http://gwt-code-reviews.appspot.com/1149803/diff/1/3#newcode320
user/src/com/google/gwt/user/client/ui/DialogBox.java:320: public void
setCaption(Caption caption) {
Your removal code here looks wrong, and I don't think it's worth making
it right. Instead make the caption an optional constructor argument, not
a settable property.

Anyone who wants that kind of flexibility and its headaches can provide
a caption that extends Panel or something.

http://gwt-code-reviews.appspot.com/1149803/diff/1/3#newcode322
user/src/com/google/gwt/user/client/ui/DialogBox.java:322: if
(this.caption == caption) {
No real benefit to this check (and not needed if we don't allow changing
the caption out).

http://gwt-code-reviews.appspot.com/1149803/diff/1/3#newcode327
user/src/com/google/gwt/user/client/ui/DialogBox.java:327: if
(this.caption != null) {
Unnecessary check.

http://gwt-code-reviews.appspot.com/1149803/diff/1/3#newcode338
user/src/com/google/gwt/user/client/ui/DialogBox.java:338:
caption.asWidget().setStyleName("Caption");
If we're accepting someone else's widget, it seems wrong to clobber
their style name.

http://gwt-code-reviews.appspot.com/1149803/diff/1/3#newcode343
user/src/com/google/gwt/user/client/ui/DialogBox.java:343: * Sets the
html string inside the caption.
" by calling its {...@link #setHTML(SafeHTML)} method."

http://gwt-code-reviews.appspot.com/1149803/diff/1/3#newcode355
user/src/com/google/gwt/user/client/ui/DialogBox.java:355: * Sets the
html string inside the caption.
" by calling its {...@link #setHTML(String)} method."

http://gwt-code-reviews.appspot.com/1149803/diff/1/3#newcode367
user/src/com/google/gwt/user/client/ui/DialogBox.java:367: * Sets the
text inside the caption.
" by calling its {...@link #setText(SafeHTML)} method."

http://gwt-code-reviews.appspot.com/1149803/diff/1/3#newcode486
user/src/com/google/gwt/user/client/ui/DialogBox.java:486: protected
void registerMouseHandlers() {
won't need this with immutable caption

http://gwt-code-reviews.appspot.com/1149803/diff/1/2
File user/test/com/google/gwt/user/client/ui/DialogBoxTest.java (right):

http://gwt-code-reviews.appspot.com/1149803/diff/1/2#newcode158
user/test/com/google/gwt/user/client/ui/DialogBoxTest.java:158: // TODO
Auto-generated method stub
snip

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

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

Reply via email to