Skip to content
Advertisement

Simple JavaScript & jQuery Functions ‘Sometimes’ Run Too Many Times

I’ve built a demo that simply allows you to create new rows and delete them. The rows are given a unique data-id attribute and an object where each data-id is pushed into a rows array.

For some reason, clicking the remove row icon sometimes tries to delete it numerous times – for example, do the following:

Open the JS console, add 2 extra rows (by clicking ‘Add one row’ twice) then delete the middle row number 2 (by clicking the remove icon on the left of the row) and you’ll see that the console logs ‘Removed Row 2’ twice – it should only log it once. If you create numerous rows and then remove rows, it sometimes repeats the action multiple times.

Does anyone know why this happens, and how to fix? I’m stumped. It should only happen once, and I assume that the entire function is running numerous times. Any fix and ‘best practices’ advice would be very helpful, thanks.

Codepen: https://codepen.io/anon/pen/drWyQb

let templateRow, addBtn, templateRows, tableBody, popup, objects, rows = [];

$(document).ready(function(){
    runDefaults();
});

function runDefaults() {
    $(".no_entries_row").detach().insertBefore(".template_row");
    $('.template_row').first().attr('data-id', '1');
    defineVariables();
    removeRow();
    testEmpty();
    showSuccess();
    addRow();
    changeBg();
}

function defineVariables() {
    templateRow = $('<div>').append($('.template_row').clone()).html();
    addBtn = $('.list_add');
    templateRows = $('.template_row');
    popup = $(".popup");
    tableBody = $('tbody');
}

function removeRow() {
    let removeId;
    
    // REPEATING ERROR SOMEWHERE HERE?
    templateRows.on("click", ".controls", function(){
        removeId = $(this).parent().attr('data-id');
        $(this).parent().fadeOut(0, function(){
            $(this).remove();
        });
        templateRows = $('.template_row');
        testEmpty();
        rows = rows.filter(obj => obj.id != removeId); console.log('Removed Row ' + removeId);
    });
}

function testEmpty() {
    let rowsLen = $('.template_row').length,
        noEntriesRow = $('.no_entries_row');
    if (rowsLen === 0)
        noEntriesRow.css('display','table-row');
    else
        noEntriesRow.css('display','none');
}
    
function showSuccess() {
    $(".submit").click(function(){
        popup.fadeIn();
    });
}

function addRow() {
    let id = 1;
    new rowObj(1);
    
    addBtn.on("click", function(){        
        id++;
        tableBody.append(templateRow);
        templateRows = $('.template_row');
        noOfRows = templateRows.length;
        $('.template_row:nth-of-type(' + (noOfRows+1) + ')').attr('data-id', id);
        removeRow();
        testEmpty();
        changeBg();
        new rowObj(id);
    });
}

function changeBg() {
    $(".label").change(function () {
    let colour = this.value;
        $(this).parent().parent().css('background', colour);
    });
}

