Yael Peet Weather Report - #52
Conversation
| <header class="header__header"> | ||
| <h1>Weather Report</h1> | ||
| <span>For the lovely city of | ||
| <span id="headerCityName" class="header__city-name"></span></span> |
There was a problem hiding this comment.
We might want to hard code a default city in the HTML if we want a city to exist that we can get the real time temp for when the page first loads up.
|
|
||
|
|
||
|
|
There was a problem hiding this comment.
Small nitpitck: we should review our code to ensure we're consistently applying spacing between functions across the project to avoid confusion from readers/maintainers around if functions are grouped for a reason or if something was added or removed and the spacing wasn't updated.
| if (state.tempInt < 0 || state.tempInt > 120) { | ||
| state.tempValue.style.color = 'red'; | ||
| state.tempValue.textContent = 'DANGER' | ||
| landscape = '🚫🚫🚫TOO MUCH WEATHER🚫🚫🚫'; |
| const registerEventHandlers = () => { | ||
| loadControls(); |
There was a problem hiding this comment.
By adding the call to loadControls inside registerEventHandlers, the function is doing more than one thing. Thinking about testing, we'd have to do a bit more set up to test this function since it needs to confirm that both loading and registering happened successfully. I would consider reorganizing this to have a separate function named something like setUpPage that first calls loadControls then calls registerEventHandlers.
Our situation here is small enough that we may not see the benefit, but consider a larger project where maybe we could pass functions to use into the registerEventHandlers function. We'd want to be able to call that independently with different parameters without needing to reload controls that we already have references to.
| const handleCurrentTempButtonClicked = () => { | ||
| findLatAndLon() | ||
| .then((location) => getWeather(location)) | ||
| .then((targetTemp) => { | ||
| console.log(targetTemp) | ||
| state.tempInt = targetTemp; | ||
| applyColorAndGarden(); | ||
| }) | ||
| } |
There was a problem hiding this comment.
Great use of helpers and chaining promises! ✨
|
|
||
|
|
||
| }; | ||
| document.addEventListener("DOMContentLoaded", registerEventHandlers); |
There was a problem hiding this comment.
Really nice use of functions to organize the code overall ^_^
3 participants
No description provided.