"Link" making function, need feedback

Hi,

Code:

/**
* Create new a href with all optional arguments.
* 
* @param	String. The uri for link. Default is a #.
* @param	String. Link text. Default is "Click here".
* @param	String. ID attribute. Default is an empty string.
* @param	String. Class attribute. Default is an empty string.
* @param	String. Link target. Default is _blank.
* @return	Object.
*/
function create_a(href, txt, id, class, target, rel) {
	
	var r = document.createElement('a');
	
	r.href = (typeof(href) == 'string') ? href : '#';
	//r.setAttribute('href', href);
	
	if (typeof(id) == 'string') { r.id = id; }
	//r.setAttribute('id', id);
	
	if (typeof(class) == 'string') { r.className = class; }
	//r.setAttribute('class', class);
	
	r.target = (typeof(target) == 'string') ? target : '_blank';
	//r.setAttribute('target', target);
	
	if (typeof(rel) == 'string') { r.rel = rel; }
	//r.setAttribute('rel', rel);
	
	 r.innerHTML = (typeof(txt) == 'string') ? txt : 'Click here';
	
	return r;
	
};

var li = document.createElement("li");
var linkage = create_a('http://google.com', 'Google!', null, null, null, null);
li.appendChild(linkage);

Questions:

  1. In the create_a() function, should I be using setAttribute instead of dot syntax? Both? What/which is more cross-browser friendly?
  2. Do you see anything wrong with my logic? If so, how would you do it differently?
  3. Is there any way to return the A object as html (for situations where I might use var.innerHTML(a)?)

Thanks!!! :spf:

Cheers,
Micky

I would write my conditions like so:



