CODEWITHSUNDEEP
jquery
Tom
Tom

Reputation: 1618

Jquery use same event on multiple elements

I've several fields that are very similar and contain passwords, so I've assigned a class to them and I'm now trying to show and hide the password or plain text.

$("body").on( 'click', '.show', function () {
$id = $(this).attr('id')

if ($('#' + $id).prop('type') == 'password') {
    $('#' + $id).addClass('hide').removeClass('show');
    $('#' + $id).prop('type', 'text');
    setTimeout( function() {
        $('#' + $id).prop('type', 'password');
        $('#' + $id).addClass('show').removeClass('hide');
    }, 2500);
    }
})

This works fine for each field and as I click into them the correct password is shown and it is reverted back to a password field after 2.5 seconds.

The issue I have is if I click one field, then another, then another each is shown but only the last one reverts back.

How do I get this to revert them back to a password field even if other fields call this function ?

Thanks

The form fields are similar to the following: <input class='show' type='password' id='access' name='access' value='' >

Upvotes: 1

Views: 25

Answers (2)

Frederik Pietzko
Frederik Pietzko

Reputation: 113

When binding a function to an event, that function gets called with the event as a parameter. That event has a field called "target".

So what you could do is simply just alter type prop on that target, instead of identifying your element by id. So something like:

$("body").on( 'click', '.show', function (e) {

if ($(e.target).prop('type') == 'password') {
    $(e.target).addClass('hide').removeClass('show');
    $(e.target).prop('type', 'text');
    setTimeout( function() {
        $(e.target).prop('type', 'password');
        $(e.target).addClass('show').removeClass('hide');
    }, 2500);
    }
})

Upvotes: 1

Rory McCrossan
Rory McCrossan

Reputation: 337560

The issue is because you're missing the var or let keyword before defining $id, it's declared globally. As such only the last value of $id is used when the timeout eventually executes. The quick way to fix this is to declare the $id in the correct scope:

var $id = $(this).attr('id'); // or: var $id = this.id

In addition, you should note that you don't need to repeatedly create a jQuery object with the same element reference. You can create it once and store it in a variable. Also, retrieving the id from the element referenced by this and then building a selector string from it is redundant; you can use this directly in your jQuery object. Finally, the $ prefix on variables should be used when the variable holds a jQuery object, not a string.

With all that in mind, try this:

$(document).on('click', '.show', function() {
  let $el = $(this);
  if ($el.prop('type') === 'password') {
    $el.addClass('hide').removeClass('show').prop('type', 'text');
    setTimeout(function() {
      $el.prop('type', 'password').addClass('show').removeClass('hide');
    }, 2500);
  }
})
<script src="https://cdnjs.cloudflare.com/ajax/libs/jquery/3.3.1/jquery.min.js"></script>
<input class="show" type="password" id="access01" name="access" value="" /><br />
<input class="show" type="password" id="access02" name="access" value="" /><br />
<input class="show" type="password" id="access03" name="access" value="" /><br />
<input class="show" type="password" id="access04" name="access" value="" />

One thing I would also say, is that exposing the password value when the field is clicked is not a secure UX. I would suggest having the password hidden by default and having a separate button to allow it to be displayed in plain text.

Upvotes: 1

Related Questions