6

I'm sure this is incredibly simple but I'm pretty new to JS and I'd like to know the best way of doing this rather than hacking it out with a workaround.

So I have a div block and it's visibility is toggled when pressing a text field above it.

<h3 onclick="javascript: toggle1();">
  <span style="cursor:pointer">
    <b>Text1</b>
  </span>
</h3>

<div id="Text1" hidden="hidden">blahblah</div>

And then I have my JS:

function toggle1() {
  $('#Text1').toggle(1000);
}

This works fine, however when the user clicks the header text I also want to alter the height of a <hr> element above it

<hr id="line1" style="height:2px;border:none;color:#03930f;background-color:#03930f;" />

I have tried adding:

if($('#Text1').is(':visible')) {
  document.getElementById("line1").style.height = "15px";
}
else { 
  document.getElementById("line1").style.height = "2px"; 
}

but this doesn't work...I assume because the toggle() function doesn't toggle the same thing the is(':visible') condition is checking.

What is the correct way of doing this?

5

1 Answer 1

5

Answer - first pass

.toggle does affect the :visible status of an element, but having given it a duration (1000) its not immediate, which means if you want something to happen after it's finished it needs to be supplied in an optional callback:

$('#Text1').toggle(1000,function(){
    if($('#Text1').is(':visible')) {
        document.getElementById("line1").style.height = "15px";
    }
    else { document.getElementById("line1").style.height = "2px"; }
});

Answer - second pass

You're mixing jquery and vanilla javascript - while there is nothing wrong with that inherantly, it's a bit confusing. Stick to one or the other. Helpfully, inside a callback you can refer to the element using $(this), and as you already know elements can be targetted by id using #elementId

$('#Text1').toggle(1000,function(){
    if($(this).is(':visible')) {
         $('#line1').css('height','15px');
    }
    else { 
         $('#line1').css('height','2px');
    }
});

Answer - third pass

In line with the above, you shouldnt mix jquery and old-fashioned event handlers in html - again stick to jQuery if thats what you're writing

<h3><span style="cursor:pointer"><b>Text1</b></span></h3>
<div id="Text1" hidden="hidden">blahblah</div>
<hr id="line1" style="height:2px;border:none;color:#03930f;background-color:#03930f;" />

and

$('h3').on('click',function(){
    $('#Text1').toggle(1000,function(){
        if($(this).is(':visible')) {
             $('#line1').css('height','15px');
        }
        else { 
             $('#line1').css('height','2px');
        }
    });
});

Updated jsfiddle: http://jsfiddle.net/xLxsctfk/3/

1
  • Thankyou very much. Really helpful. Yes I'm very new to this so I imagine lots of poor things are happening in my code. This is much cleaner. Sep 16, 2015 at 10:48

Your Answer

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

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