Great work.  I found a few nitpicks that should be easy to fix.


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

http://gwt-code-reviews.appspot.com/1149803/diff/22001/23001#newcode113
user/src/com/google/gwt/user/client/ui/DialogBox.java:113: public
interface Caption extends HasAllMouseHandlers, HasHTML, IsWidget {
Replace HasHTML with HasSafeHtml.  We're moving away from String html
methods.

http://gwt-code-reviews.appspot.com/1149803/diff/22001/23001#newcode184
user/src/com/google/gwt/user/client/ui/DialogBox.java:184: * auto-hide
is set to false and  It should
and  IT => and it

http://gwt-code-reviews.appspot.com/1149803/diff/22001/23001#newcode210
user/src/com/google/gwt/user/client/ui/DialogBox.java:210: * Creates an
empty dialog box specifying its "auto-hide" property and an
implementation of the {@link Caption}. It should
long line

http://gwt-code-reviews.appspot.com/1149803/diff/22001/23001#newcode214
user/src/com/google/gwt/user/client/ui/DialogBox.java:214: * @param
autoHide
copy param descriptions from other constructors.

http://gwt-code-reviews.appspot.com/1149803/diff/22001/23001#newcode221
user/src/com/google/gwt/user/client/ui/DialogBox.java:221: assert
captionWidget != null : "The caption must not be null";
Since caption can now come from the outside, we need to call:
caption.asWidget().removeFromParent()

http://gwt-code-reviews.appspot.com/1149803/diff/22001/23001#newcode298
user/src/com/google/gwt/user/client/ui/DialogBox.java:298: // not fire
its click event for example
For multiline comments, use /* comments:
/*
 * line 0
 * line 1
 */

http://gwt-code-reviews.appspot.com/1149803/diff/22001/23001#newcode351
user/src/com/google/gwt/user/client/ui/DialogBox.java:351: * {@link
#setHTML(SafeHTML)} method..
extra .

http://gwt-code-reviews.appspot.com/1149803/diff/22001/23001#newcode359
user/src/com/google/gwt/user/client/ui/DialogBox.java:359:
setHTML(html.asString());
assuming HasSafeHtml interface:
caption.setHTML(html);

http://gwt-code-reviews.appspot.com/1149803/diff/22001/23001#newcode372
user/src/com/google/gwt/user/client/ui/DialogBox.java:372:
caption.setHTML(html);
caption.setHTML(SafeHtmlUtils.fromTrustedString(html))

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

http://gwt-code-reviews.appspot.com/1149803/diff/22001/23002#newcode97
user/test/com/google/gwt/user/client/ui/DialogBoxTest.java:97:
assertEquals(dialogBox.getHTML(), "<b>text</b>");
Some browser capitalize HTML automatically.  Use:
dialogBox.getHTML().toLowercase();

http://gwt-code-reviews.appspot.com/1149803/diff/22001/23002#newcode100
user/test/com/google/gwt/user/client/ui/DialogBoxTest.java:100:
assertTrue(caption.asWidget().getElement() == DOM.getChild(td, 0));
cleanup after the test:
dialogBox.hide();

http://gwt-code-reviews.appspot.com/1149803/diff/22001/23002#newcode103
user/test/com/google/gwt/user/client/ui/DialogBoxTest.java:103: private
class CaptionForTesting extends Composite implements
Move class definition to the top of the test class.

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

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

Reply via email to