4

I have the following HTML written. The idea is that I can add items to the list at the top, and then generate a field which includes a name, checkbox, and text field. The text field is to be enabled/disabled depending on the checkbox. In my javascript, the toggle function is assigned to the onclick attribute of the checkbox field, but it only works on the last item on the list. Can anyone tell why this functionality isn't being assigned to all the checkboxes? If you open the resulting html code in a browser, it shows no onclick events for any checkboxes except the last one, so it appears it isn't being added. Does it somehow get removed from the previous one when I assign it to the next? How would I fix it? Thank you.

<!DOCTYPE html>
<html>

<body onload="loadAllSettings()" }>
    <script>
        var genOptFields = ["genField1", "genField2"];

        function loadAllSettings() {
            loadSettingsList("genSet", genOptFields);
        }
    </script>
    <h2>Options</h2>

    <form>
        <fieldset id="genSet">
            <legend>General</legend>
        </fieldset>
    </form>


    <script>

        function loadSettingsList(parentId, optionalFields) {
            var fieldset = document.getElementById(parentId);
            for (fieldId of optionalFields) {
                var p = document.createElement('p');

                p.append(fieldId + ":");

                var input = document.createElement('input');
                input.type = "text";
                input.disabled = true;
                input.id = fieldId;

                var cb = document.createElement('input');
                cb.type = "checkbox";
                cb.id = "cb_" + fieldId;
                cb.addEventListener("click", function () {
                    toggleCheck(fieldId);
                });

                p.appendChild(cb);
                p.appendChild(input);

                fieldset.appendChild(p);
            }
        }
        function toggleCheck(fieldId) {
            document.getElementById(fieldId).disabled = !document.getElementById("cb_" + 
            fieldId).checked;
        }
    </script>

</body>

</html>
Kyle
  • 97
  • 5
  • 1
    My assumption would be that `fieldId` is not blocked scope, and this is a variation of the https://stackoverflow.com/questions/750486/javascript-closure-inside-loops-simple-practical-example duplicate. – Taplar Jan 30 '20 at 16:55

2 Answers2

0

As explained below, your fieldId reference isnt static. As a result when calling toggle check it was always passing the last value that fieldId contained no matter what (double check this by console.logging your fieldId passed to toggle check)

function loadAllSettings() {
  const genOptFields = ["genField1", "genField2"];
  loadSettingsList("genSet", genOptFields);
}
function loadSettingsList(parentId, optionalFields) {
    const fieldset = document.getElementById(parentId);
    optionalFields.forEach(function (fieldId) {
      createParagraph(fieldId, fieldset);
    });
}
function createParagraph(fieldId, fieldset) {
  const p = document.createElement('p');
  p.append(fieldId + ":");
  createCheckbox(p, fieldId);
  createInputField(p, fieldId);
  fieldset.appendChild(p);
}
function createInputField(p, fieldId) {
  const input = document.createElement('input');
  input.type = "text";
  input.disabled = true;
  input.id = fieldId;
  p.appendChild(input);
}
function createCheckbox(p, fieldId) {
  const cb = document.createElement('input');
  cb.type = "checkbox";
  cb.id = "cb_" + fieldId;
  //set this attribute to capture value
  cb.setAttribute('data-fieldId', fieldId);
  cb.addEventListener("click", function () {
    //use static data attribute value instead of fieldId var which isnt static
      toggleCheck(this.getAttribute('data-fieldId'));
  });
  p.appendChild(cb);
}
function toggleCheck(fieldId) {
    document.getElementById(fieldId).disabled = !document.getElementById("cb_" + fieldId).checked;
}
<!DOCTYPE html>
<html>

<body onload="loadAllSettings()" }>
    <h2>Options</h2>

    <form>
        <fieldset id="genSet">
            <legend>General</legend>
        </fieldset>
    </form>
</body>

</html>
tstrand66
  • 968
  • 7
  • 11
  • why downvotes? this does explains what is going on with the op's code. – tstrand66 Jan 30 '20 at 17:08
  • @Taplar thanks for the feedback. i believe these edits resolve that concern as well as clear up the paragraph declaration and bring the rest of the js code into the js file as i missed some on the first round – tstrand66 Jan 30 '20 at 17:18
0

Since you are generating the element using body on-load event, click event become static. That is why element is always pointing to the last child. You can simply achieve your requirement by passing element scope(this) into the click event.

Here is the working solution:

<body onload="loadAllSettings()" }>
        <script>
            var genOptFields = ["genField1", "genField2"];
    
            function loadAllSettings() {
                loadSettingsList("genSet", genOptFields);
            }
        </script>
        <h2>Options</h2>
    
        <form>
            <fieldset id="genSet">
                <legend>General</legend>
            </fieldset>
        </form>
        <script>
        function loadSettingsList(parentId, optionalFields) {
                var fieldset = document.getElementById(parentId);
                for (fieldId of optionalFields) {
                    var p = document.createElement('p');
    
                    p.append(fieldId + ":");
    
                    var input = document.createElement('input');
                    input.type = "text";
                    input.disabled = true;
                    input.id = fieldId;
    
                    var cb = document.createElement('input');
                    cb.type = "checkbox";
                    cb.id = "cb_" + fieldId;
                    cb.addEventListener("click", function () {
                        toggleCheck(this);
                    });
    
                    p.appendChild(cb);
                    p.appendChild(input);
    
                    fieldset.appendChild(p);
                }
            }
            function toggleCheck(ele) {
                ele.nextElementSibling.disabled = !ele.checked;
            }
            </script>
        </body>
nadun
  • 61
  • 5