Button onclick event is not working in innerHTML of table data, while deleting a data from local storage with a button

Viewed 134

I was developing a localstorage crud operation. I stored the data's using a table. It works perfectly fine. In the action Delete button is set to delete the key value of the local storage. The button onclick is working with other function. But not working with the function cleardata.
Visit fiddle to see the code https://jsfiddle.net/saif_lesnar/vkwgq5z9/1/

<html>
<head>
    <style>
     table,th,td {
        border: 1px solid black;
        border-collapse: collapse;
    }
    </style>
    <script src="local.js"></script>
</head>
<body >
   <section >
        <form>
            <p>User Name:<br> <input type="text" id= "key"/></p>
            <p>User Number:<br><input type="number" id="value"/></p>
            <p>User Address:<br><input type="text" id="data"/></p>
            <p><input type="submit" id= "button" value="Submit"/></p>
        </form>
    </section>
    <table  id="display_data">
        <tr>
            <th>
                User Name
            </th>
            <th>
                User Number
            </th>
            <th>
                User Address
            </th>
            <th>
                Action
            </th>
        </tr>
    </table>

</body>
</html>

Heres the js code

function doFirst(){
    var button = document.getElementById('button');
    button.addEventListener("click",save,false);
    display();
}
function save(){
    var key = document.getElementById("key").value;
    var value = document.getElementById("value").value;
    var data = document.getElementById("data").value;
    var obj=[
        value,data
    ];
    localStorage.setItem(key, JSON.stringify(obj));
    display();
    key.value="";
    value.value="";
    data.value="";
}
function display(){
    var display_data = document.getElementById("display_data");


    for(var i = 0; i <localStorage.length; i++){
        var a = localStorage.key(i);
        var b = JSON.parse(localStorage.getItem(a));        
        console.log(a);
        display_data.innerHTML +="<tr><td>"+a+"</td><td>"+b[0]+"</td><td>"+b[1]+"</td><td><button onclick='cleardata("+a+")' type='button'>Delete</button></td></tr>";

        } 

    }
function cleardata(data){
    console.log(data);

    localStorage.removeItem(data);
    location.reload();
}

window.addEventListener("load",doFirst,false)

Please help me out.

2 Answers

Your code is almost correct. your issue it's on this line :

   display_data.innerHTML +="<tr><td>"+a+"</td><td>"+b[0]+"</td><td>"+b[1]+"</td><td><button onclick='cleardata("+a+")' type='button'>Delete</button></td></tr>";

The part below have an issue;

cleardata("+a+")

The behavior of this function will be : cleardata(var_name). You have to escape to precise it's a string and not a var. Like below :

display_data.innerHTML +="<tr><td>"+a+"</td><td>"+b[0]+"</td><td>"+b[1]+"</td><td><button onclick='cleardata(`"+a+"`)' type='button'>Delete</button></td></tr>";

It's works well on my side. But referring to the console you still have an error in your json parse.

It's faster to give you a quick response, but I see in your code some bad practice, so I would like to share with you, the way as you should code,

The first thing is that you will separate the responsibility, so ¿ What is that ?, you should create functions that are responsible for just one thing and that thing should be done very well, so you should separate the logic for save user into localStorage, that's because localStorage can be reused when your load all users already loaded into localStorage, in that way you can write less code in a modular way.

The second your display function should only care about just displaying the row itself, but not should knowing about localStorage, that is for reusing the same function to use when load all users in the first time and use same function when add a new user / item.

So I took a little time to make a working example based with this principle, Single Responsability

// get refs for table and save button elements
const tableUsers = document.getElementById('table-users');
const saveButton = document.getElementById('button-save');

// declare some constants
const deleteButtonClassName = 'delete-button';
const storageNameSpace = 'users';

// check if the element passed as argument is the deletable button
function isButtonDelete(target) {
 return String(target.nodeName).toLowerCase() === 'button' && target.classList.contains(deleteButtonClassName);
}

// create a deletable button to be appended when new user is created
function createDeleteButton(text) {
 const button = document.createElement('button');
  button.classList.add(deleteButtonClassName);
  button.innerText = text;
  return button; 
}

