expose functionality like dijit.layout.ContentPane and dojox.layout.ContentPane's setContent outside of dijit

[sfoster]

the ContentPane?'s hog some really useful functionality. They provide the most complete implementation of injecting html into a node - with widget parsing on the new content, script execution, style extraction/rendering, path correction etc.

If you want to use those facilities, the answer today is dijit / dojox. If your project isnt using dijit already, you are forced to load it.

I'd like to see an api like (names just for the sake of example) dojox.html.setContent(elm, htmlString); dojox.html.setContent(elm, nodeList); dojox.html.setContent(elm, nodeList, { parseContent: true, extractScripts: true});

..and so on. Eventually the implementation currently spread across the 2 ContentPanes? could call into this (if it could make it into base as dojo.setContent, or dojo.html.setContent)

(I've mentioned this idea a couple of times on #dojo, but couldnt find any ticket for it yet)

preview patch coming...

[bill]

Sounds fine. Would need to put it in dijit (or dojo) so that dijit.layout.ContentPane? can access it.

[sfoster]

adds dojox/html directory, with in-progress snapshot (inc. test file)

[sfoster]

The zip I attached (setContent_5647_20080123.zip) has dojox/html/content.js and dojox/html/tests/test_dojoContent.html

The test file takes the dojox.layout.ContentPane? test file and implements the same stuff using the dojox.html.setContent. For expediency's sake I make a new widget to standin for ContentPane?, whose setContent method calls dojox.html.setContent - that's just so I didnt have to rewrite so much in the tests. But there's nothing dijit/widget dependant here.

It passes 18/23 tests so far. I'm just working my way down and that's as far as I've got!

[sfoster]

update of the dojox.html snapshot, 2 tests failing still

[sfoster]

I added a setContent_5647_20080128.zip (cant seem to make a patch when the directory doesnt exist in the repo yet). Anyhow, that hooks up the adjustPaths etc. options. Just 2 tests are failing for me (FF).. getting closer. The failures might be just related to some idiosyncracies in the way the tests are written, still looking. This stuff is ready to play with though if you have an interest.

[sfoster]

dojo.setContent, dojo.NodeList?-setContent and some unit tests.

[sfoster]

the setContent_20080213.zip has my latest pass at this. I've been presumptive and moved a subset of the setContent functionality into dojo core, as dojo.setContent, and dojo.content._SetContentOperation. Also in there is a simple NodeList? extension showing one potential use. The unit tests in dojo/tests/content/test_setContent pass for me in FF2, Safari 3. I cant get them to run in IE - some doh issue? Anyhow, please take a look and get comments on names / style / functionality / etc.

I've not really doc'd this stuff yet, but its getting closer and I hope self-apparent for the purposes of this review.

[sfoster]

I didnt make clear that this "core" version just implements clean node content replacement with an extensible onBegin/setContent/onEnd lifecycle. And its got parseContent, extractContent options in there. The more advanced/exotic extractScripts / adjustPaths / renderStyles etc. are planned for a dojox extension of this (not included in the zip), by extending the dojo.content._SetContentOperation class. So what's here is pretty lightweight for the common use cases ~200 lines including comments. Setting this to the Core component - but I'm happy for it to live in dojox somewhere too if necessary.

[sfoster]

[CLA] [PATCH] dojo.html.set, dojo.NodeList?-html (refactored from earlier setContent implementations)

[sfoster]

the html_set_20080219.patch attachment incorporates some of the feedback I got around the api. It defines:

dojo.html.set(), dojo.html._SetContentOperation, dojo.NodeList?-html

and the previous unchanged static methods. One biggish change is that new dojo.html._setContentOperation(..) is now atomic (if that's the right word) in that it goes through the entire lifecyle from the constructor/postscript - i.e no need/opportunity to manually call the set() method. This turned out to be useful if you wanted to create _SetContentOperation-s from markup (or a subclass thereof), and in general seemed a good idea. The onBegin/set/onEnd methods give ample opportunity to extend (as was as the usual declare-d object lifecyle methods).

I've made a start on the docs - let me know how those look. Also tidied up some stylistic nits - ditto.

I updated the tests to include parser-instatiation, and fixed the nodelist test to actually test stuff.

I expect there's another iteration still here - please send me your feedback. If you can look ahead to how you might use this as well as just reviewing the code that's there - that would be great.

[sfoster]

Update to dojo.html.set + Nodelist-html + unit tests (all passing in FF2, IE6)

[sfoster]

The new patch: html_set_20080329.patch fixes/changes the following:

1. The class is now named dojo.html._ContentSetter
2. The _ContentSetter.set method is *not* called automatically like before
3. dojo.html.set() is changed accordingly, as are the unit tests - which now all pass! (for me)
4. The _ContentSetter has a tearDown method, which resets the instance to allow re-use of the object for a series of content-setting operations.
5. There's a unit test for the reuse scenario
6. Fewer outside dependencies in the tests
7. More docs

It doesnt change: the scope - this is still just the core dijit.layout.ContentPane? stuff, no extractScripts or the other bits found in dojox. That's coming another day

Where I'd like feedback:

1. Is the api still a little awkward?
2. Are the tests actually testing what needs to be tested. There are some edge cases like setting content on a form element that are not covered (in the tests or supported in the implementation thus far)
3. I've left a hook and a note to potentially flesh out the mixing-in of properties to allow more sensible roll-back between calls to set(). Currently it just deletes the node, parseResults and content properties, but it should/could also restore onBegin/onEnd etc. back to the created state (the params originally given to the ctor)
4. Any style, performance improvements. I do think its important to be able easily extend the _ContentSetter class, so I've resisted making things private or otherwise opaque to the developer-user

I'd like to get this into trunk so people can start using it and kick-off the feedback loop. At that point I'll also pick back up the other part to this ticket which is all the dojox.layout.ContentPane? stuff - renderStyles, extractScripts (#1 request) and so on. Or, perhaps we need a draft of that before the design is proven?

[jburke]

Sorry for the late review:

Note about multiversion support: dojo.html._ContentSetter's constructor call constructs an ID using a counter. It does not look like this ID is used as part of any element in the DOM, but if we think outside code will be dealing with these IDs, then it would be safer to use dojo._scopeName as part of the ID construction. Maybe id = dojo._scopeName + "Setter" + ....

I am also not sure about the shouldEmptyFirst argument on dojo.html._setNodeContent(). It seems like at least one code branch inside the _setNodeContent() will do just an innerHTML, so it does not seem like this param is useful in a consistent way? Seems like the function should always wipe out the children, since this is a "set" function and not an "add" function?

This is a larger question, maybe not sorted out immediately, but maybe dojo.html._emptyNode() should be public (just emptyNode)? I think dojo._destroyElement() should also be public too, so maybe sort that out with that discusssion. Maybe instead of emptyNode, something like destroyChildren? Hmm, naming is hard, but maybe relating it to dojo._destroyElement in some way.

dojo.html._setNodeContent uses a "cont" argument, but the docs say it is "content". I personally prefer content, and would rather see "content" in the places that "cont" is used in the patch.

dojo.html seems to require dojo.parser. This is needed for dojo.html._ContentSetter. I think it would be nice to get the dojo.html.set functionality without requiring the parser. Perhaps break out dojo.html._ContentSetter as a separate module file, with its own dojo.provide call.

I don't have good feedback on the _ContentSetter internals -- it seems to be geared for widget usage anyway? Maybe just move it to dijit.html._ContentSetter or just dijit._ContentSetter? What are use cases for using _ContentSetter outside a widget context?

[liucougar]

a performance improvement suggestion: for dom node with large number of children, the following should be faster (especially in IE) than the one in the patch:

dojo.html._emptyNode = function(/* DomNode */ node){ 
    var c=node.cloneNode(false);
    if(node.parentNode){
        node.parentNode.replaceChild(c,node);
    }
    dojo._destroyElement(node);
    return c;
}

[bill]

Checked in three months ago as [13259]: merging Sam Foster's html.set() patch + tests. Refs #5647. We can close when ContentPane? is refactored to use html.set()

[bill]

sfoster: ping. I really hope you can get ContentPane refactored before the 1.2 beta. As it is now, there's five pages of duplicated code in our repository, and the code paths are diverging. That's a really bad situation.

[sfoster]

(In [14961]) removed unused parseOnLoad alias for parseContent, fixed error in the fallback error handler which was refering to this.domNode. refs #5647

[sfoster]

[PATCH] [CLA] preview of dijit.layout.ContentPane? refactor to use dojo.html._ContentSetter

[sfoster]

[PATCH] [CLA] preview of dojox.html.set and dojox.layout.ContentPane? refactor to use dojox.html._ContentSetter

[sfoster]

the 2 new patches provide a snapshot of where I'm at with this.

I added dojox.html._base, which includes dojox.html.set - it ports over the extractScripts, renderStyles etc. goodies from dojox CP, as well as the tests.

dijit.layout.ContentPane? is fairly trivially refactored. I ended up using dojo.html._ContentSetter rather than calling the dojo.html.set convenience as it lets me instantiate and store the setter (as this._contentSetter) for repeated use during the CP's lifetime, and mixin properties into it.

Likewise with dojox.layout.ContentPane? - which is able to call the inherited _setContent method with its own dojox.html._ContentSetter as the _contentSetter, and the extended setter params.

TODO: Ideally the widgets would just use a mixin. The almost-mixinable ContentSetter? class is a tantalizing thought, but ultimately I think the mixin would need to basically be a proxy/adaptor - some of the property names arent a 1:1 match (e.g. CP has href, CS has referencePath, and we wouldnt want all of CS' guts mixed into the CP. That's my next steps.

If you have chance please play with this, run the tests ( dijit/tests/layout/test_ContentPane.html, dojox/html/tests/test_set.html, dojox/layout/tests/ContentPane.hmtl ) and respond with any thoughts on it all

[sfoster]

(In [14986]) Refs #5647 * Adds an empty() method, which empties the node and destroys any widgets in the parseResults array * Minor clean up in the test page

[sfoster]

(In [14987]) Refs #5647 Refs #2056 Refs #4980

Refactored _setContent to use dojo.html._ContentSetter * a dojo.html._ContentSetter instance is created and stored on first use (at this._contentSetter) * each subsequent call to _setContent remixes in the config properties * _ContentSetter.empty() is called implicitly when you call this._contentSetter.set() - this will destroy any widgets from a previous set operation - including Menus, Dialogs etc, as the contentSetter.parseResults array keep widget references. This is partly redundant with the destroyDescendents() call in _setContent(), but catches this corner case. It does not fix the case where widgets have been created by the parser from inlined, initial content

* added a few unit tests to the test page to verify this behavior !strict

[sfoster]

(In [14988]) Refs #5647 * dojox.html.metric is no longer implicitly loaded by dojox.html * new dojox.html._base /is/ implicitly loaded by dojox.html * dojox.html.set, and dojo.html._ContentSetter extend their dojo core counterparts, to add support for executeScripts, renderStyles options * tests largely adapted from dojox.layout.ContentPane?

[sfoster]

(In [14989]) Refs #5647

Refactored _setContent to use dojox.html._ContentSetter, the renderStyles and other private helpers have moved to dojo.html._base

* a dojox.html._ContentSetter instance is created and stored on first use (at this._contentSetter) * each subsequent call to _setContent remixes in the config properties * the _renderStyles method has been removed; the styleNodes array is now a property of this._contentSetter * minor jiggery with the test page, which was using some of the ContentPane? internals !strict

[sfoster]

(In [14990]) Refs #5647 Adding test case for destroying simple sub-widgets created by the parser, when we replace the content

[sfoster]

Looks good to me - please be on the watch out for regressions. All the tests pass in FF2, FF3, IE7, IE6, Safari 3, but there may be valid use cases not covered

[sfoster]

(In [14994]) Refs #5647, missing file from r14988, this is the actual dojox.html.set / dojox.html._ContentSetter implementation !strict

[sfoster]

(In [15171]) Refs #5647 Missing test file. This is pretty much a duplicate of the one used by dojox/layout/ContentPane/tests but as the tests are maintained seperately, I figured this should be too.