4
\$\begingroup\$

I am quite new to JavaScript and I wonder if you experts can see any obvious mistakes in this working code, and if any, suggest good improvements.

// add iframes to page
var normal_sortables = document.getElementById('normal-sortables');
normal_sortables.innerHTML = normal_sortables.innerHTML + '<div class="postbox"><iframe style="width:100%;height:300px;" id="iframe_upload" src="upload.php"></iframe><iframe style="width:100%;height:300px;" id="iframe_images" src="images.php"></iframe><iframe style="width:100%;height:300px;" id="iframe_pdf_documents" src="pdf.php"></iframe></div>';



// declaring all variables
var code_to_be_inserted = '',
textarea_content = document.getElementById('content'),
iframe_upload = document.getElementById('iframe_upload'),
iframe_images = document.getElementById('iframe_images'),
iframe_pdf_documents = document.getElementById('iframe_pdf_documents');



// upload iframes of images and pdf documents when file is uploaded
iframe_upload.onload = function () {
iframe_images.src = 'images.php';
iframe_pdf_documents.src = 'pdf.php';
}



// add image to content editor
iframe_images.onload = function () {
    var images = iframe_images.contentWindow.document.getElementsByTagName('img');
    for (var i = 0; i < images.length; i++) {
        images[i].onclick = function () {
            code_to_be_inserted = '<img alt="" src="'+this.src+'" />\n\n';
            textarea_content.value = code_to_be_inserted + textarea_content.value;
        }
    }
}



// add pdf documents to content editor
iframe_pdf_documents.onload = function () {
    var pdf_documents = iframe_pdf_documents.contentWindow.document.getElementsByTagName('a');
    for (var i = 0; i < pdf_documents.length; i++) {
        pdf_documents[i].onclick = function () {
            code_to_be_inserted = '\n\n<a href="' + this.href+'" target="_blank">Click here to open ' + this.innerHTML + '</a>';
            textarea_content.value = textarea_content.value + code_to_be_inserted;
            alert ('testar');
            return false;
        }
    }
}
\$\endgroup\$
  • \$\begingroup\$ Hope you are also aware of jslint site which is good for syntactical checking. \$\endgroup\$ – Vivek Dec 16 '11 at 10:20
4
\$\begingroup\$

If you wanted to retain any event handlers that were already attached to your elements, you could change the first piece of code like this an avoid one of the dangers of setting innerHTML on an element that already has a bunch of HTML in it:

// add iframes to page
var normal_sortables = document.getElementById('normal-sortables');
// create container div
var newDiv = document.createElement("div");
newDiv.className = "postbox";
// put content into container div
newDiv.innerHTML = '<iframe style="width:100%;height:300px;" id="iframe_upload" src="upload.php"></iframe><iframe style="width:100%;height:300px;" id="iframe_images" src="images.php"></iframe><iframe style="width:100%;height:300px;" id="iframe_pdf_documents" src="pdf.php"></iframe>';
// add container div to the sortables
normal_sortables.appendChild(newDiv);
\$\endgroup\$
  • \$\begingroup\$ I see, that seams to be good practice :) Thanks \$\endgroup\$ – Hakan Dec 16 '11 at 2:05
  • \$\begingroup\$ Shouldn't it be: newDiv.className ="postbox"; \$\endgroup\$ – Hakan Dec 16 '11 at 2:16
  • \$\begingroup\$ @Hakan - Yes, that was a copy/paste error. Fixed. \$\endgroup\$ – jfriend00 Dec 16 '11 at 2:18

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.