function rowObj(id) {
    this.id = id;
    this.description = 'Table Row';
    
    rows.push(this);
}
* {
    box-sizing: border-box;
    padding: 0;
    margin: 0;
}
body {
    padding: 30px;
    font-family: arial;
}
table {
    width: 100%;
    background-color: #fff;
    text-align: left;
    border: 1px solid #ccc;
    border-collapse: collapse;
    margin-bottom: 10px;
}
tbody tr:not(:last-of-type), thead tr { border-bottom: 1px solid #ccc }
thead th {
    background: #f0f0f0;
    padding: 10px;
}
td { padding: 10px 10px 10px 0 }
.no_entries_row > td { padding-left: 60px }
.btn_col, .controls { text-align: center }
.controls { padding: 0 }
a.list_cancel { color: #000 }
a.list_add {
    text-decoration: none;
    color: #54A2CE;
}
i[class*="fa fa-plus-square"] { color: #999999 }
.action_btn {
    display: flex;
    flex-flow: row wrap;
    align-items: center;
    width: auto;
    justify-content: center;
    margin-top: 20px;
}
.action_btn input { margin: 0 }
input[name~="submit"] {
    padding: 5px 15px;
    font-size: 1em;
    margin: 0 10px;
}
.popup {
    flex: 1 100%;
    margin-top: 30px;
    text-align: center;
    font-size: 1.5em;
    display: none;
}
.popup > p {
    padding: 20px 50px;
    background: lightgreen;
    font-weight: 700;
    border-radius: 15px;
}
@media (min-width: 950px) {
   .popup > p {
       width: 25%;
       margin: 0 auto;
    } 
}
input, select {
    width: 100%;
    border-radius: 5px;
    padding: 5px;
    border: 1px solid #ccc;
}
select {
    -webkit-appearance: menulist-button;
    -moz-appearance: menulist-button;
    appearance: menulist-button;
    padding: 5px;
    height: 25px;
}
<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Transitional//EN" "http://www.w3.org/TR/xhtml1/DTD/xhtml1-transitional.dtd">
<html xmlns="http://www.w3.org/1999/xhtml">
<head>
<meta http-equiv="Content-Type" content="text/html; charset=utf-8" />
<title>Demo</title>
<link href="css/style.css" rel="stylesheet" type="text/css" />
<link rel="stylesheet" href="https://cdnjs.cloudflare.com/ajax/libs/font-awesome/4.7.0/css/font-awesome.min.css">
<!-- jQuery -->
<script src="https://ajax.googleapis.com/ajax/libs/jquery/3.3.1/jquery.min.js"></script>     
</head>

<body>

<div class="wrapper">
	
            <table class="common_table" cellspacing="0">
          
            <thead>
            <tr>
                <th class="btn_col"></th>
            	<th class="code_col">Demo Column 1</th>
            	<th>Demo Column 2</th>
            	<th class="data_col">Demo Column 3</th>
            	<th class="data_col">Demo Column 4</th>
            	<th class="short_col">Col 5</th>
            	<th class="data_col">Demo Column 6</th>
            </tr>
            </thead>
            <tbody>
            
            <tr class="template_row">
            	<td class="controls"><a href="#" class="list_cancel" title="Delete Row"><i class="fa fa-minus-circle"></i></a></td>
                <td><input type="text" class="" id="productcode" name="" value=""/></td>
                <td><input type="text" class="long" id="productname" name="" value=""/></td>
                <td><input type="text" class="" id="stock" name="" value=""/></td>
                <td><input type="text" class="" id="vendor" name="" value=""/></td>
                <td>
                    <select name="" id="" class="label">
                        <option value = ""></option>
                        <option value = "green" style="background-color:green;">Success</option>
                        <option value = "red" style="background-color:red;">Wrong</option>
                        <option value = "orange" style="background-color:orange;">Error</option>
                    </select>
                </td>
                <td><input type="text" class="" id="client" name="" value=""/></td>
            </tr>
			<tr class="no_entries_row">
            	<td colspan="7">No Entries</td>
            </tr>

            </tbody>
            </table>
            
        	<a href="#" class="list_add"><i class="fa fa-plus-square"></i> Add one row</a>
            
            <br class="clear" />
            
            <div class="action_btn">
                <input name="submit" class="action_btn submit" type="submit" value="Save"/>
                <input name="submit" class="action_btn cancel" type="submit" value="Cancel"/>
                <div class="popup">
                    <p>Saved successfully!</p>
                </div>
            </div>
             
        </div>
        
</body>
</html>

Advertisement

Answer

Your code is, in my humble opinion, way too complex.

Below is the same row add/remove behavior without your actual issue and the row coloring based on the <select> change… With only 3 event handlers.

I did not touch your HTML markup. I added one single CSS rule (to hide the template row)… And completely re-writen the code from scratch.

Feel super-free for questions, since there potentially is many things to explain. 😉

The key, I think, is that I focussed on events triggered by a user more than on “sub-functions naming”. The result is 28 lines versus 84…

$(document).ready(function(){
  
  var table_body = $(".common_table tbody");
  
  // "Add one row" link
  $(".list_add").on("click",function(){
    console.log("Added a row");
    $(".no_entries_row").hide();
    var clone = $(".template_row").clone().removeClass("template_row");
    table_body.append(clone);
  });
  
  // Remove row icon
  $(document).on("click",".list_cancel",function(){
    console.log("Removed a row");
    $(this).closest("tr").remove();
    if(table_body.find("tr:visible").length==0){
      console.log("No more row!");
      $(".no_entries_row").show();
    }
  });
  
  // Select "success/wrong/error"
  $(document).on("change",".label",function(){
    console.log("Changed color");
    $(this).closest("tr").css({"background-color":$(this).val()});
  });
});  // End ready
* {
    box-sizing: border-box;
    padding: 0;
    margin: 0;
}
body {
    padding: 30px;
    font-family: arial;
}
table {
    width: 100%;
    background-color: #fff;
    text-align: left;
    border: 1px solid #ccc;
    border-collapse: collapse;
    margin-bottom: 10px;
}
tbody tr:not(:last-of-type), thead tr { border-bottom: 1px solid #ccc }
thead th {
    background: #f0f0f0;
    padding: 10px;
}
td { padding: 10px 10px 10px 0 }
.no_entries_row > td { padding-left: 60px }
.btn_col, .controls { text-align: center }
.controls { padding: 0 }
a.list_cancel { color: #000 }
a.list_add {
    text-decoration: none;
    color: #54A2CE;
}
i[class*="fa fa-plus-square"] { color: #999999 }
.action_btn {
    display: flex;
    flex-flow: row wrap;
    align-items: center;
    width: auto;
    justify-content: center;
    margin-top: 20px;
}
.action_btn input { margin: 0 }
input[name~="submit"] {
    padding: 5px 15px;
    font-size: 1em;
    margin: 0 10px;
}
.popup {
    flex: 1 100%;
    margin-top: 30px;
    text-align: center;
    font-size: 1.5em;
    display: none;
}
.popup > p {
    padding: 20px 50px;
    background: lightgreen;
    font-weight: 700;
    border-radius: 15px;
}
@media (min-width: 950px) {
   .popup > p {
       width: 25%;
       margin: 0 auto;
    } 
}
input, select {
    width: 100%;
    border-radius: 5px;
    padding: 5px;
    border: 1px solid #ccc;
}
select {
    -webkit-appearance: menulist-button;
    -moz-appearance: menulist-button;
    appearance: menulist-button;
    padding: 5px;
    height: 25px;
}

/* ============ ADDED =========== */
.template_row{
  display:none;
}
<link href="https://cdnjs.cloudflare.com/ajax/libs/font-awesome/4.7.0/css/font-awesome.min.css" rel="stylesheet" />
<script src="https://cdnjs.cloudflare.com/ajax/libs/jquery/3.3.1/jquery.min.js"></script>

<div class="wrapper">

  <table class="common_table" cellspacing="0">

    <thead>
      <tr>
        <th class="btn_col"></th>
        <th class="code_col">Demo Column 1</th>
        <th>Demo Column 2</th>
        <th class="data_col">Demo Column 3</th>
        <th class="data_col">Demo Column 4</th>
        <th class="short_col">Col 5</th>
        <th class="data_col">Demo Column 6</th>
      </tr>
    </thead>
    <tbody>

      <tr class="template_row">
        <td class="controls"><a href="#" class="list_cancel" title="Delete Row"><i class="fa fa-minus-circle"></i></a></td>
        <td><input type="text" class="" id="productcode" name="" value="" /></td>
        <td><input type="text" class="long" id="productname" name="" value="" /></td>
        <td><input type="text" class="" id="stock" name="" value="" /></td>
        <td><input type="text" class="" id="vendor" name="" value="" /></td>
        <td>
          <select name="" id="" class="label">
            <option value=""></option>
            <option value="green" style="background-color:green;">Success</option>
            <option value="red" style="background-color:red;">Wrong</option>
            <option value="orange" style="background-color:orange;">Error</option>
          </select>
        </td>
        <td><input type="text" class="" id="client" name="" value="" /></td>
      </tr>
      <tr class="no_entries_row">
        <td colspan="7">No Entries</td>
      </tr>

    </tbody>
  </table>

  <a href="#" class="list_add"><i class="fa fa-plus-square"></i> Add one row</a>

  <br class="clear" />

  <div class="action_btn">
    <input name="submit" class="action_btn submit" type="submit" value="Save" />
    <input name="submit" class="action_btn cancel" type="submit" value="Cancel" />
    <div class="popup">
      <p>Saved successfully!</p>
    </div>
  </div>

</div>

CodePen

Advertisement