<font size="2">Glad you are getting good feedback from Ben. Here's a reply to some of the stuff from my original note.<br></font><br><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div><br>
> 2. DataBindingMixin<br>
><br>
> a) file structure<br>
> The root of the patch is _DataBindingMixin.js, which gets mixed into<br>
> _WidgetBase. Current code for that is split across two files as:<br>
><br>
> dojo.declare("dijit._DataBindingMixin", null, { ... });<br>
><br>
> dojo.extend(dijit._WidgetBase, new dijit._DataBindingMixin());<br>
><br>
> It would be simpler like this:<br>
><br>
> dojo.extend(dijit._WidgetBase, {...});<br>
><br>
> You can still pull _DataBindingMixin.js into StatefulModel.js if you don't<br>
> want apps to need to explicitly dojo.require(DataBindingMixin).<br>
><br>
</div><snap/><br>
<br>
Correct, thats indeed the objective. The data binding mixin is not<br>
something we'd want apps to explicitly dojo.require(). By "pull into",<br>
I'm thinking you mean inline the source into the other file? Thats an<br>
option.<br></blockquote><div><br></div><div><br></div><div>Sorry, I meant that you could list _DataBindingMixin in the list of requires for StatefulModel, in the first line define() call.</div><div><br></div><div>But I agree with Ben's comment, it seems wrong to make a dijit dependency on StatefulModel just so apps can avoid an extra dojo.require() call</div>
<div><br></div><div>Would StatefulModel ever be useful outside of widgets? A recursive dojo.Stateful sounds useful in general, not sure about the other stuff in there.</div><div><br></div><div><br></div><div><span class="Apple-style-span" style="border-collapse: collapse; font-family: monospace; font-size: 13px; "><div class="im" style="color: rgb(80, 0, 80); ">
<br></div></span></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><meta charset="utf-8"><span class="Apple-style-span" style="border-collapse: collapse; font-family: monospace; font-size: 13px; "><div class="im" style="color: rgb(80, 0, 80); ">
> e) _initControl()<br>><br>> This is run from postCreate(), yet it assumes that the widget is anchored to<br>> the document. You shouldn't assume that until startup() is called; in<br>> particular it's not true for programatically created widgets ex: new<br>
> dijit.form.TextBox().placeAt(...)<br>><br></div><snip/><br><br>OK, I'll look into (d) and (e) above.<br><div class="im" style="color: rgb(80, 0, 80); "><br></div></span></blockquote><div><br></div><div><meta charset="utf-8"><div>
About having every widget trace up the tree to find a parent with a binding specified, or if there's no such parent then tracing all the way to <body>... we faced that same problem with the bidi group and their lobbying for every widget to support a dir parameter (possibly) being inherited from an ancestor . We didn't want to have every single widget calling getParent() repeatedly to inspect it's ancestors as that seemed like it would degrade performance on page load. So we ended up adding code to the parser to pass down the dir inherited from the ancestor. I wonder if that makes sense here.</div>
<div><br></div><div>In your response to Ben you gave this example:</div></div><div><br></div><div>model:</div><div><meta charset="utf-8"><span class="Apple-style-span" style="border-collapse: collapse; font-family: monospace; font-size: 13px; "> { foo : { bar : { baz : { freddy : { frog : "prince" } } } } }<br>
<br><br></span></div><div><span class="Apple-style-span" style="border-collapse: collapse; font-family: monospace; font-size: 13px; ">markup:</span></div><div><span class="Apple-style-span" style="border-collapse: collapse; font-family: monospace; font-size: 13px; "><br>
<div dojoType="dijit.mvc.Group" ref="model.foo"><br> <div dojoType="dijit.mvc.Group" ref="bar"><br> <div dojoType="dijit.mvc.Group" ref="baz"><br>
<div dojoType="dijit.mvc.Group" ref="freddy"><br> <div dojoType="dijit.form.TextBox" ref="frog"><br></span></div><div><br></div><div><br></div><div>Hopefully that doesn't execute in O(n^2), where every widget needs to trace to the top of the tree, does it? I don't really understand the code that executes for the TextBox figures out that it's connected to model.foo.bar.baz.freddy.frog. (What info is cached inside of the innermost Group?)</div>
<div><br></div><div>Anyway, the parser has an "inherited" parameter that could be used to pass down the ref from the parent. I'm not sure if using it makes sense or not.</div><div><br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<span class="Apple-style-span" style="border-collapse: collapse; font-family: monospace; font-size: 13px; "><div class="im" style="color: rgb(80, 0, 80); "></div></span><div>> 4. NumberTextBox, ValidationTextBox</div><div>
><br>
> These methods are now assuming a super-class isValid() method, but that<br>
> method only exists when _DataBindingMixin is included. I guess we need to<br>
> define an empty isValid() method in _WidgetBase? I don't know what happens<br>
> when you call this.inherited(arguments) and there's no such super-class<br>
> function, but even if it works, it seems fragile.<br>
</div><snap/><br>
<br>
Nothing happens, see line 190 (linking to a few lines before that for<br>
context): <a href="http://bugs.dojotoolkit.org/browser/dojo/trunk/_base/declare.js?rev=23778#L186" target="_blank">http://bugs.dojotoolkit.org/browser/dojo/trunk/_base/declare.js?rev=23778#L186</a><br>
<br>
That aside, yes, this was slightly tricky because there was no<br>
corresponding isValid() in _WidgetBase whereas I think there should<br>
be. Also, not all dijits check base class validity (where applicable)<br>
before deciding their isValid() status and once its added to<br>
_WidgetBase, they should (and thereby, the model validity will be<br>
accounted for in cases where the data binding mixin is applied).<br>
<div><br></div></blockquote><div><br></div><div>OK, adding it to _WidgetBase is something to consider. But again, there would be the "problem" where your mixin is simply overwriting the isValid()method in _WidgetBase.js, like it's overwriting postCreate() now. The isValid() problem is harder to solve since dojo.connect() doesn't work since you need to return a value, I guess you should be doing something like:</div>
<div><br></div><div>var oldIsValid = dijit._WidgetBase.isValid;</div><div><meta charset="utf-8">dijit._WidgetBase.isValid = function(){ return this.oldIsValid() && your-code-here; }</div><div><br></div><div><br></div>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div>
<br>
> 5. dijit.mvc._Container<br>
><br>
> This has >100 lines of cut-pasted code from _Templated and doesn't support<br>
> the new data-dojo-attach-point etc. syntax. Can you do better? See<br>
> dijit.Declaration, maybe you can have similar code.<br>
</div><snip/><br>
<br>
Yes, its listed as a TODO in code -- it'd be good to remove that<br>
duplication. The issue as I remember was that I wasn't able to reuse<br>
_Templated directly as the way it deals with templates and template<br>
caches is different from, say repeat, where the template is inline<br>
(body). I'll take another look at dijit.Declaration, but ISTR some<br>
other differences there.<br></blockquote><div><br></div><div><br></div><div>This inheritance from _Container to both Group and Repeater confuses me. While Group is just a plain wrapper widget, similar to dijit.form.Form or dijit.layout.ContentPane, Repeater is a different beast that generates from a template. I guess my question is, why does mvc._Container have any code about templates?</div>
<div> </div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div>> b) the declaredClass references here and in other parts of the code aren't<br>
> future-proof. declaredClass will likely go away in 2.0.<br>
</div><snap/><br>
<br>
OK, any future-proof equivalent? I'll investigate or use a bit more of<br>
the duck typing sauce if needed.<br></blockquote><div><br></div><div>Yes, if there isn't an obvious way to ducktype (ex: testing if a certain method like then() is available), then in dijit for example we have isContainer and isLayoutContainer flags on widgets that indicate they have a certain functionality.</div>
<div><br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div><br></div></blockquote></div><br>