Theme/skin swap... version 2! (CSS-driven)

Question

I basically have an AJAX-fetched skin (html/css) that's loaded into a dummy element (id=SkinContainer) when the page is loaded, and then the content of one of its divs (contentdiv) is fetched with another AJAXHTTPRequest.

At any rate, when the page is loaded, the user can click a button to swap the theme/skin. Below is the (working) code for my theme swap, but I'm wondering whether or not it's the best way to do this:

function themeSwap() {
    var oldTheme = themeSelect;
    themeSelect++;
    if (themeSelect>2 || themeSelect<1) {themeSelect=1;}
    $('html').addClass('js');

    var tempContentDiv = document.getElementById("contentdiv");
    var tempContent = tempContentDiv.innerHTML;

    ReplaceJSCSSFile("css/skin" + oldTheme  + ".css", "css/skin" + themeSelect + ".css", "css");
    AJAX_LoadResponseIntoElement("skinContainer", "skin" + themeSelect + ".txt", function() {themeSwapCallback(tempContent)} );
}

function themeSwapCallback (tempContent) {
    initPage();
    document.getElementById("contentdiv").innerHTML = tempContent;
    setCookie("themeSelection",themeSelect,365);
}

What this basically does is stores the innerHTML of contentdiv into the "tempContent" variable, loads the new skin into skinContainer with an AJAX fetch, and then restores the original content by setting contentdiv's innerHTML to "tempContent."

The reason why it has to use an AJAX fetch on top of the CSS swap is because the structure of the elements changes.

Have I finally have this done correctly? or is this still not perfect? :(

Answer 1

Without having any other information available as to the restrictions of the project, I have to say that I do not like this implementation whatsoever.

The smartest way to go about swapping css "themes" is by ONLY changing class definitions. There are several ways to do this, but in the case of a "themed" application the most practical approach would be having seperate .css files for each theme.

So for example, given the following HTML:

<div class="header">
    <div id="logo"></div>
</div>

...and theme-default.css:

.header { background-color: black; height: 200px; width: 100%; }
.logo { background-image: url('/images/logo.jpg'); height: 200px; width: 200px; }

...and theme-red.css:

.header { background-color: red; height: 200px; width: 100%; }
.logo { background-image: url('/images/logo-red.jpg'); height: 200px; width: 200px; }

You would only need to swap out the css URL that is being called, something like this:

$(function(){
    $("#btnChange").click(function() {
        $('link[href="theme-default.css"]').attr('href','theme-red.css');
    });
});

Answer 2

My 2 cents:

• The code is not bad, I really only have minor nitpickings

• Your theme selection code use a trinary:

var oldTheme    = themeSelect;
var themeSelect = themeSelect==1?2:1;

• Since you only use tempContentDiv once, you might as well do

var tempContent = document.getElementById("contentdiv").innerHTML;

• Alternatively, you could store the reference to contentdiv somewhere else, so that you do not have to repeat the document.getElementById call