http://gwt-code-reviews.appspot.com/1442804/diff/10001/user/src/com/google/gwt/uibinder/rebind/AbstractFieldWriter.java File user/src/com/google/gwt/uibinder/rebind/AbstractFieldWriter.java (right):
http://gwt-code-reviews.appspot.com/1442804/diff/10001/user/src/com/google/gwt/uibinder/rebind/AbstractFieldWriter.java#newcode123 user/src/com/google/gwt/uibinder/rebind/AbstractFieldWriter.java:123: public void resetWritten() { On 2011/05/23 18:25:23, rjrjr wrote:
uncalled, right?
Done. http://gwt-code-reviews.appspot.com/1442804/diff/10001/user/src/com/google/gwt/uibinder/rebind/UiBinderWriter.java File user/src/com/google/gwt/uibinder/rebind/UiBinderWriter.java (right): http://gwt-code-reviews.appspot.com/1442804/diff/10001/user/src/com/google/gwt/uibinder/rebind/UiBinderWriter.java#newcode235 user/src/com/google/gwt/uibinder/rebind/UiBinderWriter.java:235: private boolean isRenderer; On 2011/05/23 18:25:23, rjrjr wrote:
Should be final
Done. http://gwt-code-reviews.appspot.com/1442804/diff/10001/user/src/com/google/gwt/uibinder/rebind/UiBinderWriter.java#newcode861 user/src/com/google/gwt/uibinder/rebind/UiBinderWriter.java:861: die(baseClass.getName() + " must implement UiBinder or SafeHtmlRenderer"); On 2011/05/23 18:25:23, rjrjr wrote:
Are we making a comparable check for implements SafeHtmlRenderer?
Actually, this is the wrong place for this check. It should happen in the constructor, where isRenderer is determined. http://gwt-code-reviews.appspot.com/1442804/diff/10001/user/src/com/google/gwt/uibinder/rebind/UiBinderWriter.java#newcode1464 user/src/com/google/gwt/uibinder/rebind/UiBinderWriter.java:1464: // TODO(sbrubaker): Fix method signature On 2011/05/23 18:25:23, rjrjr wrote:
Is this TODO still valid? I've lost track of what it refers to. If it
is, can
you make it a bit more informative?
Done. Confirmed with sbrubaker that the TODO was stale. http://gwt-code-reviews.appspot.com/1442804/diff/10001/user/test/com/google/gwt/uibinder/elementparsers/DialogBoxParserTest.java File user/test/com/google/gwt/uibinder/elementparsers/DialogBoxParserTest.java (right): http://gwt-code-reviews.appspot.com/1442804/diff/10001/user/test/com/google/gwt/uibinder/elementparsers/DialogBoxParserTest.java#newcode80 user/test/com/google/gwt/uibinder/elementparsers/DialogBoxParserTest.java:80: "fieldName.setHTML(\"@mockToken-fieldName-Hello, I <b>caption</b>you.\");", On 2011/05/23 18:25:23, rjrjr wrote:
is the "fieldName" string coming from a constant in
ElementParserTester or
something? If so, can you open up its visibility and use it in these
tests,
rather having it inlined all over the place?
Done for the lines changed in this Issue. The string comes from ElementParserTester#FIELD_NAME. It is hard-coded all over the place, though (at least 45/278 tests in UiBinderJreSuite). May I finish surfacing it in a separate Issue? http://gwt-code-reviews.appspot.com/1442804/diff/10001/user/test/com/google/gwt/uibinder/test/client/UiBinderTest.java File user/test/com/google/gwt/uibinder/test/client/UiBinderTest.java (right): http://gwt-code-reviews.appspot.com/1442804/diff/10001/user/test/com/google/gwt/uibinder/test/client/UiBinderTest.java#newcode73 user/test/com/google/gwt/uibinder/test/client/UiBinderTest.java:73: public void testSafeHtmlRendererText() { On 2011/05/23 18:25:23, rjrjr wrote:
Doesn't need to block this patch, but as a follow up could you break
this out
into its own test class, perhaps SafeHtmlRendererTest?
SafeHtmlAsComponentsTest
should serve as an example.
SGTM. http://gwt-code-reviews.appspot.com/1442804/ -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
