Add MVC support to bind to multiple attributes (in addition to value).

[Ed Chatelain]

dojox.mvc in Dojo 1.7 supports “value” attribute in widget side and “value” property in data-model side (dojox.mvc.StatefulModel?/dojo.Stateful). This works very well with form widgets (dijit.form.*), as most of them use “value” attribute to keep track of the value.

On the other hand, there has been a desire to support more (actually, any) widgets for data binding. It crates a need to support widget attributes beyond “value”. For example:

dijit.form.CheckBox? uses “checked” attribute for its checked/unchecked state. dijit.form.Button uses “label” attribute for its text, and “iconClass” attribute for its icon. dijit.MenuItem? uses “disabled” attribute for its enabled/disabled state, “selected” attribute for its selection state, “active” attribute for its mouse/key press state, etc. It also uses “label” and “iconClass” attributes like dijit.form.Button.

We should also add "wildcard" support for binding all matching properties at once, and "convert" and "direction" support as well.

[Ed Chatelain]

Patch with the multiattribute, convert, wildcard and direction support. Thanks to Akira Sudoh (IBM, CCLA) for his work on this.

[cjolif]

In [27695]:

refs #14681. Add multi-attribute binding (might need to be cleaned-up a bit in a subsequent commit to get it works only with one syntax for simplicity reasons), wildcard & direction support. As well some additional very small clean-ups. Thanks edchat & Akira Sudoh (IBM, CCLA) for this patch. !strict.

[Ed Chatelain]

I missed a testcase in the last patch.

[cjolif]

In [27698]:

refs #14681. Missing test case. Thanks edchat.

[ben hockey]

could we remove the null check from the initial copy?

[Ed Chatelain]

Hi Ben, where is the null check you would like to have removed? We are working on another large set of updates in github, so I will have to make sure it is updated there. The next drop should happen in the next day or so. Thanks

[Ed Chatelain]

We are in the process of migrating the old testcases to the new api, so hopefully we will catch any bugs which had been reported and fixed previously.

[ben hockey]

hi ed,

