Hi Anastasia,

Nice work on this! I think this is a really good and clear illustration of how to use the Renderer in several common scenarios. Here are some code review comments, followed by specific responses to your questions inline:

In each of your "build subtree" functions, you seem to copy the treeChildren variable before adding things to it. I can't quite figure out why. Am I missing something, or is it just a bit of cruft left over from a refactoring?

In those functions, too, you can probably just return the tree array as you build it, rather than giving it a name and immediately returning it.

Given that you've nicely factored out each bit of tree generation into a separate function, maybe it makes sense to split out the stringsTree, too. What do you think?

Just for clarity's sake, it's probably worth organizing demo.render() into discrete sections, perhaps like this:

1. Setup the model (make the applier, register listeners, set up the options)
 2. Build the component tree
 3. Bind any event handlers/do setup code

The implementations of buildLocationSubtree() and buildWineListSubtree() are pretty much identical, except for the various ID names. Indeed, even buildCanapeList() shares the basic functionality of setting up the UISelect component. That said, I tried to refactor it out and found that it made for a more complicated demo. In my mind, this speaks to a couple of things:

* There should be more high-level functions for generating selections + inputs, etc, built into Infusion * Managing IDs in a component tree makes parameterization really awkward

These, again, are things we'll address in the next big Renderer release.

On 25-Sep-09, at 2:30 PM, Anastasia Cheetham wrote:
Given that the purpose of the demo is to illustrate some of the 'best practices' for working with the demo, I'd appreciate it if someone could look over the code and check whether or not I'm actually using any of the 'best practices' :-)
...
Regarding cutpoints: what is the 'right' way to use cutpoints? I'm essentially hard-coding them, but that doesn't seem like a 'best practice'.

I think you've used them correctly here. In the next release of the Renderer, we will integrate it with the DOM binder so that you can always just refer to selector names, abstracting out the mapping between IDs and selectors wherever possible.

I'd also appreciate comments on my use of the ChangeApplier to trigger the display of the changed model - will that complicate things?

I think it makes sense, and clearly illustrates how users can listen to automatic changes in the model. It's one line of code, so not complicated in my mind.

Finally: Does anyone know an easy way to pretty-print a JSON object? I'm looking around, but not finding any help.

Pretty printing would make a huge difference here. The library Jacob uses for the portal itself might do it, but I'm not sure.

I like seeing the raw model in this demo to see how autobinding works, but's pretty overwhelming as-is. Ideally, we might even highlight the sections in the model that change based on selections. Too much for this release?

Again, nice work!

Colin

---
Colin Clark
Technical Lead, Fluid Project
Adaptive Technology Resource Centre, University of Toronto
http://fluidproject.org

_______________________________________________________
fluid-work mailing list - [email protected]
To unsubscribe, change settings or access archives,
see http://fluidproject.org/mailman/listinfo/fluid-work

Reply via email to