1
\$\begingroup\$

I am working on a small script for a mobile site. Is there any way to improve on the regex here? or remove it completely?

function fn_page_change(event) {
    event.preventDefault();
    var activePage = $(".ui-active");
    var newPage = $(event.target.getAttribute("href"));
    activePage.className = activePage.className.replace(/(?:^|\s)ui-active(?!\S)/, " ui-inactive ");
    newPage.className = newPage.className.replace(/(?:^|\s)ui-inactive(?!\S)/, " ui-active");
}

I'm not using jQuery, the $ is myown

function $(selector, el) {
    if (!el) {
        el = document;
    }
    var tmp = el.querySelectorAll(selector);
    if (tmp.length === 1) {
        return tmp[0];
    }
    return tmp;
}

Here is an example

\$\endgroup\$

2 Answers 2

3
\$\begingroup\$

You will have less code if you use a regex because you need the word boundary detection that is built into regex. I'd suggest putting it in a utility function to make it cleaner and more readable:

function replaceClass(el, oldClass, newClass) {
    el.className = el.className.replace(new RegExp("\\b" + oldClass + "\\b"), newClass);
}

function fn_page_change(event) {
    event.preventDefault();
    var activePage = $(".ui-active");
    var newPage = $(event.target.getAttribute("href"));
    replaceClass(activePage, "ui-active", "ui-inactive");
    replaceClass(newPage, "ui-inactive", "ui-active");
}

If you wanted a version of replaceClass() that would handle either of the types of return values you get out of your selector function (single DOM object or array of DOM objects), then you could use this:

function replaceClass(el, oldClass, newClass) {
    var re = new RegExp("\\b" + oldClass + "\\b");
    if (el.nodeType) {
        el.className = el.className.replace(re, newClass);
    } else {
        for (var i = 0; i < el.length; i++) {
            el[i].className = el[i].className.replace(re, newClass);
        }
    }
}
\$\endgroup\$
1
\$\begingroup\$

You don't need to worry about regular expressions or word boundaries in this particular use case for two reasons:

  1. You know the exact class names you are targeting.
  2. Neither class name is embedded verbatim within the other (for example 'ui-active' and 'ui-active-false' would require a regex that reads word boundaries).

In your case though, 'ui-active' and 'ui-inactive' will never throw a false positive match for one another in string or regex form.

This means you can simply search and swap the exact class names themselves.

activePage.className = activePage.className.replace('ui-active','ui-inactive');
newPage.className = newPage.className.replace('ui-inactive','ui-active');

//OR if you do prefer to stick with regular expressions, simply use...

activePage.className = activePage.className.replace(/ui-active/,'ui-inactive');
newPage.className = newPage.className.replace(/ui-inactive/,'ui-active');

You also do not need to worry about adding an additional space in front of the class name you are adding in this case (I noticed you putting an extra space, such as ' ui-inactive' for the the replacement class, for example). Since the spaces between class names in the className value are already space delimited, the fact that this alternative approach only targets the class name itself, without touching or altering the white-space, you do not need to worry about extra spaces in your substitution values.

Again, this is not a solution that will work in every situation for replacing class names for elements, but given your specific use case, and given the class names you are specifically targeting, it provides a simpler alternative that will still easily fulfill your desired functionality.

\$\endgroup\$
2
  • \$\begingroup\$ As a side-note, I'm new to posting here and found this post through the 'Related' side panel coming from another post. And only now did I just realize the post is 5 years old. Maybe someone out there will find some use for the answer though. Whoops haha \$\endgroup\$ Commented Jan 26, 2017 at 16:39
  • \$\begingroup\$ Well, it's a nice enough revival. \$\endgroup\$ Commented Jan 26, 2017 at 17:06

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.