Hallo,
I was asked (told) to make a popup advertisement. Yuck. But, whatever, I know how to stop caring about my code in situations like this. Still, an opportunity to better my Javascript anyway.
Because a back-end script first checks to see if the person is new visitor, the script tag gets added into the HTML by the back-end, so it made most sense (I think) to have the script build the popup entirely, instead of having HTML there for everyone. I don’t care if sensible people have a popup blocker and I don’t care if non-JS’d people/the googles don’t see it. The current boss only cares about Windows with IE anyway.
So the script builds the box, and a Close button on it just manually changes the display to “none”, mostly because it didn’t make sense to play with classes for such a one-time throwaway element. I do however have an awful lot of appendChilds and it doesn’t look very nice. Tutorials tend to show it the way I’ve done it so maybe, I dunno…
Here is the current, working code:
var EvilPopup = {
init: function() {
var container = document.getElementById('container'),
div = document.createElement('div'),
ad = Basis.createAnchor('Word abonnee van <some company>', 'ad', 'http://example.com'),
close = Basis.createAnchor('sluiten', 'close', '#void'),
bannerImg = Basis.createImg('Word nu abonnee van <some company> voor slechts €5 per jaar', '250', '250', 'images/Banner_abonnee.jpg'),
closeImg = Basis.createImg('Sluiten', '16', '16', 'images/close.png');
div.id = 'shlEvilPopup';
ad.appendChild(bannerImg);
close.appendChild(closeImg);
Basis.addEventListener(close, 'click', EvilPopup.hide);
div.appendChild(ad);
div.appendChild(close);
container.parentNode.insertBefore(div, container);
},
hide: function(event) {
var div = this.parentNode;
div.style.display = 'none';
return;
}
};
Basis.begin(EvilPopup);
It creates this (bold) HTML:
<body>
[b]<div id="shlEvilPopup">
<a href="http://example.com" id="ad" title="Word abonnee van <some company>"><img src="Banner_abonnee.jpg" alt="Word nu abonnee van <some company> voor slechts &#8364;5 per jaar" height="250" width="250"></a>
<a href="#void" id="close" title="sluiten"><img src="close.png" alt="Sluiten" height="16" width="16"></a>
</div>[/b]
<div id="container">
...
(alt text for when there’s no image, title text on the anchors for keyboarders to know the text normally only mousers get, with some CSS help)
All the object stuff and init blah blah is partially because I know other scripts will appear later and at some point I’m going to have to incorporate them all into this… right now, it looks like a bit of overkill for what it’s doing.
the helper functions inside Basis:
...
Basis.createAnchor = function(title, id, href) {
var anchor = document.createElement('a');
if (title) {
anchor.title = title;
}
if (id) {
anchor.id = id;
}
anchor.href = href;
return anchor;
}
Basis.createImg = function(alt, h, w, src) {
var img = document.createElement('img');
img.alt = alt;
img.height = h;
img.width = w;
img.src = src;
return img;
}
...
My question is, is there a more elegant way to build an element, or is it just JS verbosity to have all these appendChilds? I thought of what I’ve learned of documentFragment but that’s appending a bunch of stuff at the same level… here I’m putting things inside of things inside of things. Any other places this code could have been better written (removing the display = “none” aside)?
I also have a side question: when I’m listing a bunch of arguments like above, and they’re all being filled with strings, I understand that if I don’t fill ALL the args that’s ok, the last will be ignored, but there was also the idea of doing this:
Basis.createAnchor = function(href, id, title, text)
Well, if I’m later creating an anchor who has text but no title, the script would not know which was which in this case:
var someAnchor = Basis.createAnchor(‘url’, ‘theID’, ‘some anchor text’)
“title” is the third arg in the creation function… so would it assume that’s title text? If so, does this mean when I have any number of possible missing attributes, I cannot do my helper function this way (should I instead gamble on the setAttribute thing even though IE is evil with it?)?
(Currently, all created anchors will have an href and an id, even though I have the “if (id)” part in there. In this particular case of the popup, there will always be a title and never any anchor text… so my question is a general one.)