Scissors - Christian #63
Conversation
| @@ -0,0 +1,88 @@ | |||
| <!DOCTYPE html> | |||
There was a problem hiding this comment.
Good, clear, semantic HTML. Nice work!
| @@ -0,0 +1,74 @@ | |||
| const setIndicatorColor = (tag, value) => { | |||
| if (value >= 80) { | |||
| tag.style.color = "red"; | |||
There was a problem hiding this comment.
Consider refactoring to assign a className, and then put the styles in the css file. For example:
#index.js
tag.className = “red”;
#styles.css
.red {
color: red;
}
| const setGroundLandscape = (tag, value) => { | ||
| if (value >= 80) { | ||
| tag.textContent = "🌵__🐍_🦂_🌵🌵__🐍_🏜_🦂"; | ||
| } else if ((value >= 70) && (value <= 79)) { |
There was a problem hiding this comment.
Minor note: the && is not necessary for this conditional test, we could just have:
| } else if ((value >= 70) && (value <= 79)) { | |
| } else if (value >= 70) { |
| }; | ||
| }; | ||
|
|
||
| const setGroundLandscape = (tag, value) => { |
There was a problem hiding this comment.
Notice that setGroundLandscape and setIndicatorColor have a lot of the same code. Consider how you might write a single function that achieves both pieces of functionality.
| var downArrow = document.getElementById("down-arrow"); | ||
| downArrow.addEventListener("click", decreaseTemp, false); |
There was a problem hiding this comment.
Consider moving all the Event Listener logic into a function called registerEventHandlers and add this code document.addEventListener("DOMContentLoaded", registerEventHandlers); to increase readability and changeability.
No description provided.