Opened 11 years ago

Closed 8 years ago

Last modified 7 years ago

#7819 closed enhancement (fixed)

ContentPane: missing an addChild() method

Reported by: alex Owned by: bill
Priority: high Milestone: 1.8
Component: Dijit Version: 1.2.0
Keywords: contentpane, dijit, _Container, layout Cc:
Blocked By: Blocking:

Description (last modified by alex)

so I'm not sure how to solve this, but many users expect ContentPane? to implement dijit._Container . Today it doesn't. I can see 2 possible solutions:

  • a dijit.layout.ContentPaneContainer which is a trivial subclass of ContentPane? mixes in _Container
  • extend/enhance ContenPane? to implement the addChild method

I'm not sure that I have a preference about which solution is preferable, just that one of them makes it in for 1.3.

The subclass solution might look like:

// dijit/layout/ContainerPane.js

dojo.provide("dijit.layout.ContainerPane");
dojo.require("dijit.layout.ContentPane");
dojo.require("dijit.layout._Container");

dojo.declare("dijit.layout.ContainerPane", [ dijit.layout.ContentPane, dijit._Container ], {});

Change History (22)

comment:1 Changed 11 years ago by alex

Description: modified (diff)

comment:2 Changed 11 years ago by bill

Milestone: 1.31.4
Type: defectenhancement

I'm not clear on how the ContainerPane would function.

  1. What's the use case for having an addChild() method?
  2. If you created an empty ContainerPane and then called addChild() three times, would all the children show up on the screen at once, one underneath the other?
  3. What would getChildren() return if the ContainerPane has free-form content like a normal ContentPane()?

etc.

_Container is only meant for widgets that contain a straight list of children, not for widgets with free-form HTML like ContentPane.

comment:3 Changed 11 years ago by bill

Resolution: wontfix
Status: newclosed

comment:4 Changed 11 years ago by bill

Milestone: 1.41.3
Resolution: wontfix
Status: closedreopened
Summary: dijit.layout.ContentPane missing an addChild() methodContentPane: missing an addChild() method

I think I want to implement this after all, since it provides some performance enhancements (specifically, when ContentPanes which independently sized layout widgets, those layout widgets won't resize until the ContentPane is shown).

Continues the work from #5672.

comment:5 Changed 11 years ago by bill

Resolution: fixed
Status: reopenedclosed

(In [15974]) Refactor ContentPane? to inherit from _Container, and act as a _Container, even when there are multiple independently sized layout widgets as children.

This means that ContentPane? is repsonsible for calling startup() and resize() on it's children in order to get them to initialize and layout correctly.

getChildren() does what you would expect... didn't test addChild()/removeChild() but they should work, although behavior is unclear when ContentPane? has deep HTML rather than a simple list of widgets as children.

Fixes #7819 !strict.

comment:6 Changed 11 years ago by Nathan Toone

I've tried working with dijit._Container in this sort of environment (mixed html/child widgets) - and it tends to break at times. I'm guessing that for this case (merely wanting an "addChild" function) that the breakages aren't too heinous.

Basically, the _Container is designed under the assumption that it *ONLY* contains widgets - so some of the "next/previous" navigation within the container (as well as the "hasChildren" function) assume that all nodes within the container are actual widgets.

Like I said - it's probably not a huge issue - but something to remember and make note of.

comment:7 Changed 11 years ago by bill

(In [15978]) Test and fix addChild() for ContentPane? and AccordionContainer?. Refs #6842, #7819. !strict.

comment:8 Changed 11 years ago by bill

(In [16054]) Since ContentPane? inherits from _Contained now, don't mix it into ExpandoPane? again. Fallout from [15974], refs #7819 !strict.

comment:9 Changed 11 years ago by bill

(In [16189]) Widgets that extend ContentPane? shouldn't also extend _Container or _Contained, as those are already implemented by ContentPane?. Presentation still not working but this gets it closer (note that it wasn't working before [15974] either).

Refs #7819, #8321 !strict

comment:10 Changed 11 years ago by Nathan Toone

(In [16769]) Refs #7819 fix upgrade incompatibility where onLoad does not get called anymore !strict

comment:11 Changed 11 years ago by bill

Also (In [16183]) Refs #8321 - now that ContentPane? already inherits from _Container, we get errors if we inherit from it a second time !strict

comment:12 Changed 11 years ago by bill

(In [16904]) 1. Refactor [15607] (refs #6954, #7550, #7706, #7784, #7823) so that getDescendants() works as in 1.2 again, returning all descendant widgets, even nested widgets that are defined in templates. Expose the new functionality from [15607], to find direct descendants only, into a new _Widget.getChildren() method, rather than as flag to getDescendants().

This is because:

  • [15607] gave getDescendants() a subtle API change: it no longer found widgets inside of a dijit.Declaration (since no containerNode was defined)
  • because of that, Form was broken in that it didn't find form widgets inside of a custom widgets, declared with dijit.Declaration or dojo.declare, which didn't define this.containerNode
  1. Also, rolling back #7819: ContentPane?: missing an addChild() method (refs #7819), because of the backwards compatibility issue for custom widgets that extend ContentPane? and also extend _Container/_Contained.

Summary:

  • getChildren() is now supported by all widgets, and _Container widgets have (as before) an optimized implementation of getChildren() that overrides the one in _Widget.
  • ContentPane? not supporting any _Container methods except getChildren(). Users can set a ContentPane? child by calling attr('value', ...) etc.

!strict

comment:13 Changed 11 years ago by bill

(In [16911]) _LayoutWidget checks the parent.isContainer flag to detect if the parent will call resize() on it. Since ContentPane? doesn't extend _Container anymore, need to manually define that flag. (refs #7819 !strict)

TODO: probably _LayoutWidget should check for the isLayoutWidget flag rather than the isContainer flag.

comment:14 Changed 11 years ago by bill

(In [16913]) Fix comment. Refs #7819 !strict.

comment:15 Changed 11 years ago by bill

(In [16918]) Oops, trailing comma (refs #7819 !strict)

comment:16 Changed 11 years ago by bill

Milestone: 1.3future
Resolution: fixed
Status: closedreopened

comment:17 Changed 11 years ago by bill

(In [16962]) dojo.require("dijit._Container") no longer needed after [16904]. refs #7819, fixes #8831 !strict

comment:18 Changed 11 years ago by Adam Peller

(In [16994]) Tweak summary, fix some typos. Refs #7819, sorta.

comment:19 Changed 11 years ago by Adam Peller

(In [16995]) Tweak summary, fix some typos. Refs #7819, not really. !strict

comment:20 Changed 8 years ago by bill

Milestone: future1.8
Owner: set to bill
Status: reopenedassigned

comment:21 Changed 8 years ago by bill

Resolution: fixed
Status: assignedclosed

In [28276]:

Make ContentPane? extend _Container so that addChild()/removeChild() work for basic case.

Also updating _Container documentation (aka spec) to indicate that it can be a superclass for any widget with children, but that addChild()/removeChild() should only be used when an instance contains (nothing more than) a list of child widgets.

addChild() won't work well for ContentPane nested in layout widgets because it doesn't call resize(), since it's unclear if the ContentPane is visible or not, and resize() doesn't work on hidden tabs. It also doesn't call _checkIfSingleChild() to resize a single child widget to fill the whole ContentPane.

Finally, reducing code for _Container::_getSiblingOfChild().

Fixes #7819 !strict.

comment:22 Changed 7 years ago by bill

See also [29904], which makes the addChild() behavior probably more reasonable if the ContentPane contains mixed content.

Note: See TracTickets for help on using tickets.