I have a table with three columns as follows:
CATEGORY | ITEM | STATUS
The table is populated with data from a php script
I need to make sure that the cells under the header "STATUS" will change to be of red background and white text color if a value of "PRIORITY" is the result of the php data import.
Here is my js script
<script>
function c_color(){
if (document.getElementById('CellColor').value = 'PRIORITY') {
document.getElementById('CellColor').style.color = "white";
document.getElementById('CellColor').style.background="red";
}
}
c_color();
</script>
And this is the HTML/PHP that calls the function
<td id="CellColor" style="background-color: #92c38e; text-align:
center;">
<span style="color: #ffffff; font-size: medium;">
<?php print strip_tags($category[0]['status']); ?></span></td>
As a result I get only the first cell with the red and white colors but this isn't even showing a value of "PRIORITY"
I have tried and re-tried but I can't get it right, any help will be greatly appreciated.
I have a table with three columns as follows:
CATEGORY | ITEM | STATUS
The table is populated with data from a php script
I need to make sure that the cells under the header "STATUS" will change to be of red background and white text color if a value of "PRIORITY" is the result of the php data import.
Here is my js script
<script>
function c_color(){
if (document.getElementById('CellColor').value = 'PRIORITY') {
document.getElementById('CellColor').style.color = "white";
document.getElementById('CellColor').style.background="red";
}
}
c_color();
</script>
And this is the HTML/PHP that calls the function
<td id="CellColor" style="background-color: #92c38e; text-align:
center;">
<span style="color: #ffffff; font-size: medium;">
<?php print strip_tags($category[0]['status']); ?></span></td>
As a result I get only the first cell with the red and white colors but this isn't even showing a value of "PRIORITY"
I have tried and re-tried but I can't get it right, any help will be greatly appreciated.
Share Improve this question edited Jul 24, 2017 at 9:41 Lajos Arpad 77.2k40 gold badges117 silver badges222 bronze badges asked Jul 21, 2017 at 16:17 DiegoDiego 511 gold badge1 silver badge7 bronze badges 7- What do you mean "the first cell", do you have more cells with the same ID ? – adeneo Commented Jul 21, 2017 at 16:19
- Is this table loading via an ajax call? what about just adding a 'priority' class to the cell? – vector Commented Jul 21, 2017 at 16:33
- @adeneo I meant that the first cell below the header "STATUS" – Diego Commented Jul 21, 2017 at 17:14
- @vector the table cells values are populated by a PHP script that fetches the data via XML API call from a remote server – Diego Commented Jul 21, 2017 at 17:14
- @Diego, is there a reason you don't add the appropriate classes at render time in php depending on what the value of status is? I'm assuming here the rows and cells get rendered in a loop of some sort, no? – vector Commented Jul 21, 2017 at 19:07
4 Answers
Reset to default 2Use == or === for parison in JavaScript.
Your code:
if (document.getElementById('CellColor').value = 'PRIORITY')
assigns a value because it uses a single equal sign (which will always cause the if to enter the true branch).
Also, an HTML td element will not have a value property (only form elements do). Access the text contents of non-form elements with the textContent property.
Additionally, make sure that your JavaScript code is either placed just before the closing of the <body> element or that you have it in a callback for the window's DOMContentLoaded event to ensure that it doesn't run prior to the HTML elements it needs to use even being parsed.
Now, aside from those points, here's just a friendly "best-practice" suggestion. Try to get away from hard-coding or dynamically creating "inline styles" on HTML elements. That technique gets messy pretty quick and can create scalability issues because inline styles are very difficult to override when needed. Instead, write up CSS classes ahead of time and simply add or remove the classes as necessary.
Here's an example of that:
// The following code won't run until all the HTML has been parsed into memory
window.addEventListener("DOMContentLoaded", function(){
// Just scan the DOM for the element one time
var theCell = document.getElementById('CellColor');
// Here, we're just testing to see if the cell "contains" the text "PRIORITY",
// not that it exactly equals it.
if (theCell.textContent.indexOf('PRIORITY') > -1) {
// Just add the pre-made class and you're done
theCell.classList.add("special");
}
});td {
background-color: #92c38e;
text-align: center;
}
.span {
color: #ffffff;
font-size: medium;
}
.special {
color:white;
background-color:red;
}<table>
<tr>
<td id="CellColor">PRIORITY
<span class="span">span sample</span>
</td>
</tr>
</table>First let's fix the function:
function c_color(){
if (document.getElementById('CellColor').value === 'PRIORITY') {
document.getElementById('CellColor').style.color = "white";
document.getElementById('CellColor').style.background="red";
}
}
This modifies your assignment operator to parison of ===. Previously, you have given 'PRIORITY' to the value and checked whether it is truey, which was always the case. However, I still do not like this function, let's refactor it a little bit:
function c_color(element){
if (element.value === 'PRIORITY') {
element.style.color = "white";
element.style.background="red";
}
}
and now it is much more understandable, and more importantly, reusable, since it does not assume that the color of a tag having a CellColor id should be changed if it has a priority value. Thirdly, it will have a better performance, since the function was searching for a tag three times in the DOM previously, while the approach I have given reuses the element. Now, to achieve the previous behavior, you will have to call the function like this:
c_color(document.getElementById('CellColor'))
Note that it has to be found once and then it is reused.
The section below assumes that you have multiple instances of tags having the id of CellColor.
Now, we still have a problem. You have a CellColor id which, if duplicate, then only the first corresponding item will be found using document.getElementById and your HTML is invalid, since id should be unique in the document. You can overe your problem in hacky ways, but I do not remend them. The hacky ways are to either use document.querySelectorAll('#CellColor'), which will return an array-like object to you with all the corresponding cells or to gather your tr items using document.querySelectorAll and while you iterate them, search for your item in their context using getElementById. However, as I said, I do not remend them, since they overplicate your life and, more importantly, your HTML will still be invalid. Instead, the solution should be to modify CellColor to be a class instead of an id. Then, you could use document.getElementsByClassName('CellColor') or document.querySelectorAll('.CellColor'), or, if this is slow, search in the context of the ancestor table instead of document, like document.getElementById("#yourtableid").getElementsByClassName('CellColor').
You can't use getElementById if you are going to have the same id for more than one element (as seen here https://jsfiddle/n7evzzer/). getElementById will only apply to the first result, as it only returns the first element that it finds in the DOM. This is also invalid syntax, and if you ran it through the HTML validator it would throw an error. A document should only have unique id's within it.
I would remend that you use a class instead such as priority, and then color it based on the class.
Here you can do this 100% without JavaScript as seen here:
td {
text-align: center;
}
.priority {
color: white;
background: red;
}
td:not(.priority){
color: white;
background: #92c38e;
}<table>
<tr>
<td class="priority">
<span>PRIORITY</span>
</td>
</tr>
<tr>
<td>
<span>NOT A PRIORITY</span>
</td>
</tr>
<tr>
<td class="priority">
<span>PRIORITY</span>
</td>
</tr>
<tr>
<td class="priority">
<span>PRIORITY</span>
</td>
</tr>
<tr>
<td>
<span>NOT A PRIORITY</span>
</td>
</tr>
</table>... what about something like this. Add a class attribute and then add some css to color it:
.priority{
color:#fff;
background-color:red;
}
<td><span
style="color: #ffffff; font-size: medium;"
class="<?=strtolower($category[0]['status'])?>">
<?php print strip_tags($category[0]['status']); ?></span></td>