Opened 10 years ago

Closed 10 years ago

Last modified 10 years ago

#9657 closed defect (fixed)

_Templated: some nested widgets in template no longer get startup() called

Reported by: dstarke Owned by: Nathan Toone
Priority: high Milestone: 1.4
Component: Dijit Version: 1.3.2
Keywords: Cc: Nathan Toone
Blocked By: Blocking:

Description (last modified by bill)

Commit [19488] changed the way that the _Templated class records its supporting widgets.

Since this change, I find that when I have a template that defines widgets with nested widgets inside, the inner widgets are not getting startup() called on them. For layout reasons, the inner widgets are sometimes not direct children of the widget's DOM node-- my understanding is that this should be allowed.

Researching further, I find that the _supportingWidgets array is empty even though there should be a number of widgets there. It turns out that when my template is initialized, dijit.findWidgets(), which is what is now used to populate _supportingWidgets, is returning an empty list. My understanding is that findWidgets only finds the widgets that are direct children, but in this case, there are none, or some are missing.

There does not appear to be any other mechanism to cause these inner widgets to get started up.

Reverting the portion of change 19488 that assigns the contents of _supportingWidgets (to make it the output of parse()) resolves this problem.

Change History (11)

comment:1 Changed 10 years ago by bill

Description: modified (diff)
Milestone: tbd1.4
Owner: set to bill
Summary: Some nested widgets in template no longer get startup() called_Templated: some nested widgets in template no longer get startup() called

Hmm, that's not good. Can you attach a test case?

Maybe _Templated needs to do call startup on the nested widgets that don't have a _Container as a parent, not just top level widgets, like the parser does at the page level:

d.forEach(thelist, function(instance){
	if(	!args.noStart && instance  && 
		instance.startup &&
		!instance._started && 
		(!instance.getParent || !instance.getParent())
	){
		instance.startup();
	}
});

comment:2 Changed 10 years ago by bill

Cc: Nathan Toone added

Nathan is going to write some tests for this and then we'll fix the code.

comment:3 Changed 10 years ago by Nathan Toone

(In [19799]) Refs #9657 - add test case for making sure that child widgets are correctly started up

comment:4 Changed 10 years ago by Nathan Toone

(In [19800]) Refs #9657 - roll back change 19488 - which breaks startup of nested widgets, as shown by new test case _Templated-widgetsInTemplate.html

comment:5 Changed 10 years ago by Nathan Toone

Resolution: fixed
Status: newclosed

(In [19801]) Fixes #9657 - fix issues with destroy getting called multiple times. This should address #9657

comment:6 Changed 10 years ago by Nathan Toone

(In [19802]) Refs #9657 - inherited needs to be called before the resizing - since that is where we moved the startup calls to

comment:7 Changed 10 years ago by bill

Resolution: fixed
Status: closedreopened

I'm pretty sure that after the above changes (which do fix the startup() problem), destroyRecursive() will be called twice on children of widgets that don't define isContainer:true (ie, widgets that don't extend ContentPane or_Container). For example, if foo is a simple widget not extending _Container:

dojo.declare("foo", [dijit._Widget, dijit._Templated], {
    templateString: "<div dojoType=containerNode></div>"
});

Then if we have a widget that references "foo" in it's template:

<div>
   <div dojoType=foo>
        <div dojoType=bar></div>
   </div>
</div>

The children of "foo" ("bar" in the above example) will be destroyed twice.

Admittedly that's an unusual case but it should be addressed; destroyRecursive() should really only be called on the top level widgets (foo in the example above).

Talked w/Nathan about this but reopening ticket so we don't forget.

comment:8 Changed 10 years ago by bill

Owner: changed from bill to Nathan Toone
Status: reopenednew

comment:9 Changed 10 years ago by Nathan Toone

Resolution: fixed
Status: newclosed

(In [19904]) Fixes #9657 - fix typo in test case - which then failed - and then fix the failure !strict

comment:10 Changed 10 years ago by Nathan Toone

(In [19905]) Fixes #9657 - if at first you don't succeed....there's got to be a better way of doing this. :)

comment:11 Changed 10 years ago by bill

(In [19928]) Widgets that have children must define containerNode (regardless of widgetsInTemplate). Otherwise getChildren(), destroyDescendants() etc. don't work.

So, updating the test by removing that case, and then simplifying the code to compute _supportingWidgets so it only has top level supporting widgets.

Refs #9657 !strict.

Note: See TracTickets for help on using tickets.