Lions - Ariel & Kate - Weather Report #21
Conversation
| ##Pair Plan of Action | ||
| 1. Access needs | ||
| 1. Open to working together until 6 PM after class | ||
| 2. Prefer in-person | ||
| 3. Communication via text and Slack | ||
| 2. Learning styles | ||
| 1. Prepare and process individually before meeting and planning | ||
| 2. Refer to examples | ||
| 3. How you prefer to receive feedback | ||
| 1. Nicely and politely | ||
| 2. Immediately if there is an issue | ||
| 3. Open to constructive criticism | ||
| 4. Communication skills to improve | ||
| 1. Taking turns driving and navigating | ||
| 2. Communicate code more efficiently |
| <header> | ||
| <h1>WEATHER REPORT</h1> | ||
| </header> | ||
| <!-- <section> --> |
There was a problem hiding this comment.
Don't forget to clean up comments once they're no longer needed.
| <div class="left-column box" id="sky-input"> | ||
| <h2 id="sky-text">SKY</h2> | ||
| <!-- dropdown source: https://www.w3schools.com/howto/howto_js_cascading_dropdown.asp --> | ||
| <form id="dropdown"> <!-- Not sure what this is supposed to look like --> | ||
| <select name="sky" id="sky-select"> | ||
| <option value="sunny">Sunny</option> | ||
| <option value="cloudy">Cloudy</option> | ||
| <option value="rainy">Rainy</option> | ||
| <option value="snowy">Snowy</option> | ||
| </select> | ||
| <br><br> | ||
| </form> |
There was a problem hiding this comment.
This works well! A few thoughts:
- Have
SKYbe in alabelinstead of anh2so it gets associated with theselect. - I appreciate you linking to where you got code from!
- Try to avoid using
br. Use CSS to adjust spacing where needed instead.
| <!-- blank field source: https://www.w3schools.com/tags/tryit.asp?filename=tryhtml_form_target--> | ||
| <form action="/action_page.php" method="get" target="_blank"> | ||
| <input type="text" id="city-name-input" name="city-name-input"> | ||
| </form> |
There was a problem hiding this comment.
I there are better ways to do this using JavaScript. Right now your form attempts to use action_page.php - something that doesn't exist on your site. It's OK to look up how to do things online, but tey to always make sure you understand what each part is doing.
| const getTempFromCoordinates = () => { | ||
| const cityInput = document.getElementById('city-display').textContent; | ||
| console.log(cityInput); | ||
| axios | ||
| .get('http://127.0.0.1:5000/location', { | ||
| params: { | ||
| q: cityInput, | ||
| }, | ||
| }) | ||
| .then((response) => { | ||
| console.log('success', response); | ||
| const return_obj = { | ||
| lat: response.data[0].lat, | ||
| lon: response.data[0].lon, | ||
| }; | ||
| axios | ||
| .get('http://127.0.0.1:5000/weather', { | ||
| params: { | ||
| lat: return_obj['lat'], | ||
| lon: return_obj['lon'], | ||
| }, | ||
| }) | ||
| .then((weatherResponse) => { | ||
| console.log(weatherResponse); | ||
| const cityTemp = weatherResponse.data['main']['temp']; | ||
| let tempDisplay = document.getElementById('temp'); | ||
| console.log(cityTemp); | ||
| const tempFahrenheit = | ||
| (parseInt(cityTemp) - parseInt(273.15)) * 1.8 + 32; | ||
| console.log(tempFahrenheit); | ||
| tempDisplay.textContent = tempFahrenheit; | ||
| colorTempChange(tempDisplay); | ||
| const landscape = landscapeChange(tempDisplay); | ||
|
|
||
| const landscapeContainer = document.getElementById('landscape'); | ||
| landscapeContainer.textContent = landscape; | ||
| }); | ||
| }) | ||
| .catch((error) => { | ||
| console.log('error!'); | ||
| console.log(error); | ||
| }); | ||
| }; |
There was a problem hiding this comment.
There's a lot that goes on in this function! Can you consider how this could be broken down into helpers?
| const increaseTemp = () => { | ||
| const currentTemp = document.getElementById('temp'); | ||
| currentTemp.textContent = parseInt(currentTemp.textContent) + 1; | ||
| colorTempChange(currentTemp); | ||
|
|
||
| const landscapeContainer = document.getElementById('landscape'); | ||
| const landscape = landscapeChange(currentTemp); | ||
| landscapeContainer.textContent = landscape; | ||
| }; | ||
|
|
||
| const decreaseTemp = () => { | ||
| const currentTemp = document.getElementById('temp'); | ||
| currentTemp.textContent = parseInt(currentTemp.textContent) - 1; | ||
| colorTempChange(currentTemp); | ||
|
|
||
| const landscapeContainer = document.getElementById('landscape'); | ||
| const landscape = landscapeChange(currentTemp); | ||
| landscapeContainer.textContent = landscape; | ||
| }; |
There was a problem hiding this comment.
These work well! As an alternative, can you think of how they could be combined into one function to make your code more DRY?
| const colorTempChange = (temp) => { | ||
| if (parseInt(temp.textContent) >= 80) { | ||
| temp.style.color = '#FF0000'; | ||
| } else if (parseInt(temp.textContent) >= 70) { | ||
| temp.style.color = '#FFA500'; | ||
| } else if (parseInt(temp.textContent) >= 60) { | ||
| temp.style.color = '#D1D100'; | ||
| } else if (parseInt(temp.textContent) >= 50) { | ||
| temp.style.color = '#00FF00'; | ||
| } else if (parseInt(temp.textContent) <= 49) { | ||
| temp.style.color = '#00AEAE'; | ||
| } | ||
| }; |
There was a problem hiding this comment.
The last else if can just be an else. Also, consider using classes instead of setting the color directly. This allows us to have all the styling information the the CSS file, and allows us easily have other type of styling be impacted by the temperature (for example if we decided we wanted the font to change depending on the temperature as well).
| const skyChangeOnSelect = () => { | ||
| const selectedSky = document.getElementById('sky-select').value; |
There was a problem hiding this comment.
Consider getting this value from the event instead of hardcoding the id.
| // cityNameDisplay.addEventListener('change', resetCityName); | ||
| }; | ||
|
|
||
| document.addEventListener('DOMContentLoaded', registerEventHandlers); |
There was a problem hiding this comment.
Nice job registering! Consider using the more robust form or registering that was at the bottom of the crab fan site example.
| body { | ||
| display: grid; | ||
| width: 100vw; | ||
| height: 100vh; | ||
| grid-template-columns: 12.5% 12.5% 12.5% 12.5% 12.5% 12.5% 12.5% 12.5%; | ||
| grid-template-rows: 8.3% 8.3% 8.3% 8.3% 8.3% 8.3% 8.3% 8.3% 8.3% 8.3% 8.3% 8.3%; | ||
| background-color: rgb(165, 212, 243); | ||
| } |
There was a problem hiding this comment.
Nice grid! Consider using repeat to simplify the column and row definitions.
No description provided.