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 - Stacey-AnBa #65

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

Amethyst - Stacey-AnBa #65

wants to merge 28 commits into from

Conversation

ngwin-ab
Copy link

No description provided.

Comment on lines +1 to +25
html {
background: url(../assets/images/taylor-van-riper-yQorCngxzwI-unsplash.jpg) no-repeat center center fixed;
-webkit-background-size: cover;
-moz-background-size: cover;
-o-background-size: cover;
background-size: cover;
}


body {
font-family: "Helvetica", Sans-Serif;
text-align: center;
margin: 0px;
padding: 0px;
}

/* ======================== HEADER ========================*/

header {
width: 100vw;
background: url("../assets/images/ameythst.jpeg") 75% 0 no-repeat;
background-size: cover;
}

h1 {

Choose a reason for hiding this comment

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

Amazing job with this CSS!

Comment on lines +98 to +118
/* ======================== CITY NAME DISPLAY ========================*/

#city-container {
margin: 0px;
padding: 5px;
display: flex;
justify-content: center;
align-items: center;
text-align: center;
}

#city-spotlight {
color: white;
margin-top: 1px;
margin-bottom: 1px;
font-size: 30px;
width: 200px;
}

/* ======================== TEMPERATURE DISPLAY ========================*/
#temperature-container {

Choose a reason for hiding this comment

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

Love the organization!

Comment on lines +38 to +58

<!-- Temperature -->
<div id="temperature-container">
<h3 id="temp-header">Current Temperature :</h3>

<!-- Temperature Buttons -->
<div class="temp-adustment-container">
<button id="down-button">Decrease &#8595;</button>

<span>
<h2 id="temp-value"></h2>
</span>

<button id="up-button">Increase &#8593;</button>
</div>
</div>

<!-- Sky conditions -->
<div id="sky-container">
<label for="weather-conditions" id="label">
<h4>Sky Condition :</h4>

Choose a reason for hiding this comment

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

Markup looks great!

src/index.js Outdated
Comment on lines 130 to 145
// axios
// .get('https://us1.locationiq.com/v1/search?key=YOUR_ACCESS_TOKEN&q=SEARCH_STRING&format=json')
// .then((response) => {

// })
// .catch((error) => {

// });


// additional weather data:
// let city = 'london'
// $.ajax({
// method: 'GET',
// url: 'https://api.api-ninjas.com/v1/weather?city=' + city,
// headers: { 'X-Api-Key': 'YOUR_API_KEY'},

Choose a reason for hiding this comment

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

Remember to remove debugging log statements & comments

Choose a reason for hiding this comment

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

Removed- two of them were additional features we were working on- the additional weather data and animation.

});
};

const getRtWeather = () => {

Choose a reason for hiding this comment

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

Prefer getWeather here

src/index.js Outdated

const handleUpBtnClicked = () => {
state.tempValue += 1;
console.log(state.tempValue);

Choose a reason for hiding this comment

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

Remember to remove debugging log statements

Choose a reason for hiding this comment

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

removed

Comment on lines +15 to +19
const changeCityHeader = () => {
const cityName = document.getElementById("city-spotlight");
const searchInput = document.getElementById("search-input").value;
cityName.innerHTML = `${searchInput}`;
};

Choose a reason for hiding this comment

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

Smart approach!

Copy link

@ameerrah9 ameerrah9 left a comment

Choose a reason for hiding this comment

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

Great work, An Ba & Stacey! 🥳

Thank you for your patience as we catch up on grading. Nice work! The HTML is structured well and the JS is clear and well-factored. However, there are some comments left in your code. This makes the project a Yellow 🟡

Comment on lines +21 to +33
const getLocation = () => {
let lat;
let lon;
return axios.get("http://127.0.0.1:5000/location", {
params: {
q: document.getElementById("search-input").value
}
})
.then((response) => {
state.lat = response.data[0].lat;
state.lon = response.data[0].lon;
getRtWeather();
})

Choose a reason for hiding this comment

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

Smart approach!

Comment on lines +26 to +53
<button id="search-button">Search</button>
<button id="reset-button">Reset</button>
</div>

<!-- City Header -->
<div id="city-container">

<span>
<h2 id="city-spotlight"></h2>
</span>
</div>


<!-- Temperature -->
<div id="temperature-container">
<h3 id="temp-header">Current Temperature :</h3>

<!-- Temperature Buttons -->
<div class="temp-adustment-container">
<button id="down-button">Decrease &#8595;</button>

<span>
<h2 id="temp-value"></h2>
</span>

<button id="up-button">Increase &#8593;</button>
</div>
</div>

Choose a reason for hiding this comment

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

Markup looks great!

Comment on lines +47 to +70

/* ======================== SEARCH ========================*/
#search-container {
margin: 15px;
display: flex;
overflow: hidden;
padding: 10px 5px;
background-color: #CBC3E3;
width: 70vw;
border: #5D3FD3;
border-style: solid;
justify-content: center;
}

#city-intro {
font-size: 20px;
margin-top: 2px;
margin-bottom: 2px;
margin-left: 40px;
margin-right: 40px;
color: #301934;
}

#search-input {

Choose a reason for hiding this comment

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

Amazing work with styling 👍🏾

Comment on lines +160 to +173

/* ======================== TEMPERATURE DISPLAY ========================*/
#sky-container {
display: inline;
justify-content: space-around;
margin: 0px;
overflow: hidden;
/* padding: 8px 10px; */
}

#label {
font-size: 20px;
margin: 0px;
color: #5D3FD3;

Choose a reason for hiding this comment

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

Great organization!

Comment on lines +5 to +13
const state = {
citySearchButton: null,
resetButton: null,
tempValue: 0,
lat: null,
lon: null,
upbutton: null,
downbutton: null
};

Choose a reason for hiding this comment

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

Nice work creating state 👍🏾

@staceyjo
Copy link

Removed comments and console logs.

@staceyjo
Copy link

should be green now

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