Gwen and Whitney - Zoisite #56
Conversation
| const onLoaded = () => { | ||
| loadControls(); | ||
| setDefaultValues(); | ||
| registerEventHandlers(); | ||
| }; | ||
|
|
||
| onLoaded(); |
There was a problem hiding this comment.
We had to use the onLoaded approach when we were using codesandbox because by the time the UI had rendered and shown us the code editor for to write code the event called DOMContentLoaded had already fired. The act of rendering the web page makes DOMContentLoaded happen so we couldn't use it as an event to register loadControls().
When you're writing vanilla JS for event handling like, we should call register the function called registerEventHandlers on the event DOMContentLoaded, like this:
window.addEventListener("DOMContentLoaded", registerEventHandlers)Then when your page loads registerEventHandlers will execute.
You could add loadControls() and setDefaultValues() inside registerEventHandlers()` so alll your 'start up' logic happens on page load.
| sky : null, | ||
| }; | ||
|
|
||
| const setDefaultValues = () => { |
| sky.textContent = ""; | ||
| }; | ||
|
|
||
| const registerEventHandlers = () => { |
| state.skySelect.addEventListener("change", changeSky); | ||
| }; | ||
|
|
||
| const loadControls = () => { |
| const increaseTemp = () => { | ||
| state.tempDeg += 1; | ||
| tempValue.textContent = `${state.tempDeg}` | ||
| changeColor(); | ||
| changeLandscape(); | ||
| }; | ||
|
|
||
| const decreaseTemp = () => { | ||
| state.tempDeg -= 1; | ||
| tempValue.textContent = `${state.tempDeg}`; | ||
| changeColor(); | ||
| changeLandscape(); | ||
| }; |
There was a problem hiding this comment.
Notice that these two methods do essentially the same thing but the difference is -= 1 or += 1 (also in JS you can use ++ or -- which is short hand for doing the same thing).
How could you combine the logic of these two methods into one event handler called handleTempChange()?
What if you pass in the event object as an argument and you check if the event's target has an id increaseTempControl or decreaseTempControl and depending on the id then you could ++ or --
const handleTempChange = (event) => {
if (event.target.id === 'increaseTempControl') {
state.tempDeg++;
} else {
state.tempDeg--;
}
tempValue.textContent = `${state.tempDeg}`;
changeColor();
changeLandscape();
};| } | ||
|
|
||
| const findLatAndLon= (query) => { | ||
| let latitude, longitude; |
There was a problem hiding this comment.
What if you initialized these variables in your state object? Then on lines 118 and 119, you could do:
state.latitude = response.data[0].lat;
state.longitude = response.data[0].lon;
|
|
||
| const findLatAndLon= (query) => { | ||
| let latitude, longitude; | ||
| axios.get('http://localhost:5000/location', |
There was a problem hiding this comment.
Prefer to reference string literals like this url with a descriptively named variable
| axios.get('http://localhost:5000/location', | ||
| { | ||
| params: { | ||
| q: `${state.cityNameOutput.textContent}`, |
| { | ||
| params: { | ||
| format: 'json', | ||
| lat: latitude, | ||
| lon: longitude | ||
| } | ||
| }) |
There was a problem hiding this comment.
Prefer the object being sent along in the get request to be referenced with a variable to make this line of code more readable.
WEATHER_URL = "http://localhost:5000/weather";
request_data = {
format: 'json',
lat: latitude,
lon: longitude
}
axios.get(WEATHER_URL, request_data)| console.log('success in findWeather!', response.data.main.temp); | ||
| state.tempDeg = Math.round((response.data.main.temp - 273.15) * 9/5 + 32); | ||
| tempValue.textContent = `${state.tempDeg}` | ||
| changeColor(); | ||
| changeLandscape(); | ||
| }) | ||
| .catch( (error) => { | ||
| console.log('error in findWeather!'); | ||
| }); |
There was a problem hiding this comment.
Nitpick - indentation needs to be fixed up.
While the JS still executes correctly since the language is not dependent on indentation, we should strive for correct indentation for sake of consistency and readability. When you open a PR for review when you're at work be sure to fix up your indentation/spacing and remove debugging print statements.
No description provided.