I'm sure there's a better more 'jQuery' like way of writing this. In particular the selector $(this).closest('div.login-box') being in there twice. Perhaps it's possible to use $(this)?

$('.close-box').click(function () {
        if ($(this).closest('div.login-box').attr('id') == "login-forgot") {

You could combine the calls:

if ($(this).closest('div.login-box')

However, I do not recommend this.
Although it will work, it's unnecessarily confusing.

  • \$\begingroup\$ I dunno... if he just did loginbox_id = $(this).closest('div.login-box').slideToggle("400").attr('id'); Then he could just do if (loginbox_id == "login-forgot") below it. I think it removes any confusion that may occur by placing animation calls inside a boolean statement. \$\endgroup\$ – daybreaker Jun 14 '11 at 21:02
  • 1
    \$\begingroup\$ @day: Yes. However, it's still confusing to have a mutation and a property access in the same line. \$\endgroup\$ – SLaks Jun 14 '11 at 21:19

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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