Skip to content
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

Amethyst - Gloria and Raina #66

Open
wants to merge 63 commits into
base: main
Choose a base branch
from
Open

Conversation

laughtivity
Copy link

No description provided.

Rainacam12 and others added 30 commits June 8, 2023 11:21
…cked it will increment the increment the tempature
city: "Seattle"
};

document.addEventListener("DOMContentLoaded", function () {
Copy link

@mikellewade mikellewade Jul 17, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Putting all your functionality in the event listener for when the DOM is loaded is functionally okay, however you will most likely find the things in there that should be assigned to variables or calculated on initial loads. You will also usually have your event handlers being added in this function as well. For example,

document.addEventListener("DOMContentLoaded", function () {
    const increaseTemp = document.querySelector("#increase-temp");
    const decreaseTemp = document.querySelector("#decrease-temp");
    const displayTemp = document.querySelector("#display-temp");
    const resetButton = document.querySelector("#reset-button"); 
    const searchButton = document.querySelector("#search-button");
    const cityName = document.getElementById("city-name");
    const cityInput = document.getElementById("city-input");
    const selectSky = document.querySelector("#sky-dropdown");
    const result = document.querySelector("#sky");
    const landscape = document.querySelector("#landscape");
    
    increaseTemp.addEventListener("click", increaseTemperature);
    decreaseTemp.addEventListener("click", decreaseTemperature);
    ...
}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You will also usually find this at the bottom of your file as well, that way you can put all your function definition on top of it.

@@ -0,0 +1,160 @@
const state = {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Love how you all chose to use this psuedostate! I think that it is great practice for what you will see in React though it will be utilized differently!

Comment on lines +22 to +23
let temperature = 65;
let lands = "🌾🌾 🍃 🪨 🛤 🌾🌾🌾 🍃";

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These could be things that you could be put in your state variable, you want to use your state to hold information specific to a webpage at a specific time.

Comment on lines +106 to +108
const findLatitudeAndLongitude = async () => {
// let latitude, longitude;
await axios.get('http://127.0.0.1:5000/location',

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see that you used the async/await keywords in a few of your functions. I love that you are making use of them! Here's a tip that will allow you to bring your use more in line with the keywords intention. async/await is intended to be be used in-place of promises. So when using them your code should a little more like this:

const findLatitudeAndLongitude = async () => {
        const resp = await axios.get('http://127.0.0.1:5000/location',{
            params: {
                q:cityInput.value,
                format: 'json'
            }})
        
        try {
            state.lat= response.data[0].lat;
            state.lon= response.data[0].lon;
          
            return {
            cityLat: state.lat,
            cityLon: state.lon
          }
          
        } catch {
          console.log('error in findLatitudeAndLongitude!')
        }
 }

updateTemp(temperature)

});

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome job everyone! You really ate this one up! Please feel free to reach out to me if you have any questions about the comments I left or if you want to discuss anything in greater detail! ⭐️

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants