Edit button toggles modal and load data from PHP using AJAX and JQUERY

Question

I've been working on an edit/update button that will toggle a modal that looks like this:

Update/Edit Employee Modal

Is there a better code for this JavaScript code that I made?

<script>
$('body').on('click', '.editButton',function(){
 let edit = $(this).val()
 console.log("Edit this : " + edit);
 $('#updateEmpModal').modal('toggle');
 $.ajax({
 type : 'POST',
 url : 'get.php',
 data : {'edit' : edit},
 dataType: 'json',
 success: function(data){
 $("#updateEmpForm input[name='EMAIL']") .val(data.EMAIL);
 $("#updateEmpForm input[name='PASSWORD']") .val(data.PASSWORD);
 $("#updateEmpForm input[name='RIGHTS']") .val(data.RIGHTS);
 $("#updateEmpForm input[name='LAST_NAME']") .val(data.LAST_NAME);
 $("#updateEmpForm input[name='FIRST_NAME']") .val(data.FIRST_NAME);
 $("#updateEmpForm input[name='MIDDLE_NAME']") .val(data.MIDDLE_NAME);
 $("#updateEmpForm input[name='SUFFIX']") .val(data.SUFFIX);
 $("#updateEmpForm input[name='GENDER']") .val(data.GENDER);
 $("#updateEmpForm input[name='BIRTHDATE']") .val(data.BIRTHDATE);
 $("#updateEmpForm input[name='BIRTHPLACE']") .val(data.BIRTHPLACE);
 $("#updateEmpForm input[name='CITIZENSHIP']") .val(data.CITIZENSHIP);
 $("#updateEmpForm input[name='RELIGION']") .val(data.RELIGION);
 $("#updateEmpForm input[name='ADDRESS']") .val(data.ADDRESS);
 $("#updateEmpForm input[name='CONTACT']") .val(data.CONTACT);
 $("#updateEmpForm input[name='ICE_NAME']") .val(data.ICE_NAME);
 $("#updateEmpForm input[name='ICE_CONTACT']") .val(data.ICE_CONTACT);
 $("#updateEmpForm input[name='DEPARTMENT']") .val(data.DEPARTMENT);
 $("#updateEmpForm input[name='POSITION']") .val(data.POSITION);
 $("#updateEmpForm input[name='EMP_TYPE']") .val(data.EMP_TYPE);
 $("#updateEmpForm input[name='edit']") .val(data.ID);
 }
 });
});
</script>

It is working, but I want to know if this is a bad practice and if there are better solutions.

• mickmackusa: Safe to remove the php tag here?

Answer 1

Welcome to Code Review! Hopefully you enjoy using this site and receive valuable feedback.

---

Wow, what patience you have to type out/copy all of those lines to update the form inputs. Instead of doing that, you could loop through the input fields and check if the name matches a key in the returned data. One can use .each() to iterate over the input fields, then check the name using .attr() and the in operator.

$('#updateEmpForm input').each(function() {
 const inputName = $(this).attr('name');
 if (inputName in data) {
 $(this).val(data[key]);
 }
});

To exclude certain fields, like any password inputs, the selector could be updated to include pseudo-class selectors like `:not() - for example:

$('#updateEmpForm input:not([type="password"])').each(function() {

That should handle all but the last repeated line - which can be included before or after the call to .each():

$("#updateEmpForm input[name='edit']").val(data.ID);

You could also define a mapping of input names to keys and look in the mapping for values. For example:

const inputNameKeyMapping = {
 edit: 'ID',
 //any other names that don't match keys directly
}

And use that mapping when assigning the value - something like:

$('#updateEmpForm input').each(function() {
 const inputName = $(this).attr('name');
 const key = inputNameKeyMapping[inputName] || inputName;
 if (key in data) {

That way you wouldn't need to include manual value settings lines.

---

Also, instead the click handler can be simplified from:

$('body').on('click', '.editButton',function(){

To using the .click() (short-cut) method on only elements with that class name editButton:

$('.editButton').click(function(){

---

I see let is used to declare the variable edit:

let edit = $(this).val()

However, that value is never re-assigned. To avoid unintentional re-assignment, you could use const instead.

const edit = $(this).val()

And it is best to be consistent with line terminators - if most lines have them then make sure all do.