Opened 8 years ago

Closed 8 years ago

#13481 closed enhancement (fixed)

MVC - dojox/mvc/Repeat not usable programmatically

Reported by: ben hockey Owned by: rahul
Priority: high Milestone: 1.7
Component: DojoX MVC Version: 1.7.0b1
Keywords: Cc: Ed Chatelain
Blocked By: Blocking:

Description

dojox/mvc/Repeat has 2 issues i can immediately see that make it unusable programmatically.

  1. it does not cater for the case where a templateString is passed via the constructor rather than via the innerHTML of srcNodeRef
  2. it relies on a declarative instantiation for its contents to be parsed. ie, it does not parse its own contents but relies on the parser to instantiate its contents at the same time as it is parsed.

i'm attaching a modified version of test_mvc_search-results-repeat-store.html that adds 2 use cases that i think need to be supported.

  1. programmatic instantiation of dojox/mvc/Repeat
  2. subclassing dojox/mvc/Repeat to predefine the templateString

the 2nd use case can be supported via the following patch

  • Repeat.js

     
    4040                //              Override and save template from body.
    4141                postscript: function(params, srcNodeRef){
    4242                        this.srcNodeRef = dojo.byId(srcNodeRef);
    43                         if(this.srcNodeRef){
     43                        if(this.srcNodeRef && !this.templateString){
    4444                                this.templateString = this.srcNodeRef.innerHTML;
    4545                                this.srcNodeRef.innerHTML = "";
    4646                        }

i haven't looked into how to support the first use case.

Attachments (2)

test_mvc_search-results-repeat-store.html (5.6 KB) - added by ben hockey 8 years ago.
mvc-13481.patch (24.3 KB) - added by Ed Chatelain 8 years ago.
Updated patch with doh test in addition to the fix for declarative Repeat with a subclassing dojox/mvc/Repeat to predefine the templateString, and the fix for the programmatic Repeat.

Download all attachments as: .zip

Change History (11)

Changed 8 years ago by ben hockey

comment:1 Changed 8 years ago by rahul

I'd say this is an enhancement, not a defect. Repeat's origins are in declarative, patterned UIs and we don't claim full programmatic support (though things like ref can be updated programmatically).

Clearly, this is a useful enhancement. With respect to how to support the first use case, perhaps the second use case and your patch offers a hint i.e. if templateString is passed in via the constructor, then thats a reasonable hint for repeat that it needs to parse its own contents.

comment:2 Changed 8 years ago by ben hockey

Type: defectenhancement

without programmatic support i'll leave Repeat alone then. what other components are not supported programmatically? given that i think most users will expect MVC to work in all the same ways that dijit works i would say that it should be clearly documented which use cases are not yet fully supported.

fwiw, i have built a dependency inversion (IOC) container that instantiates widgets programatically and so nothing is declarative apart from custom templated widgets. it also seems that declarative use in templated widgets is not fully supported and has not been tested at all.

updating the ticket as an enhancement.

comment:3 Changed 8 years ago by rahul

You could leave it alone or you could improve it. Once again, I think it would be a useful improvement and logical next step.

What is supported for repeat is what is being tested. Agree with documenting -- I started with the dojox.mvc landing page on docs.dojocampus for that purpose, the intent is to fill in the rest of the pages for the features (when we get to Repeat, current state of impl will be doc'ed).

WRT templated widgets, I think you are referring to #13263 which you previously opened. I see some comments on it that seem to indicate it will be looked at.

comment:4 Changed 8 years ago by rahul

P.S. On last post: Ofcourse, all help with wiki docs is welcomed.

comment:5 Changed 8 years ago by Ed Chatelain

I am sorry I had not worked on this sooner. FWIW I would consider it a bug. I have a fix for most of what you want, it is actually just a one line change in Repeat, to support programatic creation, but I am having a problem using a declarative Repeat with a subclassing dojox/mvc/Repeat to predefine the templateString. For some reason it is not getting the templateString set to the predefined value. If any of you see why that is happening please let me know.

I will add my patch which has the update to Repeat and a new testcase. I will add a doh test once I figure out this last problem I am seeing.

comment:6 Changed 8 years ago by ben hockey

i'm in no rush for this - even if you do make a change i probably won't even get to look at it for a few weeks so just do whatever suits your own needs for now.

comment:7 Changed 8 years ago by Ed Chatelain

I found the problem with using a declarative Repeat with a subclassing dojox/mvc/Repeat to predefine the templateString, the templateString was being overwritten in Repeat's postscript so I had to add another check to see if templateString had been set.

Changed 8 years ago by Ed Chatelain

Attachment: mvc-13481.patch added

Updated patch with doh test in addition to the fix for declarative Repeat with a subclassing dojox/mvc/Repeat to predefine the templateString, and the fix for the programmatic Repeat.

comment:8 Changed 8 years ago by Chris Mitchell

Resolution: fixed
Status: newclosed

In [26805]:

fixes #13481 bug with mvc programmatic repeat

comment:9 Changed 8 years ago by bill

Milestone: tbd1.7
Note: See TracTickets for help on using tickets.