CODEWITHSUNDEEP
javascript
dEd
dEd

Reputation: 17

Function is not behaving as intended. Why?

Sorry for not being more precise on the title.

https://jsfiddle.net/ptxsha6m/1/

<script>
var theID = document.getElementById('result1');

function change(){
    var checkboxes = document.getElementsByClassName('checkbox');
    var chekboxInputs = Array.from(checkboxes).map(a => a.querySelector('input'));
    var allAreUnselected = chekboxInputs.every(function(elem){
       return !elem.checked;
    });
    if(allAreUnselected){
       chekboxInputs.forEach(function(input){
          Array.from(document.querySelectorAll("." + input.getAttribute("rel"))).forEach(function(theID){
              theID.style.background = 'blue';
          });
       });
    }
    else {
      chekboxInputs.forEach(function(input){
          Array.from(document.querySelectorAll("." + input.getAttribute("rel"))).forEach(function(theID){
            theID.style.background = input.checked ? 'red' : 'gray';
          });
       });
    }
}
change();
</script>

Basically, as I wrote, it should change the div#id with the background yellow to red once a checkbox is selected. But for some reason it insists in targeting the div with class only.

I know there must be something wrong with the logic, there's a few things that I can tell but others I don't understand. For example, at "if(allAreUnselected)" shouldn't it set the background of the yellow box to blue? Since they are all unselected?

All I'm trying to achieve is be able to manipulate the div# inside the function. Any help?

Thanks

Upvotes: 0

Views: 102

Answers (2)

Elio
Elio

Reputation: 468

In line 41 what you think is the theID variable is wrong. In this case the variable theID will be shadowed from the variable that the forEach function passes as argument to the callback function(theID){..}.

forEach passes each item in the array as argument to the function you declare on each iteration. Try to change the name of this variable, ex. ...forEach(function(chekboxInput) { chekboxInput.style.background = 'blue';} nothing will change.

From here the theID is not anymore the div with id=result1 but one of the elements in the chekboxInputs that you are iterating on.

See also the answer from Daniel Beck.

Upvotes: 0

Daniel Beck
Daniel Beck

Reputation: 21475

Early in your script you capture the DOM element you're talking about in a variable named theID:

var theID = document.getElementById('result1');

Later on, where you're changing the color of the elements, you're using an anonymous function in a forEach, which has a parameter also named theID:

(/* lots of code */).forEach(function(theID){
    theID.style.background = 'blue'
});

That parameter name masks the variable defined earlier, so when you use theID inside that function it refers to each of the forEach elements instead of the #result1 element -- and you wind up never attempting to change the color of #result1.

If you want to change the color of that container, either use a different parameter name, so the variable isn't masked by it:

(/* lots of code */).forEach(function(foo){
    foo.style.background = 'blue'
    theID.style.background = // your logic here for deciding what color #result1 should be
});

...or do it outside the anonymous function:

if(allAreUnselected){
   theID.style.background = 'blue';
   chekboxInputs.forEach(function(input){
     (/* lots of code */).forEach(function(theID){   // (I'd still change that parameter name, though, because masked variables are confusing)

Upvotes: 5

Related Questions