Opened 6 years ago

Closed 6 years ago

Last modified 6 years ago

#16874 closed defect (invalid)

async-adding of children to layout-container doesn't work any more

Reported by: gerhard presser Owned by: gerhard presser
Priority: undecided Milestone: tbd
Component: Dijit Version: 1.9.0a2
Keywords: Cc:
Blocked By: Blocking:

Description

I'm loading child-components in an async way. each child has a fixed index.

it may happen, that the 2nd child get's added before the 1st child.

this worked until 1.8.3 - in 1.9.0a2 this doesn't work any more. I tracked the problem to _Container.addChild().

please see attached patch.

Attachments (1)

_Container.js.patch (599 bytes) - added by gerhard presser 6 years ago.

Download all attachments as: .zip

Change History (12)

Changed 6 years ago by gerhard presser

Attachment: _Container.js.patch added

comment:1 Changed 6 years ago by bill

Owner: changed from bill to gerhard presser
Status: newpending

Can you attach a test case? It looks like you are doing something strange but I want to confirm.

comment:2 Changed 6 years ago by gerhard presser

Status: pendingnew

for example...

var container = new AccordionContainer();
...
dojo.forEach(children,function(childDefinition,index){
     request("url").then(function(){
         container.addChild(new ContentPane(),index);
     });
});
...

comment:3 Changed 6 years ago by bill

Hmm, well that never worked. The URL's will be added as children in a random order, rather than according to index.

comment:4 Changed 6 years ago by gerhard presser

it's working in 1.8.3 ;-)

and with the supplied patch.

if there're not enough children, the new one get's appended at the end, if there're enogh, it's placed at the desired location.

since all my addChild() calls supply an index, after all calls complete, each child is at it's place.

comment:5 Changed 6 years ago by bill

I don't think so. This is why I asked you to give me a test case. Yet, you have not. Try a test case adding children in the reverse order (start with index 5 and then end with index 0), and see if they show up in the right order or not. Then, attach the test case.

comment:6 Changed 6 years ago by gerhard presser

ok - you proved me wrong - nethertheless, the new code doesn't behave like the old one.

is there any better way of accomplishing my targets?

comment:7 Changed 6 years ago by bill

Resolution: invalid
Status: newclosed

I'd suggest waiting for all the URLs to load first:

all(array.map(children, request)).then(function(responses){
        var container = new AccordionContainer();
        array.forEach(responses, function(){ container.addChild(...));
});

In any case, I'm going to close this ticket. It's true that addChild() behaves differently now when passed illegal arguments, but the new behavior of throwing an exception (albeit perhaps a hard to understand exception) is more in line with dojo's principles.

comment:8 Changed 6 years ago by Wouter Hager

In many cases, a _Container will have a child nodes map. This, or an index on the child itself, could be used to insert children at the correct position. The downside is that this introduces state. But I suspect some state will be used anyway.

comment:9 Changed 6 years ago by Wouter Hager

A solution would be to require addChild to have an additional argument 'length', which will be used to place all expected children up until length-1 based on a temporary stack.

Anyway, I think async placement is quite useful in places.

comment:10 Changed 6 years ago by bill

#17547 is a duplicate of this ticket.

comment:11 Changed 6 years ago by bill

#17548 is a duplicate of this ticket.

Note: See TracTickets for help on using tickets.