How to move this inline JS to a file

Viewed 95

I have this (simplified) inline JS which i want to move into a js file. Trying to clean up inline JS and CSS

<td><input type="number" id="total" class="main-input" onkeyup="calculate(this.value)" /> </td>

in a js file i already moved the function

function calculate(total) {
   // I do some calculations here and it returns the values live(very important) into other <td>'s
}

but i struggle greatly to move the inline onkeyup event. So what i tried to do inside my js file was this:

totalValue = parseInt(document.getElementById("total").value);
document.getElementById("total").onkeyup = function() {calculate(totalValue)};

but it doesn't work... it reads it as 0.

I tried with AddEventListener too.. no luck

3 Answers

Try this. Use addEventListener, I have write a simple code below, just to print in console on keyup.

document.getElementById("total").addEventListener('keyup' ,function() {console.log('func called')});
<td><input type="number" id="total" class="main-input"  /> </td>

Just for reference https://www.w3schools.com/jsref/met_element_addeventlistener.asp

The issue with your code:

// this stores the initial value of the input in `totalValue`
totalValue = parseInt(document.getElementById("total").value);

// this calls the update on every keyup, but uses the initial value 
// that you had stored when the script first ran
document.getElementById("total").onkeyup = function() {calculate(totalValue)};

But to add some suggestions:

  • Declare your variables with const/let. Among other perks it enforces you to be more organized.
  • Grab the element out of DOM only once and store it as a variable. Tiny performance improvement and tiny readibility and cleanliness improvement.
  • Use addEventListener, it allows adding multiple listeners, i.e. you don't override the previous listener.

So here's what I suggest:

const totalInput = document.getElementById('total')

// Using a named function allows to also detach event if needed
totalInput.addEventListener('keyup', handleKeyupOnTotalInput)

function handleKeyupOnTotalInput() {
    const value = parseInt(totalInput.value)
    calculate(value)
}

// But if you prefer shorter, you could do this:
// totalInput.addEventListener('keyup', _ => calculate(parseInt(totalInput.value)))

Or even condense everything in a single statement (but split into multiple lines for readability):

document
    .getElementById('total')
    .addEventListener('keyup', event => calculate(parseInt(event.target.value)))

Try using addEventListener:

const total = document.getElementById("total");

total.addEventListener("keyup", () => {
  console.log(total.value);
});

As you can see, you should access the value of total, inside the callback function of the event listener, so you always have the updated value available. Otherwise, in your code, totalValue is assigned a value when it's run for the first time, but not ever again. So the value will always stay the same, no matter what you write in the input field. To see that happen, try running this as an experiment:

const total = document.getElementById("total");

const value = total.value

total.addEventListener("keyup", () => {
  console.log(value);
});

Once this javascript file is executed, value is set to the value of the total input field, which is an empty string (""). So whenever you have a keyup event on the input field, it will log out an empty string.

By accessing the value of total inside the callback function to the event listener, you avoid this issue.

Related