Opened 11 years ago

Closed 10 years ago

#7802 closed enhancement (fixed)

add a dojo.createElement method and shorter variant

Reported by: alex Owned by: dante
Priority: blocker Milestone: 1.3
Component: Core Version: 1.2beta
Keywords: Cc: dante, James Burke
Blocked By: Blocking:

Description

Since before 0.9 there has been a FIXME in dojo/_base/html.js for adding a node construction API to simplify the document.createElement() API. At a minimum, the API should handle a data structure similar to the one passed to dojo.attr().

An initial version of this was checked in at [15411]. Add tests, expand docs, and ensure that the API is correct before closing this bug.

Attachments (5)

element.2.patch (16.6 KB) - added by dante 10 years ago.
updated patch includes support for nodeRef passing, and inadvertantly a style cleanup in html.js
element.patch (9.7 KB) - added by dante 10 years ago.
newest, cleanest. from trunk as of 16161
createElement.txt (46.0 KB) - added by dante 10 years ago.
grep of all document.createElement refs (some invalid)
dijit-create.patch (8.2 KB) - added by dante 10 years ago.
part I of dijit pass
create.html (1.0 KB) - added by dante 10 years ago.
test IE + create(n, { style:{opacity:x } }, ref)

Download all attachments as: .zip

Change History (28)

comment:1 Changed 11 years ago by alex

Status: newassigned

comment:2 Changed 11 years ago by alex

(In [15420]) removing bad impls of createElement() and elem(). Refs #7802. Refs #3833. !strict

comment:3 Changed 11 years ago by Eugene Lazutkin

Cc: Eugene Lazutkin added

comment:4 Changed 11 years ago by Eugene Lazutkin

The relevant thread from dojo-dev: http://thread.gmane.org/gmane.comp.web.dojo.devel/9099. Notable posts:

Basically three solutions were discussed:

  1. A function that combines three things:
    1. Creates an element.
    2. Optional: sets attributes using dojo.attr() (which calls dojo.style() if necessary).
    3. Optional: places it using dojo.place().
  2. A function that creates an element and returns a NodeList. The idea is to use the existing machinery and familiar API of NodeList.
  3. A function that creates an element and returns a node wrapper that emulates a NodeList but without an overhead of the unnecessary Array instance.

Obvious advantage of the 2nd and 3rd proposals is the advanced flexibility through chainability. Obvious downside is creating an extra object, which can be potentially heavy.

Following names were proposed for such function:

  • dojo.createElement()
  • dojo.createNode()
  • dojo.create()
  • dojo.element()
  • dojo.elem()

