2

I am making a table in html that uses javascript to add a new row, have editable cells and then allows the user to click save and the edits to the cells become saved. However, the code I wrote only works and does this in the first row that is added. When I attemp to add a new row, my save() function no longer works. I know this is because I am accessing the table cells with ids and so multiple elements have the same id, but I do not know how to fix it.

<table id="myTable"> 
<tr>
<th>1</th>
<th>2</th>
</tr>
</table>
<button onclick="addRow()">Add row button</button>
<script>
function addRow() {

  let tableRef = document.getElementById('myTable');

  let newRow = tableRef.insertRow(-1);

  let firstcell = newRow.insertCell(0);

  firstcell.setAttribute("id","firstcell")

  let secondcell = newRow.insertCell(1);
  secondcell.setAttribute("id","secondcell")

  var input1=document.createElement('input')

  secondcell.appendChild(input1);

  var btn = document.createElement('button');

btn.type = "button";

btn.className = "save";

let btntext= document.createTextNode("save button");

btn.appendChild(btntext)

btn.setAttribute("onclick","save()")

firstcell.appendChild(btn);

}



function save(){

  input1rref=document.getElementById("input1")

  input1value=input1rref.value

  input1text=document.createTextNode(input1value);

  x=document.getElementById("secondcell")

  x.replaceChild(input1text,input1rref)
}
</script>
<style>
  table{
    padding:10px;
    border: 2px solid black;

  }
  button{
    padding:10px 20px;
  }
</style>
cfer0923
  • 21
  • 2
  • Please provide a full [mre] (HTML, CSS and full JavaScript - at least a curly brace is missing from save function) – Can O' Spam Jul 22 '22 at 15:24
  • `commentin=document.getElementById("inputcomment")` - if you have two with the ID "inputcomment" which should this grab? IDs must be unique within the document. doc.getElById will always get the first one in the document as they're supposed to be unique. Remove your IDs and uses classes and relative DOM navigation. – freedomn-m Jul 22 '22 at 15:25
  • I've fixed my question and replaced the previous code with a minimal reproducible example – cfer0923 Jul 22 '22 at 15:47

2 Answers2

1

If you have more than one form control (ex. <input>, <select>, <button>, etc), wrap everything in a <form>. Use .class, tagName, name, and [attribute] to reference elements. As you can see in the example below, id is used only once to reference the <form>, but even that could've been referenced by name easily.

Figure I - HTML Layout

<form id='formID' name='formName'>
  <input id='inputID' name='formControl' type='number'>
  <button name='formControl' type='button'>GO!</button>
  <table class='tableClass'>
    <tr><td></td><td></td></tr>
  </table>
  <select name='formControl'>
    <option value='1'>I</option>
    <option value='2'>II</option>
    <option value='3'>III</option>
    <option value='4'>IV</option>
  </select>
  <output name='outputName'></output>
</form>

Figure II - Referencing Elements

// Reference with .querySelector() to get the first element match
// "." prefix for class
const table = document.querySelector('.tableClass'); 
// "#" prefix for id
const form = document.querySelector('#formID');
// "[]" wrap for other attributes
const input = document.querySelector('[name="formControl"]');
// No prefix or wrap for tagName
const button = document.querySelector('button');
// Use .querySelectorAll() to gather all matches into a NodeList
const options = document.querySelectorAll('option')
// Reference the third <option>
const third = options[2]
// HTMLFormElement Interface
const form = document.forms.formID;
const formALT1 = document.forms.formName;
const formALT2= document.forms['formName'];
const formALT3 = document.forms[0];
// References all form controls under <form>
const fc = form.elements
// Reference form controls id, name, or index notation
const input = fc.inputID;
const output = fc.outputName;
const select = fc[2];
// HTMLFormControlsCollection Interface
// Group all form controls that share the same name
const allFCNames = fc.formControl

Read the following

Events

Event Delegation

Inline Event Handlers are Garbage

HTMLFormElement

HTMLFormControlsCollection

Form Controls

Details are commented in example

/**
 * @desc  Utility function that will append a given htmlString into
 *        a given element and renders it into HTML.
 * @param {string|object} selector - If passed as a string it will
 *        find element by CSS selector. If passed as a variable, it
 *        will reference a DOM Object. This is the object to append
 *        HTML to.
 * @param {string} htmlString - A string that resembles HTML layout
 */ 
function setHTML(selector, htmlString) {
  const node = typeof selector === 'string' ?
    document.querySelector(selector) :
    selector;
  node.insertAdjacentHTML('beforeend', htmlString);
}
// Reference the <form>
const tForm = document.forms.interface;
// Bind the click event to <form>
tForm.onclick = action;
// Bind the change event to <form>
tForm.onchange = format;

