Sorry, one more round.

http://gwt-code-reviews.appspot.com/1450810/diff/5006/user/src/com/google/gwt/user/client/ui/PotentialElement.java
File user/src/com/google/gwt/user/client/ui/PotentialElement.java
(right):

http://gwt-code-reviews.appspot.com/1450810/diff/5006/user/src/com/google/gwt/user/client/ui/PotentialElement.java#newcode38
user/src/com/google/gwt/user/client/ui/PotentialElement.java:38: public
class PotentialElement extends JavaScriptObject {
Should extend Element.

http://gwt-code-reviews.appspot.com/1450810/diff/5006/user/src/com/google/gwt/user/client/ui/PotentialElement.java#newcode46
user/src/com/google/gwt/user/client/ui/PotentialElement.java:46: public
static native Element build(UIObject o) /*-{
When PotentialElement extends Element, you can return PotentialElement
here. Need to fix the first sentence of the javadoc accordingly.

http://gwt-code-reviews.appspot.com/1450810/diff/5006/user/src/com/google/gwt/user/client/ui/PotentialElement.java#newcode47
user/src/com/google/gwt/user/client/ui/PotentialElement.java:47: return
{
Pass this through Element.as() for sanity check.

http://gwt-code-reviews.appspot.com/1450810/diff/5006/user/src/com/google/gwt/user/client/ui/PotentialElement.java#newcode53
user/src/com/google/gwt/user/client/ui/PotentialElement.java:53: lang:
'',
nodeType: @com.google.gwt.dom.client.Node.ELEMENT_NODE;

http://gwt-code-reviews.appspot.com/1450810/diff/5006/user/src/com/google/gwt/user/client/ui/PotentialElement.java#newcode56
user/src/com/google/gwt/user/client/ui/PotentialElement.java:56:
__gwt_resolve: function() {
indent

http://gwt-code-reviews.appspot.com/1450810/diff/5006/user/src/com/google/gwt/user/client/ui/PotentialElement.java#newcode57
user/src/com/google/gwt/user/client/ui/PotentialElement.java:57:
this.__gwt_resolve = null;
this.__gwt_resolve =
@com.google.gwt.user.client.ui.UIObject::cannotResolveTwice()();

On the other hand we could cache the real element and return it on
repeated
calls, but that seems like inviting trouble.

http://gwt-code-reviews.appspot.com/1450810/diff/5006/user/src/com/google/gwt/user/client/ui/PotentialElement.java#newcode62
user/src/com/google/gwt/user/client/ui/PotentialElement.java:62: }-*/;
private static final void cannotResolveTwice() /*-{
  throw "A PotentialElement cannot be resolved twice.";
}-*/;

http://gwt-code-reviews.appspot.com/1450810/diff/5006/user/src/com/google/gwt/user/client/ui/PotentialElement.java#newcode67
user/src/com/google/gwt/user/client/ui/PotentialElement.java:67: public
static PotentialElement as(JavaScriptObject o) {
Offline you pointed out that you can't make this assertion, because
Document.appendChild() calls it for everything. How about making this
return Element, and inline the call to resolve? For that matter, we
could make resolve private if this is the only call to it.

Also note the change of the argument from JSO to Element

/**
 * If given a PotentialElement, returns the real Element to be
 * built from it. Otherwise returns the given Element itself.
 * <p>
 * Note that a PotentialElement can only be resolved once.
 * Making repeated calls to this method with the same PotentialElement
 * is an error.
 */
public static Element resolve(Element maybePotential) {
  return o.<PotentialElement>cast().resolve();
}

http://gwt-code-reviews.appspot.com/1450810/

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

Reply via email to