It looks like dojo.element() was the preferred choice with dojo.elem() close second. (e.g., see http://article.gmane.org/gmane.comp.web.dojo.devel/9219).

Following implementations were tested. The "Simple" version:

dojox.html.dom.createElement = function(
  /*String|Node*/ tag, /*Object?*/attrs,
  /*Node?*/ parent, /*String?*/ pos
){
  var node = typeof tag == "string" ? 
    (tag.charAt(0) == "#" ? dojo.byId(tag.substr(1)) : dojo.doc.createElement(tag)) :
    tag;
  if(node){
    if(attrs){
      dojo.attr(node, attrs);
    }
    if(parent){
      dojo.place(node, parent, pos);
    }
  }
  return node;    // Node
};

The function can accept "tag", "#id" (prefixed with #), or a node, serving as a shortcut for setting attributes, styles, and placement. All parameters after the first one are optional, or can be null/undefined.

The "NodeWrap" version (see for details of NodeWrap in the attached file):

dojox.html.dom.element = function(
  /*String|Node*/ tag, /*Object?*/attrs,
  /*Node?*/ parent, /*String?*/ pos
){
  var node = typeof tag == "string" ? 
    (tag.charAt(0) == "#" ? dojo.byId(tag.substr(1)) : dojo.doc.createElement(tag)) :
    (tag instanceof NodeWrap ? tag.node : tag);
  if(node){
    if(attrs){
      dojo.attr(node, attrs);
    }
    if(parent){
      dojo.place(node, parent, pos);
    }
  }
  return node;    // Node
};

There are two small differences with the "Simple" version:

  • It can accept a NodeWrap.
  • It wraps node when returning.

The "NodeList" version:

dojox.html.dom.elem = function(
  /*String|Node|NodeList*/ tag
){
  if(tag instanceof dojo.NodeList){
    return tag; // NodeList
  }
  var node = typeof tag == "string" ? 
    (tag.charAt(0) == "#" ? dojo.byId(tag.substr(1)) : dojo.doc.createElement(tag)) : tag,
    t = new dojo.NodeList();
  t.push(node);
  return t;    // NodeList
};

Following tasks were benchmarked:

  • Create a "span" node.
  • Add a custom attribute named "custom" with payload "red".
  • Set the text color to "blue".
  • Set innerHTML to "<em>Hello world!</em>".
  • Append it to a "host" node.

All snippets included a clearing of a host node after appending.

The "Simple" snippet:

c("span", {
  custom: "red",
  style: {color: "blue"},
  innerHTML: "<em>Hello world!</em>"
}, host);

The "NodeWrap" snippet:

c("span").
  attr({
    custom: "red",
    style: {color: "blue"},
    innerHTML: "<em>Hello world!</em>"
  }).
  place(host);

As you can see this version "cheats" by doing the bulk of work using dojo.attr() and dojo.place() directly available on NodeWrap. It was possible to split off a call to dojo.style(), but unlike NodeList we don't provide addContent() for stand-alone nodes --- we have to do it with dojo.attr() anyway.

The "NodeList" version:

c("span").
  addContent("<em>Hello world!</em>").
  attr({custom: "red"}).
  style({color: "blue"}).
  place(host);

This version is the smallest one.

The above snippet uses one call for every distinctive property. addContent(), attr(), and style() can be rolled in one attr() call. Still this mode was requested by chaining addicts.

Each snippet was run 1000 times, the test was performed 10 times, the median value was used to calculate the speed up/slow down. The median value gives the most consistent results on all platforms. All values below are 1-based percents from the smallest value (1.05 means this option was 5% slower than the fastest option).

BrowserSimpleNodeWrapNodeList
Firefox 3.0.31.001.042.36
Opera 9.60b11.001.093.43
WebKit (Midori 0.0.21)1.001.102.95
Google Chromium b211.001.056.72

These numbers were taken on Linux computer with a quad-core processor.

Based on these results it was proposed to implement the "Simple" version of the function in the base addressing possible deficiencies in dojo.attr(), dojo.place(), and dojo.style() respectively treating dojo.element() as a lean wrapper for these functions.

While the "NodeList" version is much slower compared with other versions, it still may make sense to add it as a shortcut for:

dojo.query(dojo.element(tag))

The "NodeWrap" version appears to be good performance-wise, but it has following things against it:

  • It requires extra code for NodeWrap class.
  • NodeList should be refactored to remove duplication in common with NodeWrap.
  • The concept should be thoroughly documented.
  • The extra concept to learn for newbies.

In the course of discussion following things were noted or remarked:

  • Pete proposed to add a special parsing to dojo.query() to treat dojo.query("<p>Hello!</p>") as a node creation. But there is an opposition: "dojo.query() is unexpected place to create nodes".
  • A suggested NodeList improvement: add destroy() method that will delete and destroy nodes.
  • Create a separate function that can instantiate a fragment. It will go to Dojo Core. See related ticket #7890 for details.
  • dojo.place() bugs were found (#7546, #7547) and fixed subsequently.
  • Dojo code contains following number of node creation/attribute setting/node placing sequences:
    • Dojo: 37 times
    • Dijit: 44 times
    • DojoX: 188 times (excluding xmpp)
  • There was a general agreement to target 1.3.
  • More suggested NodeList improvements:
    • Factor out the event processing code. Right now the long list of events is an internal variable that has to be replicated if we need it for some other reason. See #7974.
    • Factor out addContent() method --- we want to reuse it for single nodes too. This way we can fix/improve things in one place. See #7890.

comment:5 Changed 11 years ago by Eugene Lazutkin

Correcting a cut-n-paste mistake in my previous comment:

dojox.html.dom.element = function(
  /*String|Node*/ tag, /*Object?*/attrs,
  /*Node?*/ parent, /*String?*/ pos
){
  var node = typeof tag == "string" ? 
    (tag.charAt(0) == "#" ? dojo.byId(tag.substr(1)) : dojo.doc.createElement(tag)) :
    (tag instanceof NodeWrap ? tag.node : tag);
  if(node){
    if(attrs){
      dojo.attr(node, attrs);
    }
    if(parent){
      dojo.place(node, parent instanceof NodeWrap ? parent.node : parent, pos);
    }
  }
  return new NodeWrap(node);    // NodeWrap
};

I already mentioned that it takes a NodeWrap as the "node" parameter, and returns a NodeWrap. Additionally it can take a NodeWrap object as the "parent" parameter, making it functionally complete.

comment:6 Changed 10 years ago by dante

Priority: normalhighest

I do this in about any page I want repetitive DOM manip:

// simple "advanced" create function
	dojo.element = function(node, attr, ref, pos){ 
		var n = dojo.doc.createElement(node); 
		if(attr){ dojo.attr(n, attr); }
		if(ref){ dojo.place(n, ref, pos); }
		return n;
	}

This is in line with "simple" above, skips "advanced" markup creation all together (use innerHTML attr) and allows for placement safely inline. -1 on looking for # as the tag in the creation string, it seems a little counter intuitive to me. I would be happy to put this in _base/html.js and document myself with docs and tests, and start the triage through the codebase to use this new function where appropriate.

changing the priority of this to reflect the importance of completion before 1.3

comment:7 Changed 10 years ago by dante

(it also prevents us from having to support complex table markup situations in base, deferring that to dojo.html.set)

comment:8 Changed 10 years ago by dante

Cc: phiggins removed

comment:9 Changed 10 years ago by dante

touching to ping cc'd folks. Added implementation and unit test patches. I'm +1 on this as-is, but want to confirm with everyone before committing.

comment:10 Changed 10 years ago by Eugene Lazutkin

Owner: changed from alex to Eugene Lazutkin
Status: assignednew

Changed 10 years ago by dante

Attachment: element.2.patch added

updated patch includes support for nodeRef passing, and inadvertantly a style cleanup in html.js

Changed 10 years ago by dante

Attachment: element.patch added

newest, cleanest. from trunk as of 16161

comment:11 Changed 10 years ago by Eugene Lazutkin

Cc: Eugene Lazutkin removed
Status: newassigned

comment:12 Changed 10 years ago by Eugene Lazutkin

#7975 is the ticket for legalizing dojo.destroyElement().

Changed 10 years ago by dante

Attachment: createElement.txt added

grep of all document.createElement refs (some invalid)

comment:13 Changed 10 years ago by dante

Owner: changed from Eugene Lazutkin to dante
Status: assignednew

added dojo.create and dojo.destroy in [16163]

Changed 10 years ago by dante

Attachment: dijit-create.patch added

part I of dijit pass

comment:14 Changed 10 years ago by dante

Status: newassigned

comment:15 Changed 10 years ago by dante

(In [16184]) refs #7802 and refs #7461 - adding better docs for dojo.create (noting about alias to dojo.doc.createElement) and adding other examples. !strict

comment:16 Changed 10 years ago by dante

(In [16253]) refs #7802 - modify Dijit (only base and root ns, no form or layout yet) to use dojo.create pattern. This is earlier than anticipated for this ci but was tested in ie6/7 and no apparent errors are thrown, but will need further testing as a change caused by this checkin might be really subtle. (eg attr before placement, etc) This is to be used to benchmark toDom actions. !strict

comment:17 Changed 10 years ago by Eugene Lazutkin

(In [16300]) Reusing dojo.create() in the base (back and dnd functionality), !strict, refs #7802.

comment:18 in reply to:  17 Changed 10 years ago by Eugene Lazutkin

Replying to elazutkin:

(In [16300]) Reusing dojo.create() in the base (back and dnd functionality), !strict, refs #7802.

In reality not in the base, but in the core. :-)

comment:19 Changed 10 years ago by Eugene Lazutkin

I found one interesting gotcha, which should be reflected in documentation somehow, or otherwise acted upon. This code doesn't work correctly on IE:

var node = dojo.create(tag, {style: {opacity: 0.5}}, ref);

This code works:

var node = dojo.create(tag, null, ref);
dojo.attr(node, {style: {opacity: 0.5}});
// or
// dojo.style(node, "opacity", 0.5);

It looks like the node should be live in order to be translucent on IE due to IE's filters limitations.

We can tweak dojo.create() and place the node before running dojo.attr(), but I am not sure if it is the right solution in general, e.g., it may cause flickering. Alternatively we can do it on IE only.

comment:20 Changed 10 years ago by dante

@elazutkin - that must be IE6 only, this code works fine in a local test case in IE7 (i've not tested ie6 here)

// create a draggable handle thing over our target
this.picker = d.create('div', {
	"class":"imageDragger",
	style: { opacity: this.hoverable ? 0 : d.isIE ? 0.25 : 1, 
		width: gs + "px", height: gs + "px"
	}
}, this.domNode, "before");

it successfully applies the 0.25 transparency in IE7 at least, as this is using DnD and without that little opacity the new node above cannot be dragged in IE

Changed 10 years ago by dante

Attachment: create.html added

test IE + create(n, { style:{opacity:x } }, ref)

comment:21 Changed 10 years ago by dante

@elazutkin - attached create.html to test your statement. works in IE6 and 7 for me. test shows setting opacity before and after innerHTML and always before place() calls. Is there a specific node type you were using? only tested LI / DIV here.

comment:22 in reply to:  20 Changed 10 years ago by Eugene Lazutkin

Replying to dante:

@elazutkin - that must be IE6 only, this code works fine in a local test case in IE7 (i've not tested ie6 here)

Yes, it was IE6 in context of the DnD avatar --- setting opacity on avatar's <tr> elements. I had to split my calls in two like I shown above to get the desired effect.

comment:23 Changed 10 years ago by dante

Resolution: fixed
Status: assignedclosed

@uhop - i'd believe that about <tr> ... edge case?

functionality in base, user docs compelte, api docs accurate. need to finish overall dom guide.

Note: See TracTickets for help on using tickets.