// Event handler passes Event Object by default
function action(e) {
  // e.target is the <button> the user clicked
  // The click event is delegated to the <button> clicked by className
  switch (e.target.className) {
    case 'add':
      addRow(e);
      break;
    case 'delete':
      deleteRow(e);
      break;
    case 'edit':
    case 'edit open':
      editRow(e);
      break;
    default:
      break;
  }
}

function addRow(e) {
  // Reference <tbody>
  const tData = document.querySelector('.data');
  // Reference <thead>
  const tHead = tData.parentElement.createTHead();
  // Get the number of columns
  let cQty = tHead.rows[0].cells.length;
  // Define the value/text of each <option> in an array
  const opts = ["Select Benchmark", "Cash", "Munis", "Bonds", "Stocks", "Commods", "Alts"];
  // Reference inserted <tr>
  let tRow = tData.insertRow();
  // Iterate >cQty< times
  for (let i = 0; i < cQty; i++) {
    // Reference inserted <td>
    let tCel = tRow.insertCell();
    /*
    Determine which is the current <td> by (i)ndex
    Each case creates a <td> and it's content
    Some elements have too many attributes so using .setAttribute()
    would bloat the code signifigantly. In those cases render the 
    HTML instead (case 1, case 2, and case 4)
    */
    switch (i) {
      case 0:
        const sel = document.createElement('select');
        sel.name = 'benchmark';
        opts.forEach(o => {
          let opt = new Option(o, o);
          sel.add(opt);
        });
        tCel.append(sel);
        break;
      case 1:
        const tilt = `<input name='tilt' class='open' type='number' min='0' step='.01' placeholder='0.00'>`;
        setHTML(tCel, tilt);
        break;
      case 2:
        const comment = `<input name='comment' class='open' type='text' placeholder='Enter comment'>`;
        setHTML(tCel, comment);
        break;
      case 3:
        const log = document.createElement('time');
        log.textContent = new Date().toLocaleString();
        log.className = 'log';
        tCel.append(log);
        break;
      case 4:
        const buttons = `
        <button name='btn' class='delete' type='button' title='Delete Row'>➖</button>
        <button name='btn' class='edit open' type='button' title='Edit Row'>✏️</button>`;
        setHTML(tCel, buttons);
        break;
      default:
        break;
    }
  }
}
// Here we simply find the closest <tr> and remove it
function deleteRow(e) {
  e.target.closest('tr').remove();
}
// This function just toggles the 2 <input> [readonly] attributes
function editRow(e) {
  e.target.classList.toggle('open');
  const row = e.target.closest('tr');
  const inputs = row.querySelectorAll('input');
  inputs.forEach(inp => {
    inp.toggleAttribute('readonly');
    inp.classList.toggle('open');
  });
}
// Formats a value into a float with 2 decimal places
function format(e) {
  if (e.target.matches('[name="tilt"].open')) {
    e.target.value = parseFloat(e.target.value).toFixed(2);
  }
}
html {
  font: 300 2.5vmin/1 'Segoe Ui';
}

table {
  table-layout: fixed;
  border-collapse: collapse;
  width: 96%;
}

th {
  border-bottom: 3px solid black;
}

th:first-of-type,
th:nth-of-type(2) {
  width: 11rem;
}

th:nth-of-type(3) {
  width: 17rem;
}

th:nth-of-type(4) {
  width: 13rem;
}

th:nth-of-type(5) {
  width: 9rem;
}

td {
  text-align: center;
  padding: 4px;
}

input,
select,
button {
  display: inline-flex;
  align-items: center;
  font: inherit;
}

button {
  cursor: pointer;
}

input {
  border: 0;
}

.open {
  border-radius: 4px;
  outline: 3px inset blue;
  outline-offset: 1px;
}

[name="tilt"] {
  width: 9rem;
  text-align: right;
  font-family: Consolas;
}

[name="comment"] {
  width: 17rem;
}

time {
  width: 10rem;
}
<form id='interface'>
  <table>
    <thead>
      <tr>
        <th>Benchmark</th>
        <th>Tilt</th>
        <th>Comment</th>
        <th>Time Log</th>
        <th><button name='btn' class='add' type='button'>➕ Add Row</button></th>
      </tr>
    </thead>
    <tbody class='data'></tbody>
  </table>
</form>
zer00ne
  • 41,936
  • 6
  • 41
  • 68
0

Reason is you are assigning the same ids to the controls in multiple rows (as the user adds rows).

What you should do:

  1. Keep track of the number of rows and append row number to the controls in all the setAttribute("id"... lines. Example:
superassetCell.setAttribute("id", "benchmarkcell");

// should be 
superassetCell.setAttribute("id", "benchmarkcell" + row_number);

getElementById works by definition for element with the unique id.

  1. When calling save method, each row should pass the row number as argument to save.

Checkout this question

ram adhikari
  • 323
  • 1
  • 5