// the addNewUserRow function will care about only for rendering the new user, this function doesn't know about localStorage engine, so it's pure
function addNewUserRow(user) {
 const row = document.createElement('tr');
 const colName = document.createElement('td');
  colName.innerText = user.name;
  
  const colNumber = document.createElement('td');
  colNumber.innerText = user.number;
  
  const colAddress = document.createElement('td');
  colAddress.innerText = user.address;
  const colAction = document.createElement('td');
  const deleteButton = createDeleteButton('Borrar');
  
  deleteButton.dataset.id = user.id;
  row.setAttribute('id', user.id);
  
  colAction.appendChild(deleteButton);
  [colName, colNumber, colAddress, colAction].forEach((child) => row.appendChild(child));
  tableUsers.appendChild(row);
}

/*
this method will be attach to onClick on table element, so we are delegating all the events to table element.

So why should you do it like this ?

just to avoid allocating to much memory to each row created by the user, so you are attaching onClick="yourFunctionName" to each button created when a new user is going to be appended, when you do that you are linking a handle to a on click event for that button, so instead of doing that, we are delegating all events to the main parent in this case the table element.
*/
function handleActions(event) {
 if (isButtonDelete(event.target)) {
   return deleteRow(event);
  }
}

// will delete the user into localStorage and then will remove the row into the table
function deleteRow(event) {
 const button = event.target;
  const id = button.dataset.id;
  
  if (id) {
   removeUserInStorage(id);
   document.getElementById(id).remove();
  }
}

// this method will care about to create an user object from inputs and then will clear each input
function buildUserObject(inputs) {
 const user = { id: Date.now() };
  for(var i = 0; i < inputs.length; i++) {
   const input = inputs[i];
    const name = input.name;
    const value = input.value;
    user[name] = value;
    input.value = '';
  }
  return user;
}

// this method will get the users data into localStorage, note that if for any reason we get an error from JSON.parse will return an empty object or just if there is not users yet.
function getUsers() {
 const users = localStorage.getItem(storageNameSpace);
  const defaultObject = {};
  try {
   return JSON.parse(users) || defaultObject;
  } catch(error) {
   return defaultObject;
  }
}

// this function will save the users data into localStorage
function saveUsers(users) {
 localStorage.setItem(storageNameSpace, JSON.stringify(users));
}

// this function will add new user into users object to be saved later
function saveInLocalStorage(user) {
 const users = getUsers();
  users[user.id] = user;
 saveUsers(users);
}

// this method will delete the user into the users object and then will save them
function removeUserInStorage(id) {
 const users = getUsers();
  delete users[id];
  saveUsers(users);
}

// this function will be called when a new user is created
function saveUser() {
 const inputs = document.getElementsByClassName('input-user');
  const userObject = buildUserObject(inputs);
  saveInLocalStorage(userObject);
  addNewUserRow(userObject);
}

// this function will call only when the page is loaded, to load all users keep into localStorage
function loadUsers() {
 const users = getUsers();
  const ids = Object.keys(users);
  ids.forEach((id) => {
   const user = users[id];
    addNewUserRow(user);
  });
}

// setup the listeners needed to run our app
tableUsers.addEventListener('click', handleActions, false);
saveButton.addEventListener('click', saveUser, false);

// load users if exist
loadUsers();
.input-user {
  width: 90%;
  padding: 10px;
  margin-bottom: 5px;
}

.input-container{
  text-align: center;
}

#button-save {
  margin: 0 auto;
  width: 100%;
  background-color: #2ecc71;
  color: #fff;
  padding: 10px;
  margin-bottom: 10px;
}

table {
  width: 100%;
}
<div class="input-container">
<input placeholder="user name" class="input-user" name="name" /><br/>
<input placeholder="user number" class="input-user" name="number" type="number" /><br/>
<input placeholder="user address" class="input-user" name="address" /><br/>
</div>
<button id="button-save">
save user
</button>

<table id="table-users" border="1" cellspacing="0" cellpadding="5" >
  <th>User Name</th>
  <th class="delete-button" >User Number</th>
  <th>User Address</th>
  <th>Action</th>
</table>

I hope this code can help you and will improve your coding skills.

Related