request for review

[Colin Clark]

Hi Laurel,

We spoke a bit about this in person, but I wanted to follow up with a  
more detailed code review and some questions about how you imagine  
this component will be used in practice.

On 1-Dec-09, at 12:26 PM, Laurel A. Williams wrote:

> I have started work on a "menu component". I would like to request a  
> quick review of my work so far, which is simply to provide a  
> framework for rendering a fairly deep unordered list.
> The model I've provided only goes down 2 levels, but the idea is  
> there and it works.
>
> Pointers to better js and fluid technique would be much appreciated.

Watching your progress with the menu component, I've been assuming  
that this is a learning opportunity for you to get to know how to  
create components from scratch and use the Renderer. With this in  
mind, a few pointers:

  * Your component tree is mostly the same thing, duplicated over and  
over again. Take a look at how most components use fluid.transform()  
to iterate over a list of data and generate an appropriate tree. If  
you're not familiar with it, fluid.transform() is a pretty interesting  
function to learn, as it illustrates the elegance of functional  
programming idioms that aren't typically possible in languages like  
Java.

* I'd suggest not naming your IDs based on the type of DOM element  
they are bound to. For example, you've got IDs like "level0UL" and  
"level1LI". The whole point of a component tree is to provide a set of  
instructions for how to render a user interface in the abstract,  
independent of the specific type of markup in the template.

* Rather than hard-baking a particular depth of nesting, and creating  
a static cutpoints structure and tree, perhaps you could consider  
programmatically generating these so that there's less duplication?  
You might consider taking a look at the Table of Contents component as  
an example, since it generates very similar output as your menu  
component.

> JIRA: http://issues.fluidproject.org/browse/FLUID-3385
> Code: https://source.fluidproject.org/svn/scratchpad/menu/
>
> My next steps for this code will be to write unit tests for the  
> rendered menu and then to try to generate the component tree  
> programmatically from the model data.


On a more conceptual level, I'm having trouble getting my head around  
why a user might want to use this particular component. Quite  
earnestly, my first question is: what does this component *do*? I see,  
on a technical level, that it takes a model consisting of text and  
URLs, and renders a list of links. But what advantage does using this  
sort of model-driven, self-rendering component provide to a user?

Your JIRA mentions this from the perspective of our Web site, so  
perhaps that will be a clarifying use case. In general, Infusion  
components tend to provide some sort of dynamic interaction. We  
generally only use the Renderer when data changes frequently, is  
complex, or is provided from a third-party source such as a data feed  
or server. Do you imagine that our new Web site's navigation scheme  
will change frequently or dynamically in response to the user's  
interactions?

Another thing to consider: not all Infusion components use the  
Renderer. Many of them, such as Inline Edit, are "markup-driven," and  
offer enhanced behaviour on top of a bed of basic markup provided by  
the user. One of the key advantages to this approach is that we don't  
try to impose any kind of artificial model on top of things that can  
be expressed with plain old markup. Perhaps this style of component is  
more appropriate for our menu, assuming it has some kind of dynamic  
behaviour? I still can't quite imagine what this component will do,  
aside from Rendering some static markup once.

I apologize if I'm missing some feature that you've got planned down  
the road for this component. Based on the code I see today, it seems  
to me that plain old static HTML will probably solve this issue more  
simply and elegantly. If it's a question of avoiding duplicate markup  
within each page, perhaps you could investigate strategies for loading  
static fragments of markup via Ajax?

Thoughts?

Colin

---
Colin Clark
Technical Lead, Fluid Project
http://fluidproject.org