Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

This is my code:

function toggleElements()
{
    for (var i = 0; i < arguments.length; i++) {
        $(arguments[i])[0].style.display = (
            getProperty($(arguments[i])[0], "display") !== "none"
                ? "none"
                : "inline-block");
    }
}

(getProperty is the "deepCss" function from here and retrieves the current (computed) style of an element).

I use it like this:

<div id="searchdiv" onclick="toggleElements('#searchoplus', 
                                            '#searchominus', 
                                            '#search_simple', 
                                            '#search_extended');">

which will switch on those elements in the list that weren't visible, and switch off those that were.

I guess I can get rid of the for loop and use jQuery's idiomatic chained-expression syntax, but I don't know how. Can anyone help?

share|improve this question
add comment

3 Answers

up vote 2 down vote accepted

My jQuery's a bit rusty but if #searchoplus, #searchominus, etc. initially have a display value of inline-block you should be able to get your code down to:

function toggleElements() {
  $.each(arguments, function(index, id) {
    $(id).toggle();
  });
}

toggle() saves the initial value of the display and puts it back in place when it's re-toggled.

share|improve this answer
    
searchoplus and search_simple have display: inline-block while searchominus and search_extended initially have display: none. See my answer for a workaround. –  Felix Dombek Nov 21 '12 at 18:49
add comment

I'm not certain I understood what you want, but this should at least be a starting point.

I have assumed that "idiomatic chained-expression syntax" is referring to chainable methods. In other words you could write this:

setElements('#searchoplus', 
            '#searchominus', 
            '#search_simple', 
            '#search_extended').toggle().doSomethingElse();

To accomplish this, you need to create a function that returns an object that has a method that also returns the object.

function setElements(){

    //This is the class for creating the object
    function elObj(argumentz){

        //This method toggles and then returns the 'elObj' object
        this.toggle=function(){
            for (var i = 0; i < argumentz.length; i++) {
                $(argumentz[i])[0].style.display = (
                    document.getProperty($(argumentz[i])[0], "display") !== "none"
                        ? "none"
                        : "inline-block");
            }
            return this;
        }

        //This is another method that also returns the object
        this.doSomethingElse=function(){
            console.log("Something else");
            return this;                    
        }
    }


    //Create the new object and return it
    return new elObj(arguments);

}

Here is the full code: http://jsfiddle.net/4Mpcs/7/

share|improve this answer
    
I definitely misunderstood. I was thinking you wanted a non-jQuery solution... –  twiz Nov 21 '12 at 18:57
    
Just making sure, you don't want to just use $('#searchoplus,#searchominus,#search_simple,#search_extended'), right? –  twiz Nov 21 '12 at 21:40
    
+1 Well, this is still a valuable answer, even though I was looking for a way to utilize the existing power of jQuery. –  Felix Dombek Nov 22 '12 at 5:35
add comment

I have found that there are two possibilities.

1. The literal translation:

function toggleElements()
{
    $(arguments).each(function (index, element) {
        $(element)[0].style.display = (
            getProperty($(element)[0], "display") !== "none"
                ? "none"
                : "inline-block");
    });
}

2. Using toggle():

function toggleElements()
{
    $(arguments).each(function (index, element) {
        $(element).toggle();
    });
}

This second version needs a workaround to use the display: inline-block attribute correctly. You need to specify this attribute in your CSS file even for elements which are hidden when the page is loaded, and hide them by setting display: none in the style attribute of the element itself to get the correct behaviour with the jQuery toggle function.

share|improve this answer
1  
Be careful using each(), it's only meant for iterating over jQuery objects and can cause problems when used on anything else. jQuery.each() is the safe alternative :) –  Clive Nov 21 '12 at 18:50
add comment

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

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