-
Notifications
You must be signed in to change notification settings - Fork 100
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
Panthers Nicole Mejia #84
base: main
Are you sure you want to change the base?
Conversation
<div id="temp-increase-button" class="increase-button buttons"></div> | ||
<div id="temp-decrease-button"class="decrease-button buttons"></div> | ||
</div> | ||
</div> | ||
</div> | ||
<div id="dream-sky" class="dream-panels"> | ||
<h3>Dream Sky</h3> | ||
<div class="dream-panel-content"> | ||
<select name="skies" id="sky-select"> | ||
<option value=""> | ||
Pick One | ||
</option> | ||
<option id="sunshine-sky" value="sunshine"> | ||
Sunshine | ||
</option> | ||
<option id="cloudy-sky" value="clouds"> | ||
Cloudy | ||
</option> | ||
<option id="rain-sky" value="rain"> | ||
Rain | ||
</option> | ||
<option id="snow-sky" value="snow"> | ||
Snow | ||
</option> | ||
</select> | ||
</div> | ||
</div> | ||
<div id="dream-humidity" class="dream-panels"> | ||
<h3>Dream Humidity</h3> | ||
<div class="dream-panel-content"> |
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.
Markup looks awesome! Great work 🏁
const state = { | ||
temp: 0, // starting temp | ||
humidity: 0, // starting humidity | ||
city: "Chattanooga", // default city | ||
clicked: false, // whether the temp increase button is in clicked state | ||
}; |
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.
Nice work with this initial set up!
if (document.getElementById("search-bar").value) { | ||
cityInput = document.getElementById("search-bar").value; | ||
}; |
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.
Nice approach using this guard clause 👍🏾
const getFiveDayForecast = () => { | ||
let cityInput = state.city; | ||
|
||
if (document.getElementById("search-bar").value) { | ||
cityInput = document.getElementById("search-bar").value; | ||
}; | ||
|
||
return axios.get('http://127.0.0.1:5000/location', {params: {q: cityInput} | ||
}) | ||
.then(location => { | ||
return { | ||
lat: location.data[0].lat, | ||
lon: location.data[0].lon | ||
} |
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.
Looks good! Nice work getting this second call to work and extending your project 🥳
// RESET CITY TO DEFAULT | ||
const resetToDefault = () => { | ||
window.location.reload(); | ||
}; |
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.
Smart approach!
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.
Great work, Nicole! 🎉
Thank you for your patience as we catch up on grading. Your JavaScript looks really clean, your commit message are excellent, and your closing semi-colons are consistent! This project is a Green. 🟢
Keep it up! 💯
enhancements:
i added another endpoint to the proxy server to request the five day forecast data. to refactor, instead of calling '/weather' for the default city's current weather, then calling '/forecast' for the five day forecast, i would only call '/forecast' and use the first date's data to update the default city's current weather. i kept it for the project requirement's sake.