Reviewers: rjrjr, juliog,

Description:
Change the wrapElement API to receive the id and a parent element. I
also ported some of the bookeeping we were doing in our internal Panel
to RenderablePanel, please let me know what you think.


Please review this at http://gwt-code-reviews.appspot.com/1446811/

Affected files:
M user/src/com/google/gwt/uibinder/elementparsers/IsRenderableInterpreter.java
  M user/src/com/google/gwt/user/client/ui/IsRenderable.java
  M user/src/com/google/gwt/user/client/ui/RenderableComposite.java
  M user/src/com/google/gwt/user/client/ui/RenderablePanel.java


Index: user/src/com/google/gwt/uibinder/elementparsers/IsRenderableInterpreter.java
===================================================================
--- user/src/com/google/gwt/uibinder/elementparsers/IsRenderableInterpreter.java (revision 10224) +++ user/src/com/google/gwt/uibinder/elementparsers/IsRenderableInterpreter.java (working copy)
@@ -29,7 +29,6 @@
 class IsRenderableInterpreter implements XMLElement.Interpreter<String> {

   private final String fieldName;
-
   private final UiBinderWriter uiWriter;

   public IsRenderableInterpreter(String fieldName, UiBinderWriter writer) {
@@ -50,15 +49,11 @@

FieldWriter childFieldWriter = uiWriter.parseElementToFieldWriter(elem);

-    String elementPointer = idHolder + "Element";
     fieldWriter.addAttachStatement(
-        "com.google.gwt.user.client.Element %s = " +
-        "com.google.gwt.dom.client.Document.get().getElementById(%s).cast();",
-        elementPointer, fieldManager.convertFieldToGetter(idHolder));
-    fieldWriter.addAttachStatement(
-        "%s.wrapElement(%s);",
+        "%s.wrapElement(%s, %s.getElement());",
         fieldManager.convertFieldToGetter(childFieldWriter.getName()),
-        elementPointer);
+        fieldManager.convertFieldToGetter(idHolder),
+        fieldManager.convertFieldToGetter(fieldName));

     // Some operations are more efficient when the Widget isn't attached to
     // the document. Perform them here.
Index: user/src/com/google/gwt/user/client/ui/IsRenderable.java
===================================================================
--- user/src/com/google/gwt/user/client/ui/IsRenderable.java (revision 10224)
+++ user/src/com/google/gwt/user/client/ui/IsRenderable.java    (working copy)
@@ -29,11 +29,10 @@
 public interface IsRenderable extends SafeHtmlRenderer<String> {

   /**
-   * Replace the previous contents of the receiver with the given element,
-   * presumed to have been created via a previous call to {@link #render}.
-   * Assumes the element is attached to the document.
+ * Tells the widget to find its element, given an id and a parent element in + * the DOM tree. The id is assumed to be the same passed in at render time.
    */
-  void wrapElement(Element element);
+  void wrapElement(String id, Element parentElement);

   /**
    * Perform any initialization needed when the widget is not attached to
Index: user/src/com/google/gwt/user/client/ui/RenderableComposite.java
===================================================================
--- user/src/com/google/gwt/user/client/ui/RenderableComposite.java (revision 10224) +++ user/src/com/google/gwt/user/client/ui/RenderableComposite.java (working copy)
@@ -16,6 +16,7 @@
 package com.google.gwt.user.client.ui;

 import com.google.gwt.core.client.GWT;
+import com.google.gwt.dom.client.Document;
 import com.google.gwt.dom.client.Element;
 import com.google.gwt.event.logical.shared.AttachEvent;
 import com.google.gwt.safehtml.client.SafeHtmlTemplates;
@@ -117,12 +118,12 @@
   }

   @Override
-  public void wrapElement(Element element) {
+  public void wrapElement(String id, Element parentElement) {
     if (renderable != null) {
       assert (initFinished == false);
-      renderable.wrapElement(element);
-    } else {
-      this.elementToWrap = element;
+      renderable.wrapElement(id, parentElement);
+    } else {
+      this.elementToWrap = Document.get().getElementById(id).cast();
     }
   }

Index: user/src/com/google/gwt/user/client/ui/RenderablePanel.java
===================================================================
--- user/src/com/google/gwt/user/client/ui/RenderablePanel.java (revision 10224) +++ user/src/com/google/gwt/user/client/ui/RenderablePanel.java (working copy)
@@ -57,12 +57,18 @@
     RootPanel.getBodyElement().appendChild(hiddenDiv);
   }

+  protected static enum Phase {
+    BUILT, RENDERED, WRAPPED, DONE
+  }
+
// TODO(rdcastro): Add setters for these, or maybe have a list instead of a
   // single callback.
   public Command wrapInitializationCallback = null;
   public Command detachedInitializationCallback = null;

+  protected Phase currentPhase = Phase.BUILT;
   protected SafeHtml html = null;
+
   private String styleName = null;

   /**
@@ -139,7 +145,9 @@

   @Override
   public com.google.gwt.user.client.Element getElement() {
-    if (!isFullyInitialized()) {
+    // If neither render nor wrapElement has been called yet,
+ // we have to assume this is the root of the tree and finish initialization
+    if (currentPhase == Phase.BUILT) {
       // In case we haven't finished initialization yet, finish it now.
       buildAndInitDivContainer();
       html = null;
@@ -172,10 +180,15 @@

   @Override
   public void performDetachedInitialization() {
+    // this can only be called after wrapElement
+    assert currentPhase == Phase.WRAPPED;
+
     if (detachedInitializationCallback != null) {
       detachedInitializationCallback.execute();
       detachedInitializationCallback = null;
     }
+
+    currentPhase = Phase.DONE;
   }

   @Override
@@ -187,28 +200,34 @@

   @Override
   public void render(String id, SafeHtmlBuilder builder) {
+    // render() must be the first method called when initializing via the
+    // IsRenderable path.
+    assert currentPhase == Phase.BUILT;
+
     if (styleName != null) {
builder.append(TEMPLATE.renderWithIdAndClass(id, styleName, getInnerHtml()));
       styleName = null;
     } else {
       builder.append(TEMPLATE.renderWithId(id, getInnerHtml()));
     }
+
+    currentPhase = Phase.RENDERED;
   }

   @Override
   public void setStyleName(String styleName) {
-    if (!isFullyInitialized()) {
+    if (currentPhase == Phase.DONE) {
+      super.setStyleName(styleName);
+    } else {
       // If we haven't built the actual HTML element yet, we save the style
       // to apply later on.
       this.styleName = styleName;
-    } else {
-      super.setStyleName(styleName);
-    }
-  }
-
-  @Override
-  public void wrapElement(Element element) {
-    if (isFullyInitialized()) {
+    }
+  }
+
+  @Override
+  public void wrapElement(String id, Element parentElement) {
+    if (currentPhase != Phase.RENDERED) {
       /*
* If wrapElement is being called after the widget is fully initialized, * that's probably a programming error, as it's much more efficient to
@@ -220,7 +239,12 @@
           + "panel)");
     }

-    setElement(element);
+    setElement(Document.get().getElementById(id));
+
+ // This is only here because wrapInitializationCallback may call getElement()
+    // and we must know that we don't need to recreate the whole tree.
+    currentPhase = Phase.WRAPPED;
+
     html = null;
     if (wrapInitializationCallback != null) {
       wrapInitializationCallback.execute();
@@ -233,14 +257,6 @@
    */
   protected SafeHtml getInnerHtml() {
     return html;
-  }
-
-  /**
- * Whether the initilization of the panel is finished (i.e., the corresponding
-   * DOM element has been built).
-   */
-  protected boolean isFullyInitialized() {
-    return html == null;
   }

   /**
@@ -253,6 +269,10 @@
     Element element = Document.get().createDivElement();
     element.setInnerHTML(getInnerHtml().asString());
     setElement(element);
+
+ // This is only here because wrapInitializationCallback may call getElement()
+    // and we must know that we don't need to recreate the whole tree.
+    currentPhase = Phase.DONE;

// If there's any wrap callback to call, we have to attach the div before
     // calling it, and then detach again.


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

Reply via email to