Conversation
beccaelenzil
left a comment
beccaelenzil
left a comment
There was a problem hiding this comment.
Great work on your first javascript on the web project! You've met the learning goals around dynamically applying styles and event handling. I've left a few in-line comments on small ways you may consider refactoring. Nice work!
| </head> | ||
| <body> | ||
|
|
||
| <header id="city_name_header"> |
| change_widgets; | ||
| const createGroundListener = () => { | ||
| document.getElementById('temp_display').addEventListener('DOMSubtreeModified', function () { | ||
| if (document.getElementById('temp_display').innerHTML <= 44) { |
There was a problem hiding this comment.
Nice work dynamically implementing these styles. It looks great!
Notice that there is quite a bit of repeated code here. Here are a couple ways you might consider refactoring. 1) Rather than applying the style directly to the element, you could assign a class to each element here, and then use that class to apply to styles. Assign each element that you are dynamically styling to a constant, and then you can use that constant to either directly change the style or assign a class name.
|
|
||
| const createResetListener = () => { | ||
| const resetButton = document.getElementById('reset'); | ||
| resetButton.addEventListener('click', function(){ |
There was a problem hiding this comment.
For the callback functions in each of the addEventListener functions, consider encapsulating that functionality in a named function rather than an anonymous function to enhance readability.
| city_name_header; | ||
| change_widgets; |
There was a problem hiding this comment.
It looks like these lines can be removed
2 participants
No description provided.