Skip to content

Maz + Kaliane Van#71

Open
mlweir98 wants to merge 43 commits into
Ada-C19:mainfrom
MotifNoticer:main
Open

Maz + Kaliane Van#71
mlweir98 wants to merge 43 commits into
Ada-C19:mainfrom
MotifNoticer:main

Conversation

@mlweir98

Copy link
Copy Markdown

No description provided.

mlweir98 and others added 30 commits June 8, 2023 13:13
Comment thread src/index.js
@@ -0,0 +1,140 @@
// `"use strict";`

const state = {

Copy link
Copy Markdown

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 thread src/index.js
Comment on lines +22 to +38
const loadControls = () => {
state.increaseTempButton = document.getElementById('increaseTempButton');
state.decreaseTempButton = document.getElementById('decreaseTempButton');
state.tempNumber = parseInt(document.getElementById('tempNumberContainer').innerText);
state.tempNumberContainer = document.getElementById('tempNumberContainer');
state.tempNumberClass = document.getElementById('tempNumberContainer').className;
state.skyEmojiContainer = document.getElementById('skyEmojiContainer');
state.skyEmoji = document.getElementById('skyEmojiContainer').innerText;
state.landEmojiContainer = document.getElementById('landEmojiContainer');
state.landEmoji = document.getElementById('landEmojiContainer').innerText;
state.cityNameContainer = document.getElementById('cityNameContainer');
state.cityName = document.getElementById('cityNameContainer').innerText;
state.cityInput = document.getElementById('cityInput');
state.realTempButton = document.getElementById('realTempButton');
state.skyDropdown = document.getElementById('skyDropdown');
state.weatherCity = document.getElementById('weatherCity');
};

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Oh wow! I love this! This shows me you centering readability! Naming conventions like this make your code a lot more intuitive!

Comment thread src/index.js
Comment on lines +83 to +92
const getRealTemp = async (locationName) => {
const locationResp = await axios.get('http://localhost:5000/location',{ params: {q: locationName } })

const coords = [locationResp.data[0]['lat'], locationResp.data[0]['lon']]

const weatherResponse = await getWeather(coords[0],coords[1])

return weatherResponse

}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Okay async/await! You used this perfectly!

Comment thread src/index.js
state.tempNumber = response.data['main']['temp']
};

const registerEventHandlers = () => {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⭐️⭐️⭐️

Comment thread src/index.js
registerEventHandlers();
};

onLoaded(); No newline at end of file

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I would suggest putting your onLoaded method inside an event handler for when the document is loaded, that way you can have an extra layer of protection when it comes to things loading in order. Basically, it would help ensure the loadControls and. registerEventHandlers functions would run after the DOM elements are loaded. It would look something like this:

document.addEventListener("DOMContentLoaded", function () {
    loadControls()
    registerEventHandlers()
}

Copy link
Copy Markdown

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