<font size="2">Glad you are getting good feedback from Ben.   Here&#39;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>
&gt; 2. DataBindingMixin<br>
&gt;<br>
&gt; a) file structure<br>
&gt; The root of the patch is _DataBindingMixin.js, which gets mixed into<br>
&gt; _WidgetBase.   Current code for that is split across two files as:<br>
&gt;<br>
&gt; dojo.declare(&quot;dijit._DataBindingMixin&quot;, null, { ... });<br>
&gt;<br>
&gt; dojo.extend(dijit._WidgetBase, new dijit._DataBindingMixin());<br>
&gt;<br>
&gt; It would be simpler like this:<br>
&gt;<br>
&gt; dojo.extend(dijit._WidgetBase, {...});<br>
&gt;<br>
&gt; You can still pull _DataBindingMixin.js into StatefulModel.js if you don&#39;t<br>
&gt; want apps to need to explicitly dojo.require(DataBindingMixin).<br>
&gt;<br>
</div>&lt;snap/&gt;<br>
<br>
Correct, thats indeed the objective. The data binding mixin is not<br>
something we&#39;d want apps to explicitly dojo.require(). By &quot;pull into&quot;,<br>
I&#39;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&#39;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); ">
&gt; e) _initControl()<br>&gt;<br>&gt; This is run from postCreate(), yet it assumes that the widget is anchored to<br>&gt; the document.   You shouldn&#39;t assume that until startup() is called; in<br>&gt; particular it&#39;s not true for programatically created widgets ex:  new<br>
&gt; dijit.form.TextBox().placeAt(...)<br>&gt;<br></div>&lt;snip/&gt;<br><br>OK, I&#39;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&#39;s no such parent then tracing all the way to &lt;body&gt;...    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&#39;t want to have every single widget calling getParent() repeatedly to inspect it&#39;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 : &quot;prince&quot; } } } } }<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>
 &lt;div dojoType=&quot;dijit.mvc.Group&quot; ref=&quot;model.foo&quot;&gt;<br> &lt;div dojoType=&quot;dijit.mvc.Group&quot; ref=&quot;bar&quot;&gt;<br>  &lt;div dojoType=&quot;dijit.mvc.Group&quot; ref=&quot;baz&quot;&gt;<br>
   &lt;div dojoType=&quot;dijit.mvc.Group&quot; ref=&quot;freddy&quot;&gt;<br>    &lt;div dojoType=&quot;dijit.form.TextBox&quot; ref=&quot;frog&quot;&gt;<br></span></div><div><br></div><div><br></div><div>Hopefully that doesn&#39;t execute in O(n^2), where every widget needs to trace to the top of the tree, does it?    I don&#39;t really understand the code that executes for the TextBox figures out that it&#39;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 &quot;inherited&quot; parameter that could be used to pass down the ref from the parent.  I&#39;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>&gt; 4. NumberTextBox, ValidationTextBox</div><div>

&gt;<br>
&gt; These methods are now assuming a super-class isValid() method, but that<br>
&gt; method only exists when _DataBindingMixin is included.   I guess we need to<br>
&gt; define an empty isValid() method in _WidgetBase?   I don&#39;t know what happens<br>
&gt; when you call this.inherited(arguments) and there&#39;s no such super-class<br>
&gt; function, but even if it works, it seems fragile.<br>
</div>&lt;snap/&gt;<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 &quot;problem&quot; where your mixin is simply overwriting the isValid()method in _WidgetBase.js, like it&#39;s overwriting postCreate() now.  The isValid() problem is harder to solve since dojo.connect() doesn&#39;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() &amp;&amp; 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>
&gt; 5. dijit.mvc._Container<br>
&gt;<br>
&gt; This has &gt;100 lines of cut-pasted code from _Templated and doesn&#39;t support<br>
&gt; the new data-dojo-attach-point etc. syntax.   Can you do better?   See<br>
&gt; dijit.Declaration, maybe you can have similar code.<br>
</div>&lt;snip/&gt;<br>
<br>
Yes, its listed as a TODO in code -- it&#39;d be good to remove that<br>
duplication. The issue as I remember was that I wasn&#39;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&#39;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>&gt; b) the declaredClass references here and in other parts of the code aren&#39;t<br>
&gt; future-proof.   declaredClass will likely go away in 2.0.<br>
</div>&lt;snap/&gt;<br>
<br>
OK, any future-proof equivalent? I&#39;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&#39;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>