Javascript for attribute assign not working-CodePudding

I have a table with different text values in it cells, and I want to assign different bgcolors to each cell, depending on their text.

Here's the code I've made -

var subjects = ["Geography", "English", "Biology", "Math", "Sanskrit", "Physics", "Chemistry", "History"];
        var colors = ["#DEF862", "#62F87F", "#62E2F8", "#7F62F8", "#F662F8", "#F8627A", "#F87062", "#E09484"];
        var elems = document.getElementsByTagName("td");
        for (var i=0; i<elems.length; i  ){
            if (subjects.includes(elems[i].textContent) == true){
                let ind = subjects.indexOf(elems[i].textContent);
                elems[ind].setAttribute("bgcolor", colors[ind]);
            }
        }

The length of elems is 25, and the loop goes through all of them. However, it only works for the first 10, after which is doesn't do anything.

Why is this happeing?

CodePudding user response：

The basic logic problem is you should use elems[i] instead of elems[ind] which is constrained by the length and indexing of the string arrays

CodePudding user response：

There is no bgcolor attribute on HTML elements (the global attributes are here), so setting it has no effect.

If you want to use inline styling (I wouldn't, I'd suggest using a class), you can do it by using the element's CSSStyleDeclaration object via its style property:

elems[i].style.backgroundColor = colors[ind];

Note also that I've changed ind to i, since as charlietfl points out, ind is the index in subjects, not elems.

Also, there's no point in calling includes and then turning around and calling indexOf, just call indexOf, it'll give you -1 in the case where the text isn't found:

var subjects = ["Geography", "English", "Biology", "Math", "Sanskrit", "Physics", "Chemistry", "History"];
var colors = ["#DEF862", "#62F87F", "#62E2F8", "#7F62F8", "#F662F8", "#F8627A", "#F87062", "#E09484"];
var elems = document.getElementsByTagName("td");
for (var i=0; i<elems.length; i  ){
    const ind = subjects.indexOf(elems[i].textContent);
    if (ind !== -1) {
        elems[i].style.backgroundColor = colors[ind]);
    }
}

Side note: If you are ever going to run the code again with a different input string, you probably want to remove the background from elements that no longer match, which is another reason for using a class rather than inline styling (you don't have to worry about restoring any previous custom value that may have been there before you changed it; you just remove the class).

I'd also use

An object array rather than parallel arrays
const and let rather than var (there's really no place for var in modern JavaScript code)
for-of rather than a for loop

...but it's a matter of style.

For what it's worth, here's my take:

const highlighting = [
    {subject: "Geography", color: "#DEF862"},
    {subject: "English", color: "#62F87F"},
    {subject: "Biology", color: "#62E2F8"},
    // ...
];
const elems = document.getElementsByTagName("td");
for (const elem of elems) {
    const text = elem.textContent;
    const highlight = highlighting.find(({subject}) => subject === text);
    if (highlight) {
        elem.style.backgroundColor = highlight.color;
    } else {                                // *** If needed
        elem.style.backgroundColor = "";    // ***
    }
}

Live Example:

Show code snippet

const highlighting = [
    {subject: "Geography", color: "#DEF862"},
    {subject: "English", color: "#62F87F"},
    {subject: "Biology", color: "#62E2F8"},
    // ...
];
const elems = document.getElementsByTagName("td");
for (const elem of elems) {
    const text = elem.textContent;
    const highlight = highlighting.find(({subject}) => subject === text);
    if (highlight) {
        elem.style.backgroundColor = highlight.color;
    } else {                                // *** If needed
        elem.style.backgroundColor = "";    // ***
    }
}

<table>
    <tbody>
        <td>Geography</td>
        <td>English</td>
        <td>Biology</td>
        <td>...</td>
    </tbody>
<table>

Or with a data-* attribute instead:

const subjects = ["Geography", "English", "Biology", "Math", "Sanskrit", "Physics", "Chemistry", "History"];
const elems = document.getElementsByTagName("td");
for (const elem of elems) {
    if (subjects.includes(elem.textContent)) {
        elem.setAttribute("data-subject", elem.textContent);
    } else {
        elem.removeAttribute("data-subject");
    }
}

with this CSS

[data-subject=Geography] {
    background-color: #DEF862;
}
[data-subject=English] {
    background-color: #62F87F;
}
[data-subject=Biology] {
    background-color: #62E2F8;
}
/* ... * /

Live Example:

Show code snippet

const subjects = ["Geography", "English", "Biology", "Math", "Sanskrit", "Physics", "Chemistry", "History"];
const elems = document.getElementsByTagName("td");
for (const elem of elems) {
    if (subjects.includes(elem.textContent)) {
        elem.setAttribute("data-subject", elem.textContent);
    } else {
        elem.removeAttribute("data-subject");
    }
}

[data-subject=Geography] {
    background-color: #DEF862;
}
[data-subject=English] {
    background-color: #62F87F;
}
[data-subject=Biology] {
    background-color: #62E2F8;
}
/* ... * /

<table>
    <tbody>
        <td>Geography</td>
        <td>English</td>
        <td>Biology</td>
        <td>...</td>
    </tbody>
<table>