[dojo-contributors] MVC support for dijit based on dojo.Stateful

Bill Keese bill at dojotoolkit.org
Fri Mar 4 22:33:10 EST 2011


Glad you are getting good feedback from Ben.   Here's a reply to some of the
stuff from my original note.


> > 2. DataBindingMixin
> >
> > a) file structure
> > The root of the patch is _DataBindingMixin.js, which gets mixed into
> > _WidgetBase.   Current code for that is split across two files as:
> >
> > dojo.declare("dijit._DataBindingMixin", null, { ... });
> >
> > dojo.extend(dijit._WidgetBase, new dijit._DataBindingMixin());
> >
> > It would be simpler like this:
> >
> > dojo.extend(dijit._WidgetBase, {...});
> >
> > You can still pull _DataBindingMixin.js into StatefulModel.js if you
> don't
> > want apps to need to explicitly dojo.require(DataBindingMixin).
> >
> <snap/>
>
> Correct, thats indeed the objective. The data binding mixin is not
> something we'd want apps to explicitly dojo.require(). By "pull into",
> I'm thinking you mean inline the source into the other file? Thats an
> option.
>


Sorry, I meant that you could list _DataBindingMixin in the list of requires
for StatefulModel, in the first line define() call.

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

Would StatefulModel ever be useful outside of widgets?    A recursive
dojo.Stateful sounds useful in general, not sure about the other stuff in
there.



> e) _initControl()
> >
> > This is run from postCreate(), yet it assumes that the widget is anchored
> to
> > the document.   You shouldn't assume that until startup() is called; in
> > particular it's not true for programatically created widgets ex:  new
> > dijit.form.TextBox().placeAt(...)
> >
> <snip/>
>
> OK, I'll look into (d) and (e) above.
>
>
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.

In your response to Ben you gave this example:

model:
 { foo : { bar : { baz : { freddy : { frog : "prince" } } } } }


markup:

 <div dojoType="dijit.mvc.Group" ref="model.foo">
 <div dojoType="dijit.mvc.Group" ref="bar">
  <div dojoType="dijit.mvc.Group" ref="baz">
   <div dojoType="dijit.mvc.Group" ref="freddy">
    <div dojoType="dijit.form.TextBox" ref="frog">


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?)

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.



> > 4. NumberTextBox, ValidationTextBox
> >
> > These methods are now assuming a super-class isValid() method, but that
> > method only exists when _DataBindingMixin is included.   I guess we need
> to
> > define an empty isValid() method in _WidgetBase?   I don't know what
> happens
> > when you call this.inherited(arguments) and there's no such super-class
> > function, but even if it works, it seems fragile.
> <snap/>
>
> Nothing happens, see line 190 (linking to a few lines before that for
> context):
> http://bugs.dojotoolkit.org/browser/dojo/trunk/_base/declare.js?rev=23778#L186
>
> That aside, yes, this was slightly tricky because there was no
> corresponding isValid() in _WidgetBase whereas I think there should
> be. Also, not all dijits check base class validity (where applicable)
> before deciding their isValid() status and once its added to
> _WidgetBase, they should (and thereby, the model validity will be
> accounted for in cases where the data binding mixin is applied).
>
>
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:

var oldIsValid = dijit._WidgetBase.isValid;
dijit._WidgetBase.isValid = function(){ return this.oldIsValid() &&
your-code-here; }



> > 5. dijit.mvc._Container
> >
> > This has >100 lines of cut-pasted code from _Templated and doesn't
> support
> > the new data-dojo-attach-point etc. syntax.   Can you do better?   See
> > dijit.Declaration, maybe you can have similar code.
> <snip/>
>
> Yes, its listed as a TODO in code -- it'd be good to remove that
> duplication. The issue as I remember was that I wasn't able to reuse
> _Templated directly as the way it deals with templates and template
> caches is different from, say repeat, where the template is inline
> (body). I'll take another look at dijit.Declaration, but ISTR some
> other differences there.
>


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?



> > b) the declaredClass references here and in other parts of the code
> aren't
> > future-proof.   declaredClass will likely go away in 2.0.
> <snap/>
>
> OK, any future-proof equivalent? I'll investigate or use a bit more of
> the duck typing sauce if needed.
>

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.



>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.dojotoolkit.org/pipermail/dojo-contributors/attachments/20110305/a0d68b50/attachment.htm 


More information about the dojo-contributors mailing list