placeAt() API confusing

[bill]

See attached file, It doesn't layout correctly, although if startup() is called at the very end it does.

[bill]

teslan's test case

[bill]

Turns out it's a problem with the test case, or arguably a design problem in the spec of the placeAt() function. In other words, placeAt() is working according to spec, but the spec turns out to be confusing. I'm CC'ing Pete to see if he has anything to say, since placeAt() is his function.

The current rules are:

(1) if the reference widget has an addChild() method then pass it in without quotes. So, this works:

myWidget.placeAt(myLayoutWidget)

But this fails:

myWidget.placeAt("myLayoutWidget")

If placeAt() is passed a string, it's treated as a reference to a DOMNode, not as a reference to a widget.

(2) If the reference widget does not have an addChild() method, the opposite is true. This is correct:

myWidget.placeAt("myContentPane")

or slightly clearer:

myWidget.placeAt(myContentPane.domNode)

but this will fail:

myWidget.placeAt(myContentPane)

Of course, one way to avoid this problem is to stop using placeAt(). The test case could be rewritten to use addChild() for the layout widgets and set("content", ...) for the ContentPane.

[bill]

test case with placeAt() calls corrected to be used according to it's spec... it's working

[bill]

Turns out this is complicated to implement to work intuitively, since:

childWidget.placeAt(parentWidget, 5);

should call parentWidget.addChild(), whereas

childWidget.placeAt(parentWidget, "before");
childWidget.placeAt(parentWidget, "after");

should be calling domConstruct.place().

Further, we need special handling for first and last when an addChild() method is available:

childWidget.placeAt(parentWidget, "first");
       --> parentWidget.addChild(childWidget, 0);
childWidget.placeAt(parentWidget, "last");
       --> parentWidget.addChild(childWidget, parentWidget.getChildren().length);

(This applies equally whether parentWidget is a widget reference, or a widget id.)

[bill]

Even worse are the "before", "after", "replace" and "only" options, which currently somewhat work when you pass a DOMNode or string to placeAt(). They don't generally do what's expected when you pass an id of a widget, but they do something.

A correct implementation of childWidget.placeAt(reference, "before"), childWidget.placeAt(reference, "after") and childWidget.placeAt(reference, "replace") need to call addChild() on the grandparent widget, plus for position="replace", removeChild() and destroyRecursive(), and would be something like:

var refWidget = registry.byId(reference);
     gpw = refWidget && refWidget.getParent();
if(gpw && gpw.addChild){
    var parentIndex = gpw.getIndexOfChild(refWidget);
    gpw.addChild(this, position == "before" ? parentIndex : parentIndex+1);
}else{
   domConstruct.place(this.domNode, reference, position);
}
if(position == "replace" && refWidget){
    gpw.removeChild && gpw.removeChild(refWidget);
    refWidget.destroyRecursive();
}

and for childWidget.placeAt(reference, "only"):

if(refWidget){
   array.forEach(refWidget.getChildren(), function(child){
        refWidget.removeChild && refWidget.removeChild(child);
        child.destroyRecursive();
   }
}
domConstruct.empty(refWidget ? refWidget.containerNode || refWidget.domNode : reference);  // in case any stray DOM left
this.placeAt(reference, 0);

[bill]

Code for a placeAt() that supports all the positional parameters that domConstruct.place() does. But I don't want to check this in since it's a fair amount of code.

[bill]

In [28251]:

Allow placeAt() to take an id of a widget or DOMNode, rather than an id refers to a DOMNode. This will (hopefully) avoid confusion given that a widget's id == it's DOMNode's id.

If the parent widget is started then call startup() on the child widget automatically, even if the parent doesn't have an addChild() method.

Also upgrading test to AMD.

Fixes #14946, #15066 !strict.