DomHelper enhancement

[joshgr]

Hi Jack,

I'm a senior dev that's just coming up to speed with javascript and web-development. So far, I've been frustrated by the poor quality of most of the available javascript libs, so I'm relieved (and impressed!) by the attention to usability and maintainability that's apparent in your stuff. Nice job!

Ok, enough blubbering, here's my idea:

I'm very excited about your DomHelper API for HTML rendering. In fact, I recently developed an API that's very similar, consuming 'options' objects that are nearly identical to yours. I find that this configuration format (eg, {tag: 'div', cls: myClass, id: myId, children: [ child-nodes ]}, etc) is a very convenient and natural way to convey the markup, because it exposes the object-like nature of the DOM schema without bogging down in Element verbosity or quotation/concatenation gook. Basically, this type of API allows the dev to craft simple objects that concisely, clearly, and unambiguously describe the DOM objects s/he wishes to create, and that's a great thing. Furthermore, these 'renderCommand' objects can be built up programmatically, and saved and reused as templates -- more very nice features. I won't be surprised if ALL web layout is eventually performed like this -- why bother with noisy open/close tags when curly-braces work better?

However, your DomHelper implementation doesn't go as far as it could to expose the DOM schema via JSON objects. Specifically, the "class" and "style" attributes are still treated as raw text in your API. This leaves the preparation of the text for these attributes in the hands of the developer, requiring textual preprocessing of those values if they are programmatically determined.

I suggest enhancing the API to (optionally) accept an array of strings for the 'cls' field, and a sub-object (map) for 'style', like this:

{tag: 'div', cls: ['class1', 'class2', 'class3 class4'], style: {width: '100px', height: '200px'}}

This is easy to implement, and would ideally be backwards-compatible by inspecting the typeof the fields in question.

Whattaya think? (My apologies if this has already been discussed -- it's all new to me!)

-josh

[jack.slocum]

It's definitely possible and not difficult to implement.

It seems easier to write:

{tag: 'div', cls: 'class1 class2 class3 class4', style: 'width: 100px; height: 200px'}


than:

{tag: 'div', cls: ['class1', 'class2', 'class3 class4'], style: {width: '100px', height: '200px'}}


I have no problem with adding it, but can you tell me what the benefits are? Thanks.

[joshgr]

Sure thing. First, I agree that your version above is simpler and easier to write in the common case, and I definitely believe that the API should continue to support the textual style. But there are situations where logic is involved in computing the classes and styles to apply to an element. In these situations, it can be nice to be able to gradually build up/modify these parameters within the 'options' object itself.

The DOM objects break out the style attributes into their own sub-object, because they are easier to manipulate that way. Furthermore, most javascript libraries have utility methods for dealing with an element's class-names as a collection, because the space-delimited list is awkward. I think that supporting these more natural interaction styles would be a win for the users of your API.

I'm not suggesting that this is a big deal, but I do foresee the same text-manipulation logic cropping up constantly in many different applications. Why not centralize that logic in the rendering API and let the developer focus on the information they need to convey?

Here's a contrived example:

function getArticleRenderTemplate(article) {
var articleTemplate = {
tag: 'span',
html: article.text,
cls: ['article'],
style: {width: this.articleWidth + 'px'}
};

if (this.isTall()) {
articleTemplate.style.height = this.layoutOptions.tallRowHeight + 'px';
}

if (article.isSelected()) {
articleTemplate.cls.push('selectedArticle');
} else if (article.section.isCollapsed()) {
articleTemplate.style.display= 'hidden';
}

return articleTemplate;
}

Here's a more involved example, similar to something that I'm really doing in my app. I build several regions in my layout that all share some characteristics (their ids are constructed via a pattern, and their widths are computed based on their position in a structure). However, other attributes of the regions are quite variable.

I have a system that applies the common aspects (and defaults) to these regions in a method, while conveniently applying the region-by-region details in a layout structure. The result is code that I think is concise and fairly easy to read and manipulate. Doing this without the blown-out cls and style attributes would be quite a bit uglier, and (more importantly) would require the same awkward text manipulation that's likely to crop up in many other situations:


regionTemplate: function(regionType, options) {
if (!options) options = {};
if (!options.tag) options.tag = 'div';

options.id = regionType + this.rowId;

if (!options.style) options.style = {};
options.style.width = this.rowWidth + 'px'; // no semi-colon/concat logic required

options.cls = forceArray(options.cls); // returns [], [value], or value
options.cls.push(regionType); // don't need to remember spaces
return options;
},

buildRenderTemplate: function() {
var template = regionTemplate('rowbox', {
children: [
regionTemplate('rowheader'),
regionTemplate('rowtitle', {tag: 'span', html: this.rowTitle}),
this.spacerTemplate,
regionTemplate('rowbody', {
cls: [this.highlightClass],
style: {height: this.rowHeight},
children:
buildRowBodyTemplates()
}),
regionTemplate('rowfooter', cls: [this.footerClass])
]
)};

return template;
},



Thoughts?

[jack.slocum]

First, thanks for taking the time to explain it out for me. It makes sense so I will add it.

Jack

[Animal]

Why not add this capability:


Article.prototype =
{
getArticleRenderTemplate: function() {
var articleTemplate = {
tag: 'span',
html: article.text,
cls: this.getClass,
style: this.getStyle
};
return articleTemplate;
},

getStyle: function()
{
var result = "width:" + this.articleWidth + 'px';
if (this.isTall())
result += ";height:" + this.layoutOptions.tallRowHeight + 'px';
if (!article.isSelected())
result += ";display:hidden';
}

getClass: function()
{
var result = "article";
if (article.isSelected())
result += " selectedArticle';
return result;
}
};

[joshgr]

Of course, Animal, there are plenty of easy workarounds. For example, I'm currently constructing my templates with class-lists and style-objects, then post-processing them in bulk to replace everything with raw text prior to rendering. No big deal.

The point is that it's clumsy. Class-names are inherently a collection, and styles are a set of name/value pairs. Forcing the API user to produce the strings is pushing the string-manipulation work up the stack, so that it has to be duplicated in many places when it could be handled just once.

Compare mine to yours below. They're pretty similar, but mine has less quotation/concatenation gook, so I think it's easier to read (and write!). Also, mine would be much more amenable to CHANGING values that were already assigned (ie, to adjust the width from the default). Finally, if anyone prefers the string variant, they're welcome to continue doing it that way.

Cheers!

Mine:

getStyle: function() {
var result = {width: this.articleWidth + 'px'};
if (this.isTall())
result.height = this.layoutOptions.tallRowHeight + 'px';
if (!article.isSelected())
result.display = 'hidden';
}


Yours:

getStyle: function() {
var result = "width:" + this.articleWidth + 'px';
if (this.isTall())
result += ";height:" + this.layoutOptions.tallRowHeight + 'px';
if (!article.isSelected())
result += ";display:hidden';
}

[Animal]

Looks good. Name/value pairs for style is prob the best way to go, lists of individual Strings is best for class names.

I just like the idea of the template being able to accept function references to resolve the attribute values.

[joshgr]

My mistake, I misunderstood what you were suggesting (I thought the missing parens after this.getStyle were just a typo).

I'm not crazy about passing a function here... Not sure when this would be useful. Rendering instructions are just data; no behaviors involved (except perhaps the event-handlers -- onclick, etc -- where passing a function might actually be interesting!).

What I mean is that I can't imagine a situation where it would make a difference to just add parens to call the function in place, instead of passing the closure to the API unexecuted. I'm sure any example where this would make a difference could be refactored to not need it, and the result would probably be easier to understand.