-
Notifications
You must be signed in to change notification settings - Fork 88
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
Weather Report - HW - Sea Turtles #76
base: main
Are you sure you want to change the base?
Conversation
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 job Hope! Everything is working 💃🏽 ! I left some comments on resources you can use to clean up what we discussed in our 1:1! Let me know if you have questions!
<body> | ||
|
||
<div>Weather Report</div> | ||
<main> | ||
<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.
'https://images.unsplash.com/photo-1584267385494-9fdd9a71ad75?ixlib=rb-1.2.1&ixid=MnwxMjA3fDB8MHxwaG90by1wYWdlfHx8fGVufDB8fHx8&auto=format&fit=crop&w=2670&q=80', | ||
}; | ||
|
||
const domElements = { |
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.
adding your domElements here is a good way to organize your file
domElements.currentTemp.textContent = state.temperature; | ||
}, | ||
tempBackgroundChange: (backgroundColor, landscape) => { | ||
console.log(backgroundColor); |
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.
remove your console log here
return Math.floor(1.8 * (kelvin - 273) + 32) | ||
|
||
} | ||
const setBackgroundColorLandscape = (temperature) => { |
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.
👍🏽
} | ||
}; | ||
|
||
const setSkyImage = (skyCondition) => { |
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.
👍🏽
}; | ||
|
||
// const axios = require('axios'); | ||
const getLatitudeLongitude = 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.
I would move your api functions before registering your event handlers just to put them all together with the other functions
@@ -0,0 +1,65 @@ | |||
body { |
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 know we talked about using the grid to overlap elements. Here is a resource to do that https://www.joomlashack.com/blog/tutorials/layering-items-within-css-grid/
Had to close previous pull request. Wrong branch and some issues tracking some missing code. Still trouble-shooting a bit but everything seems to be here for the most part.