-
Notifications
You must be signed in to change notification settings - Fork 85
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Ruby Kimberly & Sage #51
base: main
Are you sure you want to change the base?
Conversation
…llowing elements: header, temp section, sky section, city name section, and weather garden section.
Temp section
…nction. Landscape changes as temp is changed by user.
…lso updated in the header at the top of the page. Added event listeners for the reset button, wave 3 completed
…urrent temperature of specified city
…own and displays different sky settings. Also added event listener for this change
Sky section
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Excellent job! There are comments below on places to make the code cleaner/simpler. Great job!
<script src="./src/index.js"></script> | ||
<header class="header__header"> | ||
<h1>Weather Report</h1> | ||
<span>Follow your curio-city: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
😂 Excellent!
temp: 70, | ||
}; | ||
|
||
const convertKToF = (temp) => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Excellent!
formatTempAndGarden(); | ||
}) | ||
.catch((error) => { | ||
console.log('Could not retrieve the weather:') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minor nitpick, but it seems you meant to include printing the error, but didn't!
getWeather(); | ||
}) | ||
.catch((error) => { | ||
console.log('Could not find latitute and longitude:', error.response); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe errors don't have a response, you would just print the error directly.
}); | ||
}; | ||
|
||
const getCoordinates = async () => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Generally, if you add async
to the function definition, that means you are using await
instead of .then()
. Since you're using .then()
, you do not need the async
here.
gardenContent.classList = `garden__content ${skyColor}`; | ||
} | ||
|
||
const formatTempAndGarden = () => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Excellent!
newLandscape.textContent = landscape; | ||
}; | ||
|
||
const increaseTemp = () => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Excellent!
formatTempAndGarden(); | ||
}; | ||
|
||
const decreaseTemp = () => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Excellent!
const decreaseTempBtn = document.getElementById('decreaseTempBtn'); | ||
decreaseTempBtn.addEventListener('click', decreaseTemp); | ||
|
||
updateCityName(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This function call is not necessary when the page first loads since it's the one that needs to be called when users are typing something in to the search bar.
formatTempAndGarden(); | ||
}; | ||
|
||
const registerEventHandlers = () => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Excellent!
No description provided.