the change is already in a branch in github (​https://github.com/asudoh/dojox_mvc/commit/b50d4b8d5a6c369d7a1fe6f082c83b39c88079b2) which is why i was making sure it was coming here too. it should probably be a little bit different though because of the way copy works. once the check for null is gone, the initial call to copy should maybe be something like this (change 6th arg to undefined) for it to work:

copy(parseFunc, target, targetProp, source, sourceProp, undefined, value);

this has the effect that something with an undefined value will not do an initial copy but null will be copied. there are times where i'm intentionally setting initial values to null and i want that to propagate when i first bind.

also, in bindTwo, direction is missing a var declaration

[cjolif]

In [27904]:

refs #14681. Reorganize test files. !strict.

[Ed Chatelain]

Ben I will work on the updates for the null checks in BindTwo? after the current set of updates is completed to get the latest code from github into svn.

[cjolif]

In [27916]:

refs #14681. Updates for multiple attribute support, along with array support & follow ups on tests re-organization. Thanks edchat & Akira Sudoh (IBM, CCLA). !strict.

[cjolif]

In [27917]:

refs #14681. New directories for more tests content to come.

[cjolif]

In [27932]:

refs #14681. More tests + some clean ups (including removing some svn:executable properties on js files!). Thanks edchat (IBM, CCLA). !strict.

[cjolif]

In [27935]:

refs #14681. More clean-ups. Thanks edchat (IBM, CCLA). !strict.

[Ed Chatelain]

The next set of updates for dojox.mvc. Includes many updates for better controller support and additional tests for the new mvc APIs.

[cjolif]

In [27968]:

refs #14681.
1/ Have all controller inheriting _Controller and make mixin versions to be used with widgets
2/ Have getStatefulOptions by default
3/ various other changes
!strict.

[Ed Chatelain]

Controller name update, next patch will rename BindTwo? to sync.

[cjolif]

In [27980]:

refs #14681. Remove the duality between regular class & Dijit mixins by merging both. Thanks edchat & Akirah Sudoh (IBM, CCLA). !strict.

[cjolif]

In [27981]:

refs #14681. Rename BindTwo? -> sync after dicussion with Ed & Ben. !strict.

[Ed Chatelain]

Next set of updates from work done in github, added many testcases and inline comments and examples, in addition to code updates and bug fixes - patch 1 of 2..

[Ed Chatelain]

Next set of updates from work done in github, added many testcases and inline comments and examples, in addition to code updates and bug fixes - patch 2 of 2..

[cjolif]

In [28174]:

refs #14681. Next set of updates from github. Mostly testcaes & inline documentation. Also added EditStoreRefListController? (mixin of EditStoreRef? & ListController?) as well as equals to compare models. Thanks Akira Sudoh & Ed Chatelain (IBM, CCLA). !strict.

[cjolif]

In [28175]:

refs #14681. Tests for the latest features. Thanks Akira Sudoh & Ed Chatelain (IBM, CCLA).

[ben hockey]

is anyone reviewing these changes?

i know we don't have a formal review process with dojo but with very little effort i keep finding small issues that cause me to wonder what i might find if i had the time to look more thoroughly. most of the problems i find are arguably minor but they lead me to doubt the quality of the code.

i'm happy that all my concerns are addressed quickly but there is a lot of code being added and i'm concerned that the problems i'm able to see easily might only be the tip of the iceberg.

[Ed Chatelain]

Hi Ben, yes it is being reviewed by cjolif before he commits, but I guess the review is not as close as it should be due to the large number of changes. Sorry about the large drops of code, we will try to keep them smaller from now on, so that it will be easier to review the changes better. Do you have specific areas you think we need to take a closer look at? Thanks

[cjolif]

Replying to neonstalwart: Ben, thanks for your interest in this. I'm doing a high level review of this code both when it lands on github and when i'm committing it here. I do not go in a thorough review of looking for small bugs here and there but as you said I don't think we have process for that. That said I trust Ed and Akira for using the remaining time until the release (3 months?) to track down small issues and fix them. If you have time for more thorough reviews than I do I guess you should look at the github to get advance warning of what's coming. As a final note I would add that dojox/mvc probably have nothing to be blamed for compare to most of other dojox modules in term of quality!

[ben hockey]

ed - the few comments i've already made on github at (​https://github.com/edchat/dojox_mvc/pull/29) are all i have for now. i've only skimmed the code.

christophe - poor quality in existing code does not justify poor quality in new code. that said, i'll give ed and akira time to find issues and try to withhold my comments until closer to the release. btw, currently the tests don't run due to a couple of small typos (see #15011). do you include running the tests as part of your high level review?

[Ed Chatelain]

Ben all of the tests ran fine for me on Chrome on Windows. But I should have caught that and gotten the fix for 15011 in, sorry I missed it. I will fix that now. Thanks,

[ben hockey]

ed, yes, the tests will pass on chrome after #15011 is fixed but did christophe notice that the tests wouldn't load when he reviewed the code?

once #15011 is fixed, you'll notice there is a failure in IE for dojox.mvc.tests.doh_mvc_mobile-demo - i only tried IE7 & 8 and i'm not sure if it's expected or not. it seems the inputs with id="nameInput${this.index}" don't get created and the "Repeat Data Binding Example page" tests fail.

[cjolif]

Ben, I would have hoped that you did not think I wanted to justify poor quality with poor quality! I wanted to raise the fact that in my mind even if you seem to think differently dojox/mvc has a relatively good quality (even if certainly not perfect I agree) and that most other dojox modules have not that level of quality so blaming dojox/mvc in particular sounded a bit unfair to me even if obviously errors have been made! How many dojox projects even in the most used ones have DOH tests running? So it sounded strange to me to blame the quality of one of the most dojox tested project?

When it comes to running tests, I was usually running the full suite when I started the first commits on this work and was never getting issues so I started to trust more and more Ed and Akira and started just running a few tests instead of the full test suite to save time considering Ed did the testing before. Errare humanun est, from now on I will run the full test suite!

[Ed Chatelain]

This is done.