if(((href === typeof(string)) && (href != ""))) ? r.setAttribute('href', href); : r.setAttribute('href', #);


=== conpares the actual “type” of data (string, array, object, .ect), unlike ==. Also check to see if it’s empty. IF both pass, then put it into the setAttribute().

EDIT: Failed to anwser both questions. Yes, use the setAttribute method.

Don’t know about the 3rd question. Someone else chime in.

Hi USPatriot! Many thanks for your pro help & quick reply. I really appreciate it. :slight_smile:

Sweeet! Thanks for the code fix/upgrade! Is the “if” a typo?

Beautiful! I am stoked! Thanks for tips. :spf:

Yah, I am not sure if it is even possible… If I try using innerHTML on the “a” it just outputs the href… From what I have tested, using the function I posted in my original post, only appendChild(a) works. I am wonder if there is some sort of .toString() method that I can use on that type of object?

Thanks a billion USPatriot!!! Have a great night/day!

Cheers,
Micky

No problem Micky. Yeah, ignore the “if” I was thinking .NET at the time. lol. Just curious, what type of framework are you creating? Or, is just to make a project more modular?

Yah, thanks again. :slight_smile:

I am in the process of building embeddable HTML widget (populated via json request)… As tempting as it sounds, I am trying to avoid using a framework (i.e. jQuery) for the sake of keeping things light. :smiley:

I am stoked that this project has allowed me to get my head back into raw JS (lots of dust on those JS brain cells), but I really miss the jQuery goodness. :lol:

Thanks again for the help! I totally appreciate it. :slight_smile:

I will post my updated function tomorrow morning.

Cheers,
Micky

Micky, I knew I was forgetting something, your “typeof” object shouldn’t need to be a function “()”. Instead your code should be changed to:



(typeof href == "string")



I apologize. I don’t do alot of JS! lol Good Luck with your project!!!

Ah, cool! Now that you mention it, I was wondering about that a few days ago… Thanks for the clarification! :slight_smile:

Thanks again for all of your help. :spf:

Cheers,
M

Updated code:

/**
* Create new a href with all optional arguments.
* 
* @param	String. The uri for link. Default is a #.
* @param	String. Link text. Default is "Click here".
* @param	String. ID attribute. Default is an empty string.
* @param	String. Class attribute. Default is an empty string.
* @param	String. Link target. Default is _blank.
* @return	Object.
*/
function widget_create_a(href, txt, id, class, target, rel) {
	
	var r = document.createElement('a');
	
	href = ((typeof href === 'string') && (href != '')) ? href : '#';
	r.setAttribute('href', href);
	
	if ((typeof id === 'string') && (id != '')) { r.setAttribute('id', id); }
	
	if ((typeof class === 'string') && (class != '')) { r.setAttribute('class', class); }
	
	target = ((typeof target === 'string') && (target != '')) ? target : '_blank';
	r.setAttribute('target', target);
	
	if ((typeof rel === 'string') && (rel != '')) { r.setAttribute('rel', rel); }
	
	r.innerHTML = ((typeof txt === 'string') && (txt != '')) ? txt : 'Click here';
	
	return r;
	
};

Not fully tested, but I do like the upgrades. :slight_smile:

Thanks again USPatriot! :spf:

M

Here’s one for creating a dom element:

/**
* Create new dom element with optional class and id attributes.
* 
* @param	String. Element to create. Default is div.
* @param	String. ID attribute. Default is no ID.
* @param	String. Class attribute. Default is no class.
* @return	Object.
*/
function create_el(el, id, class) {
	
	el = ((typeof el === 'string') && (el != '')) ? el : 'div';
	
	r = document.createElement(el);
	
	if ((typeof id === 'string') && (id != '')) { r.setAttribute('id', id); }
	
	if ((typeof class === 'string') && (class != '')) { r.setAttribute('class', class); }
	
	return r;
	
};
...
var bod = create_el('ul', null, 'myClass');
someEl.appendChild(bod);
...

Maybe it will help others? Feedback is welcome.

M

Wow, the “EDIT” time for entries is not very long.

After posting my last entry, I discovered that “class” is a reserved JS word.

Just FYI… Not that my code is worth much, I just wanted to correct my mistake! :smiley:

Cheers,
Micky

I just discovered that IE7 on Vista does not like setAttribute(). Looks like using both the dot syntax (object syntax in JS?) and setAttribute() is the way to go. Here is my create_el() function, but updated to use both:

/**
* Create new dom element with optional class and id attributes.
* 
* @param	String. Element to create. Default is div.
* @param	String. ID attribute. Default is no ID.
* @param	String. Class attribute. Default is no class.
* @return	Object.
*/
function create_el(e, i, c) {
	
	e = ((typeof e === 'string') && (e != '')) ? e : 'div';
	
	r = document.createElement(e);
	
	if ((typeof i === 'string') && (i != '')) { r.id = i; r.setAttribute('id', i); }
	
	if ((typeof c === 'string') && (c != '')) { r.className = c; r.setAttribute('class', c); }
	
	return r;
	
};

IE would rather have setAttribute(“className” …) instead of “class”.

I ended up not using setAttribute, but dot notation, for this reason.

So I had my var called target and I ended up with
target.className = target.className + classValue;

instead of
target.setAttribute(‘class’, target.className + classValue);

I also ended up with a create anchor function:
function createAnchor(text, href) {
var anchor = document.createElement(‘a’);
anchor.appendChild(document.createTextNode(text));
anchor.href = ‘#’ + href;
return anchor;
}

Yours seems more careful.

Ahhh, “className”! I did not test that. Good catch! Thanks for bringing that to my attention. :slight_smile:

Nice. I wonder if you could shorten it to this(?):

target.className += classValue;

Ah, yes, I can see where your predicament is… Using setAttribute to append a new class, rather than wiping out classes that might already exist. In your example, you are using the dot notation to get the className inside of the setAttribute… Kinda feels oxymoron-ic/ish. :smiley:

This might be one solution:

var classValue = 'foo';
var cn = target.getAttribute('className');
target.setAttribute('className', cn + ' ' + classValue);

Just googled, and this person says “className” is fully supported in all browsers.

Oooh! Nice! Thanks for sharing. :spf: :tup:

Thanks for the feedback!

Have a great day!
Cheers,
Micky

Lol, wouldn’t you know it… Firefox 3.6/Mac does not like setAttribute(‘className’, c)… But it does like setAttribute(‘class’, c)!

See, these are things you learn when not using a Framework like jQuery. :lol:

I just tested several major browsers (mac/pc) and all of them were cool with this syntax:

r.className = c;

My head is spinning. :eye:

Yes, the problem is that when setAttribute got introduced, good browsers said “well, the attribute is actually called ‘class’, so we’ll use that”. Since it’s just a string being passed to setAttribute, it isn’t a problem. IE, on the other hand, just mapped over all the “dot notation” properties. The same problem arises with the “for” attribute (in LABEL elements) as for is a reserved word too (for fairly obvious reasons):

this.htmlFor = 'foo';
this.setAttribute('for', 'foo'); // non-IE
this.setAttribute('htmlFor', 'foo'); //IE

If you want to return your new element as an HTML string you can do this:

function create_el(e, i, c, s) {
  
    e = ((typeof e === 'string') && (e != '')) ? e : 'div';
  
    r = document.createElement(e);
  
    if ((typeof i === 'string') && (i != '')) { r.id = i; r.setAttribute('id', i); }
  
    if ((typeof c === 'string') && (c != '')) { r.className = c; r.setAttribute('class', c); }
  
    if (s) {
      var frag = document.createElement('div');
      frag.appendChild(r);
      return frag.innerHTML;
    }
   
    return r;
  
}

So you pass “true” in the 4th arg if you want a string.

If I were you, I’d just return false if the “e” var is not a string. Ideally you should also check it against all possible allowed element names (if using HTML).

I originally thought of using createDocumentFragment but since documentFragments are Nodes rather than Elements, they won’t support innerHTML, so a dummy DIV has to be created.

Ahhhh, that explains it! Thank you for the detailed info and code Raffles!!! :spf:

Oh, now that is cool! I was wondering if I had to do something like that… Frag is a pretty sweet name for it. :smiley:

Thank you so much for the code sample, that really helps. :slight_smile:

That definitely sounds more robust… In my latest JS project, a majority of the time I want a div el, but I am going to take your advice and make it return false.

Also, what exactly do you mean “check it against all possible allowed element names”? Should I setup an array of “allowed” element types in the function, and then check against that (exit if not in array)?

That is a great idea! Could you still use documentFragments for other things?

Too bad one can’t just create a new Object(), and append it to that… But I guess that is what you are doing with the dummy div.

Quick question, do you nullify your vars when you are done using them in order to free up memory? Kinda like how one would garbage collect listeners in Flash.

Thanks again!!!

Cheers,
Micky

Quick question, do you nullify your vars when you are done using them in order to free up memory? Kinda like how one would garbage collect listeners in Flash.

Normally garbage collection Just Works.
I’ve read that when you have closures in Javascript, IE’s memory leak (not sure if IE8 still has this problem) kicks in: you get circular references, either a var is referring to a DOM element or a DOM element getting mentioned by a function. There’s some code floating around there (usually called onUnload()) which specifically makes sure that when the page closes, all those circular references are removed for IE’s sake.

*edit he changed his url’s so it took me a bit to find the correct page, but here’s one version:
http://novemberborn.net/2005/04/javascriptevent-cache-10

Hi Stomme! Thanks for the reply! :slight_smile:

Ahh, that is good to know.

Interesting… Hmm, for this widget I am self-invoking function (which, from what I have read, creates its own closure) to kick-off the script:

(function () {
...
... setup and call other functions here
...
})();

I wonder if I should just ditch that? I have seen it used in some widgets, but not in others.

Oooh, nice! Thanks for finding that link! I am going to play with that code now. :wink:

Have an excellent day! Thanks for all of the help, I really appreciate it.

Cheers,
Micky

Interesting… Hmm, for this widget I am self-invoking function (which, from what I have read, creates its own closure) to kick-off the script:… </code> I wonder if I should just ditch that? I have seen it used in some widgets, but not in others.

Normally something like that example is a Good Thing and I’d keep it. It keeps variables safer than if they’re just floating around in the global namespace.

Before you read too much into anything else I say, know that I’m a JS and programming n00b, but Raffles and some others around here are pretty guru-y.