Conversation
| @@ -0,0 +1,68 @@ | |||
| h1 { | |||
There was a problem hiding this comment.
many styles here and layouts could have been bootstrap classes
i.e. card, card-body, rounded, or shadow-sm
| forecastDiv.style.display = 'none'; | ||
|
|
||
| searchButton.addEventListener('click', () => { | ||
| const city = cityInput.value; |
There was a problem hiding this comment.
here the input needs to be trimmed
There was a problem hiding this comment.
ahh, yes. Thanks for this catch!
| }); | ||
| } | ||
|
|
||
| function fetchForecast(city) { |
There was a problem hiding this comment.
this function is too big. Need to be extracted into smaller chunks to help with readability and maintainability
There was a problem hiding this comment.
Thanks, Rony. How would this look as an example?
There was a problem hiding this comment.
the function is getting data and adding it to the dom, you can create a get data function that returns a promise of that data and then split the for loop logic as well.
The best you can do is ask yourself, is this function only fetching the forecast? not only, is fetching and... rendering, and arranging the data and displaying, etc... all these can be functions. I suggest you review this deeper with your mentor
| currentWeatherDiv.style.display = 'block'; | ||
| }) | ||
| .catch((error) => { | ||
| console.error('Error, didn\'t fetch weather', error); |
There was a problem hiding this comment.
this gives no feedback to the user
There was a problem hiding this comment.
What would be the corrected code to give feedback to the user?
There was a problem hiding this comment.
ideally would be an actual message that is rendered in the UI but for now alert could work as well
2 participants
No description provided.