Why my logic is wrong? Checkbox and line-through

Question

I am a newbie in JS. Just want help to figure out why my logic is wrong, when I try to change the style of the text when the checkbox.checked === true.

Here is the code.

JS:

var c = document.getElementById("cbx");
var l = document.getElementById("cbxtxt");

if ( c.checked === true ) {
  l.style.textDecoration = "line-through";
}

HTML:

<html>
  <head></head>
  <body>
    <input type="checkbox" id="cbx">
    <label for="cbx" id="cbxtxt">Shaneningans</label>
    //<script type="text/javascript" src="cbx_test.js"></script>
  </body>
</html>

Thanks in advance!

I think == is enough, no need for === (Okay no :) didn't know that, javascript is weird) — Mr.Yellow, Mar 15 '19 at 13:00
@Mr.Yellow no need for a comparison at all, because the `.checked` property will always be either `true` or `false` — Pointy, Mar 15 '19 at 13:00
And what you are doing is checking if it is checked once, you need to write a function and call it with OnChange or something along those lines, see here https://stackoverflow.com/questions/21905663/call-oncheckedchanged-event-from-javascript — Mr.Yellow, Mar 15 '19 at 13:01
no need to use `=== true` because the `c.checked` is already a boolean without comparing to true — Scrimothy, Mar 15 '19 at 13:05
[How do I create a stack snippet?](https://meta.stackoverflow.com/questions/358992/ive-been-told-to-create-a-runnable-example-with-stack-snippets-how-do-i-do) — adiga, Mar 15 '19 at 13:08
As a side note and in this case, you don't need javascript for that. CSS `#cbx:checked ~ label { textDecoration: line-through; }` is enough — A. Wolff, Mar 15 '19 at 13:10

p.s.w.g · Accepted Answer · 2019-03-15T13:19:32.043

You need to wrap your logic in an event listener so that it runs every time the checkbox is checked / unchechecked. Also, you probably want to handle what happens when the checkbox is unchecked.

var c = document.getElementById("cbx"); // for checbox
var l = document.getElementById("cbxtxt"); // for label

c.addEventListener("change", function() {
  l.style.textDecoration = c.checked ? "line-through" : "none";
})

<input type="checkbox" id="cbx">
<label for="cbx" id="cbxtxt">Shaneningans</label>

To explain this line:

l.style.textDecoration = c.checked ? "line-through" : "none"

As others have said c.checked === true isn't really necessary, as you can just directly use c.checked as your condition. To make the code a bit more concise, I use the conditional operator (?:) instead of a an if / else.

Finally, just to demonstrate how @A. Wolff's suggestion of using pure CSS would work:

#cbx:checked~label {
  text-decoration: line-through;
}

<input type="checkbox" id="cbx">
<label for="cbx" id="cbxtxt">Shaneningans</label>

score 1 · Answer 2 · answered Mar 15 '19 at 13:03

You need to subscribe to checkbox's change event, otherwise your code only runs once when the <script> element is parsed.

Consider this:

var c = document.getElementById("cbx"); // for checbox
var l = document.getElementById("cbxtxt"); // for label

c.addEventListener('change', function(evt) { 
  //this anonymous function will run each time the checkbox changes state
  if (c.checked === true) {
    l.style.textDecoration = "line-through";
  } else {
    l.style.textDecoration = "";
  }
});

score 0 · Answer 3 · answered Mar 15 '19 at 13:09

you need to add event listener

var l = document.getElementById("cbxtxt"); // for label
var c = document.getElementById("cbx"); // for checbox

c.addEventListener('change', (event) => {
  if (event.target.checked) {
    console.log('checked')
    l.style.textDecoration = "line-through";
  } else {
    console.log('not checked')
     l.style.textDecoration = "blink";
  }
})

score 0 · Answer 4 · answered Mar 15 '19 at 13:35

You would rather use classes instead of id attributes.

I guess you'll need to display different checkboxes, that in your case must have different ids (the same id is not recommended on the page).

That's why you can use my snippet to figure out how to work with multiple checkboxes.

var checkboxes = document.querySelectorAll(".my-form .my-custom-checkbox-class"); // Search all checkboxes in the form

for (var checkBox of checkboxes) {
  checkBox.addEventListener("change", onCheckboxChangeHandler, false);  // Add event listener to each checkbox
}

function onCheckboxChangeHandler(e) {
  var clickedCheckbox = this
  var formGroupContainer = clickedCheckbox.closest('.form-group');   // Find closest parent element with class .form-group
  var label = formGroupContainer.querySelector('.my-custom-lbl-class');   // Find label closest to clicked checkbox
  label.style.textDecoration = clickedCheckbox.checked ? "line-through" : "none"; //Do everything you need with styles
}

.my-form {
  padding: 20px;
}

.form-group {
  padding: 5px 10px;
  font-size: 18px;
}

<div class="my-form">
        <div class="form-group">
            <input type="checkbox" id="cb1" class="my-custom-checkbox-class">
            <label for="cb1" class="my-custom-lbl-class">Waterfall</label>
        </div>

        <div class="form-group">
            <input type="checkbox" id="cb2" class="my-custom-checkbox-class">
            <label for="cb2" class="my-custom-lbl-class">River</label>
        </div>
    </div>

Why my logic is wrong? Checkbox and line-through

4 Answers4