Today I'm working on making a simple counter with HTML / CSS and JS.

My javascript isn't working and I don't know why.. I read a lot on the internet and tried to add "async" but it seems that my code needs a review

let counter = document.getElementById('counter');
let increaseButton = document.getElementById('increase');
let decreaseButton = document.getElementById("decrease");
let resetButton = document.getElementById("reset");
let count = 0;

function functionIncrease() {
    count   ;
    counter.innerHTML(count);
    color();
}
function functionDecrease() {
    count--;
    counter.innerHTML(count);
    color();
}
function functionReset() {
    count = 0;
    counter.innerHTML(count);
    color();
}
function color() {
    if (count > 0) {
        counter.style.color = "green";
    } else if (count > 0) {
        counter.style.color = "red";
    }
}

increaseButton.addEventListener("click", functionIncrease());
decreaseButton.addEventListener("click", functionDecrease());
resetButton.addEventListener("click", functionReset())

<main>
    <h1>My Simple Counter</h1>
    <p id="counter">0</p>
    <div >
        <button id="decrease">Decrease</button>
        <button id="reset">Reset</button>
        <button id="increase">Increase</button>
    </div>
</main>

I think it's just a stupid error, but I really need your help !

Thank you!

CodePudding user response：

As your error says innerHTML is not a function, it's a variable.
change it to =

And remove the brackets of the listeners, you should pass the function reference not call it.

let counter = document.getElementById('counter');
let increaseButton = document.getElementById('increase');
let decreaseButton = document.getElementById("decrease");
let resetButton = document.getElementById("reset");
let count = 0;

function functionIncrease() {
    count   ;
    counter.innerHTML = count "";
    color();
}
function functionDecrease() {
    count--;
    counter.innerHTML = count "";
    color();
}
function functionReset() {
    count = 0;
    counter.innerHTML = count "";
    color();
}
function color() {
    if (count > 0) {
        counter.style.color = "green";
    } else if (count > 0) {
        counter.style.color = "red";
    }
}

increaseButton.addEventListener("click", functionIncrease);
decreaseButton.addEventListener("click", functionDecrease);
resetButton.addEventListener("click", functionReset)

<main>
    <h1>My Simple Counter</h1>
    <p id="counter">0</p>
    <div >
        <button id="decrease">Decrease</button>
        <button id="reset">Reset</button>
        <button id="increase">Increase</button>
    </div>
</main>

CodePudding user response：

There are many improvements to make. Clean and efficient coding will also resolve the issue at hand by itself.

1. Don't add a single eventListener to every button itself that will call an independent function. Use querySelectorAll to address all buttons in one call. You get a Node-List which you can use the forEach iteration on.
2. Use an event delegation to check what element has been clicked. Then you can return the id of the clicked button with event.target.id.
3. Use a switch statement to either decrease, increase, or reset the counter instead of using independent functions with literally 2/3 of the same content.
4. Use textContent which is faster then innerHTMLas the DOm doesn't need to be re-parsed. Also, it poses no security issue of a potential XSS Injection.
5. Instead of calling a new function to color the counter, simply use a ternary conditional operator.

let counter = document.querySelector('#counter'),
    button = document.querySelectorAll('.button button'),
    count = 0;

button.forEach(el =>
  el.addEventListener('click', function(event) {
    switch (event.target.id) {
      case 'decrease':
        count--;
        break;
      case 'reset':
        count = 0;
        break;
      case 'increase':
        count  ;
        break;
    }
    counter.textContent = count;
    counter.style.color = (count > 0) ? 'green' : 'red';
  })
)

<main>
  <h1>My Simple Counter</h1>
  <p id="counter">0</p>
  <div >
    <button id="decrease">Decrease</button>
    <button id="reset">Reset</button>
    <button id="increase">Increase</button>
  </div>
</main>

CodePudding user response：

Issues Found

• Dont call the listener function while registering the event listner. Just mention the function reference. Just like increaseButton.addEventListener("click", functionIncrease);
• Also the innerHTML should be set with counter.innerHTML = count

