CODEWITHSUNDEEP
javascripthtmldisplay
Filtered
Filtered

Reputation: 75

Javascript display == "none" not actually displaying

I wrote a basic toggle function that I have used before but for some reason it is not entirely working. The reason I say that is because when I console log the display it says the display is none but the element never disappears. I know for a fact that the function is firing and that it detects the elements display as "", however it sets it to none (and logging that to the console proves it) but the actual element does not disappear from the screen.

function filter_toggle(){
    var form = document.getElementById("filter_form");
    var display = form.style.display;
    console.log(display);
    if (display == ""){
        display = "none";
    }
    else if (display == "none"){
        display = "";
    }
}
<div id="filter_wrap">
                <div id="filter_toggle" class="basic_toggle" onclick="filter_toggle()">--Filter--</div>
                <div id="filter_form" class="bg-1D1D1D">
                ...more elements...
                </div>
</div>

Upvotes: 2

Views: 1915

Answers (2)

Djaouad
Djaouad

Reputation: 22776

The variable display has been assigned the value of the display property of the object (which is a string value), changing the variable will not change the property, instead, you can use style:

function filter_toggle() {
  var form = document.getElementById("filter_form");
  var style = form.style;
  
  if (style.display != "none") {
    style.display = "none";
  } else if (style.display == "none") {
    style.display = "block";
  }
}
<div id="filter_wrap">
  <div id="filter_toggle" class="basic_toggle" onclick="filter_toggle()">--Filter--</div>
  <div id="filter_form" class="bg-1D1D1D">
    Hello, World!
  </div>
</div>

Upvotes: 2

Scott Marcus
Scott Marcus

Reputation: 65806

Your code checks the style.display property, but then only sets display, when it should be setting style.display. Also, there's no need (in this case) for else if. You can just use else. And, instead of display = "", it's more accurate to toggle between display:none and display:block (for hiding/showing block level elements, like div or display:inline for hiding/showing inline elements like span).

function filter_toggle(){
    var style = document.getElementById("filter_form").style;
    console.log(style.display);
    if (style.display == "none"){
        style.display = "block";
    } else {
        style.display = "none";
    }
}
<div id="filter_wrap">
                <div id="filter_toggle" class="basic_toggle" onclick="filter_toggle()">--Filter--</div>
                <div id="filter_form" class="bg-1D1D1D" style="display:none;">
                ...more elements...
                </div>
</div>

You could also make this more compact with a ternary operator.

function filter_toggle(){
    var style = document.getElementById("filter_form").style;
    // This is a compact if/then/else
    style.display = style.display == "none" ? "block" : "none";
}
<div id="filter_wrap">
                <div id="filter_toggle" class="basic_toggle" onclick="filter_toggle()">--Filter--</div>
                <div id="filter_form" class="bg-1D1D1D">
                ...more elements...
                </div>
</div>

However, you are reinventing the wheel and using inline styles (which should always be avoided if possible due to the extra code they introduce and their brittle nature). The .classList.toggle() function does this for you and works with more manageable CSS classes:

let form = document.getElementById("filter_form");

function filter_toggle(){
  form.classList.toggle("hidden");
}
.hidden { display:none; }
<div id="filter_wrap">
                <div id="filter_toggle" class="basic_toggle" onclick="filter_toggle()">--Filter--</div>
                <div id="filter_form" class="bg-1D1D1D">
                ...more elements...
                </div>
</div>

Upvotes: 0

Related Questions