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

update #77

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

update #77

wants to merge 1 commit into from

Conversation

sanzcrpt
Copy link

No description provided.

Copy link

@tgoslee tgoslee left a comment

Choose a reason for hiding this comment

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

Good job San! I left some comments and shared some resources you can look into. Let me know if you have any questions after reviewing!

Comment on lines +44 to +52

<h3>Select City Name</h3>
<select id=".timeDiv">
<input type="text" id="city-input" value="City Name Here"/>
<h3>Reset City:</h3>
<button id="city-reset">Reset Me</button>
</select>

</section>
Copy link

Choose a reason for hiding this comment

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

I think you are missing the opening section tag for line 52 it should be on line 44.


<section class="divTwo">
<h3>Thermostat</h3>
<div class=".adhDiv">
Copy link

Choose a reason for hiding this comment

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

You did a good job using the section tag to organize your webpage. I want to share this resource on the flow for what tags to use. Hopefully, it will be helpful on when to use divs and when not to
html_flowchart
o

Comment on lines +3 to +6
const incTemp = document.querySelector("#increase-temp");
const decTemp = document.querySelector("#decrease-temp");
const tempDisplay = document.querySelector("#temp-display");
const landscape = document.querySelector("#landscape");
Copy link

Choose a reason for hiding this comment

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

putting your query selectors here I think is helpful!


let temp = 70;

const tempColors = () => {
Copy link

Choose a reason for hiding this comment

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

👍🏽

}
};

const tempLand = () => {
Copy link

Choose a reason for hiding this comment

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

👍🏽


};

const registerEventHandlers = () => {
Copy link

Choose a reason for hiding this comment

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

👍🏽


const resetButton = document.querySelector("#city-reset")
resetButton.addEventListener("click", reset);

Copy link

Choose a reason for hiding this comment

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

you are missing your functions to make the API calls to get the accurate weather. Think about where on your app you would want to add a button to get the current cities actual weather

Comment on lines +53 to +54
padding-top: 30px;
padding-bottom: 30px;
Copy link

Choose a reason for hiding this comment

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

you can write padding on one line like padding: 30px 0px 30px 0px
the order is padding: top left bottom right

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.

2 participants