Fixed Code

let counter = document.getElementById("counter");
let increaseButton = document.getElementById("increase");
let decreaseButton = document.getElementById("decrease");
let resetButton = document.getElementById("reset");
let count = 0;

function functionIncrease() {
  count  ;
  counter.innerHTML = count;
  color();
}
function functionDecrease() {
  count--;
  counter.innerHTML = count;
  color();
}
function functionReset() {
  count = 0;
  counter.innerHTML = count;
  color();
}
function color() {
  if (count > 0) {
    counter.style.color = "green";
  } else if (count > 0) {
    counter.style.color = "red";
  }
}

increaseButton.addEventListener("click", functionIncrease);
decreaseButton.addEventListener("click", functionDecrease);
resetButton.addEventListener("click", functionReset);

<main>
  <h1>My Simple Counter</h1>
  <p id="counter">0</p>
  <div >
    <button id="decrease">Decrease</button>
    <button id="reset">Reset</button>
    <button id="increase">Increase</button>
  </div>
</main>

CodePudding user response：

On your addEventListener, you have to change with functions names.

Also the .innerHTML was wrong : counter.innerHTML(count); to counter.innerHTML = count;

var counter = document.getElementById('counter');
var increaseButton = document.getElementById('increase');
var decreaseButton = document.getElementById("decrease");
var resetButton = document.getElementById("reset");
var count = 0;

function functionIncrease() {
    count   ;
    counter.innerHTML = count;
    color();
}
function functionDecrease() {
    count--;
    counter.innerHTML = count;
    color();
}
function functionReset() {
    count = 0;
    counter.innerHTML = count;
    color();
}
function color() {
    if (count > 0) {
        counter.style.color = "green";
    } else if (count > 0) {
        counter.style.color = "red";
    }
}

increaseButton.addEventListener("click", functionIncrease);
decreaseButton.addEventListener("click", functionDecrease);
resetButton.addEventListener("click", functionReset)

<!DOCTYPE html>
<html>
    <head>
        <title>Simple Counter</title>
        <meta name="viewport" content="width=device-width, initial-scale=1" />
        <link href="styles.css" rel="stylesheet">
        
    </head>
    <body>

        <main>
            <h1>My Simple Counter</h1>
            <p id="counter">0</p>
            <div >
                <button id="decrease">Decrease</button>
                <button id="reset">Reset</button>
                <button id="increase">Increase</button>
            </div>
        </main>
        
        <script src="./script.js" async></script>
    </body>
</html>

CodePudding user response：

let counterDisplayElem = document.querySelector('.counter-display');
let counterMinusElem = document.querySelector('.counter-minus');
let counterPlusElem = document.querySelector('.counter-plus');
let reset=document.querySelector('.reset');
let count = 0;

updateDisplay();

counterPlusElem.addEventListener("click",()=>{
    count  ;
    updateDisplay();
    color();
}) ;

counterMinusElem.addEventListener("click",()=>{
    count--;
    updateDisplay();
    color();
});

reset.addEventListener("click",()=>{
    count=0;
    updateDisplay();
});

function updateDisplay(){
    counterDisplayElem.innerHTML = count;
};

function color() {
    if (count > 0) {
        counterDisplayElem.style.color = "green";
    } else if (count < 0) {
        counterDisplayElem.style.color = "red";
    }
}

<h1 >(..)</h1>
<button >-</button>
<button > </button>
<button >Reset</button>

CodePudding user response：

I think the problem is

increaseButton.addEventListener("click", functionIncrease());
decreaseButton.addEventListener("click", functionDecrease());
resetButton.addEventListener("click", functionReset())

You have to replace it to

increaseButton.addEventListener("click", functionIncrease);
decreaseButton.addEventListener("click", functionDecrease);
resetButton.addEventListener("click", functionReset)

addEventListener requires a function to be called, but you are passing the result of the function, not the function itself.