5
\$\begingroup\$

This is a social share widget for guubo.com. The live widget can be seen here (though you need to be signed in guubo.com, otherwise you will see a logged-out user interface). JS is not my first language, therefore some reviews are welcome.

// create iframe
var iframe      = document.createElement('iframe');

iframe.setAttribute('style', 'border:none; overflow:hidden; width: 93px; height: 32px;');
iframe.setAttribute('scrolling', 'no');
iframe.setAttribute('frameborder', '0');
iframe.setAttribute('allowTransparency', 'true');

document.body.appendChild(iframe);

// access iframe object 
iframe.doc      = null;

if(iframe.contentDocument)
{
    iframe.doc  = iframe.contentDocument;
}
else if(iframe.contentWindow)
{
    iframe.doc  = iframe.contentWindow.document;
}
else if(iframe.document)
{
    iframe.doc  = iframe.document;
}

iframe.doc.open();
iframe.doc.close();


// load CSS to iframe window
var css_object  = document.createElement('link');

css_object.rel  = 'stylesheet';
css_object.type = 'text/css';

css_object.href = 'http://guubo.com/public/css/share.css?' + Math.random();

iframe.doc.head.appendChild(css_object);

var css_object  = css_object.cloneNode('false');

css_object.href = 'http://guubo.com/public/css/reset.css?' + Math.random();

iframe.doc.head.appendChild(css_object);

// load CSS to parent window
var css_object  = css_object.cloneNode('false');

css_object.href = 'http://guubo.com/public/css/share.css?' + Math.random();

iframe.doc.body.innerHTML   = '<a href="http://guubo.com/?url=' + encodeURIComponent(document.location.href) + '&name=' + encodeURIComponent(document.title) + '" target="_blank" class="guubo_ui guubo_share_button" onclick="parent.guuboCreateDialog(); return false;"><?=$share_count?></a>';

function calculateCeneteredPosition(Xwidth, Yheight)
{
    // First, determine how much the visitor has scrolled

    var scrolledX, scrolledY;

    if( self.pageYOffset ) {
        scrolledX = self.pageXOffset;
        scrolledY = self.pageYOffset;
    } else if( document.documentElement && document.documentElement.scrollTop ) {
        scrolledX = document.documentElement.scrollLeft;
        scrolledY = document.documentElement.scrollTop;
    } else if( document.body ) {
        scrolledX = document.body.scrollLeft;
        scrolledY = document.body.scrollTop;
    }

    // Next, determine the coordinates of the center of browser's window

    var centerX, centerY;

    if( self.innerHeight ) {
        centerX = self.innerWidth;
        centerY = self.innerHeight;
    } else if( document.documentElement && document.documentElement.clientHeight ) {
        centerX = document.documentElement.clientWidth;
        centerY = document.documentElement.clientHeight;
    } else if( document.body ) {
        centerX = document.body.clientWidth;
        centerY = document.body.clientHeight;
    }

    return {left: scrolledX + (centerX - Xwidth) / 2, top: scrolledY + (centerY - Yheight) / 2};
} 

function countCharacters()
{
    var guubo_message       = document.getElementById('guubo_message');
    var count_message       = document.getElementById('guubo_count');

    count_message.innerHTML = 116-guubo_message.value.length;
}

function submitForm()
{
    document.forms["guubo_form"].submit();

    guubo_dialog.innerHTML  = 'Message sent to guubo.com!';

    setTimeout(function(){
        guuboCloseDialog();
    }, 1000);
}

function guuboCloseDialog()
{
    document.body.removeChild(guubo_dialog);
}

function guuboCreateDialog()
{
    guubo_dialog        = document.createElement('div');
    var position        = calculateCeneteredPosition(400, 150);

    guubo_dialog.setAttribute('id', 'guubo_dialog');
    guubo_dialog.setAttribute('class', 'guubo_dialog');

    guubo_dialog.setAttribute('style', 'left: ' + position.left + 'px; top: ' + position.top + 'px;');
    guubo_dialog.innerHTML  = '\
    <form name="guubo_form" id="guubo_form" action="http://guubo.com/share3.php" method="post">\
        <div id="guubo_dialog_header">\
            <div class="left"><a href="http://guubo.com" target="_blank">guubo.com</a> – chain share & analytics service</div>\
            <div class="right"><a href="#" onclick="parent.guuboCloseDialog(); return false;">close dialog</a></div>\
        </div>\
        \
        <textarea name="message" id="guubo_message" onKeyDown="countCharacters();" onKeyUp="countCharacters();" maxlength="116">' + document.title + '</textarea>\
        <input type="submit" value="Share" onclick="parent.submitForm(); return false;" />\
        <p><span id="guubo_count"></span> characters remaining. guubo.com will automatically append URL to the message.</p>\
    </form>\
    <div class="clear"></div>';

    document.body.appendChild(guubo_dialog);

    countCharacters();
}
\$\endgroup\$

1 Answer 1

3
\$\begingroup\$

Your code sets a lot of global variables, and you should always avoid them. The easiest way to do that is "to wrap your code in a closure and manually expose only those variables you need globally to the global scope."

Your script works a lot with DOM and DOM related objects. A lot of the goals you seem to be trying to reach for in that area could be achieved more expressively using the robust patterns found in a good library like jQuery, Prototype, Dojo, or Mootools. It is nearly always best to pick one of them and learn the API rather than trying to reinvent the wheel. Changes will be easier, and, likely, your code will be more cross-browser compatible. Don't use such a library for everything under the sun, but most of them cover a lot of common concerns.

With jQuery, for example, this line:

guubo_dialog.setAttribute('style', 'left: ' + position.left + 'px; top: ' + position.top + 'px;');

need not depend on an awkward string concatenation to set the style attributes. It could instead look much cleaner and be more adaptable:

$(guubo_dialog).css({
    'left': position.left + 'px',
    'top': position.top + 'px'
});

To keep your code style consistant, consider using JSLint or JSHint to tell you where you've gone wrong. guubo_dialog, for example, ought to be explicitly declared as a variable somewhere.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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