http://gwt-code-reviews.appspot.com/1149803/show
Thanks so much for reviewing this guys. @Ray, Do you want me to go back to allowing a setter for the caption? 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 { On 2011/01/18 15:59:56, jlabanca wrote:
Replace HasHTML with HasSafeHtml. We're moving away from String html
methods. if Caption doesn't implement HasHTML, DialogBox#getHTML() gets ugly. I've added the HasSafeHtml interface but left HasHTML there to keep getHTML() clean. 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 On 2011/01/18 15:59:56, jlabanca wrote:
and IT => and it
Done. 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 On 2011/01/18 15:59:56, jlabanca wrote:
long line
Done. http://gwt-code-reviews.appspot.com/1149803/diff/22001/23001#newcode214 user/src/com/google/gwt/user/client/ui/DialogBox.java:214: * @param autoHide On 2011/01/18 15:59:56, jlabanca wrote:
copy param descriptions from other constructors.
Done. 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 On 2011/01/18 15:59:56, jlabanca wrote:
For multiline comments, use /* comments: /* * line 0 * line 1 */
Done. 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.. On 2011/01/18 15:59:56, jlabanca wrote:
extra .
Done. 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()); On 2011/01/18 15:59:56, jlabanca wrote:
assuming HasSafeHtml interface: caption.setHTML(html);
Done. 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); On 2011/01/18 15:59:56, jlabanca wrote:
caption.setHTML(SafeHtmlUtils.fromTrustedString(html))
Done. 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>"); On 2011/01/18 15:59:56, jlabanca wrote:
Some browser capitalize HTML automatically. Use: dialogBox.getHTML().toLowercase();
Done. 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)); On 2011/01/18 15:59:56, jlabanca wrote:
cleanup after the test: dialogBox.hide();
Done. 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 On 2011/01/18 15:59:56, jlabanca wrote:
Move class definition to the top of the test class.
Done. http://gwt-code-reviews.appspot.com/1149803/show -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
