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

Poppy-shark #64

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

Poppy-shark #64

wants to merge 5 commits into from

Conversation

PoppyMeng
Copy link

No description provided.

Copy link

@audreyandoy audreyandoy 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 Poppy! Your project meets all the requirements and I love the gif backgrounds.

My two pieces of design feedback include:

  1. making the either the background stretch or center the section with top and bottom margin so that it's not being cut off
  2. adding responsiveness so your app adjusts to different screen sizes.

Otherwise, fantastic work Poppy!

Grade: 🟢

Comment on lines +4 to +36
// let map;
// let service;
// function callback(place, status) {
// if (status == google.maps.places.PlacesServiceStatus.OK) {
// createMarker(place);
// }
// }
// let place= //input city
// let config = {
// method: 'get',
// url: 'https://maps.googleapis.com/maps/api/place/textsearch/json?query=place&key=YOUR_API_KEY'

// };

// axios(config)

// .then((response)=>{
// const place = response.data[results][0];
// request={
// placeId: place.place_id
// }
// service = new google.maps.places.PlacesService(map);
// service.getDetails(request, callback);
// });
// .then((response)=>{
// PlacePhoto=response.photos[0];
// URL=PlacePhoto.getUrl();
// parameters={
// maxwidth=400,
// key=YOUR_API_KEY
// }
// return axios.get(URL, parameters);
// });

Choose a reason for hiding this comment

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

Commented out code should be removed from PRs.

Comment on lines +37 to +42
const state = {
degree:60,
isF: true,
cityName: 'Poppy City, USA',

};

Choose a reason for hiding this comment

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

Nice working setting up state as an object. Many frontend frameworks store 'global' data like this in an object as well.

Comment on lines +50 to +63
const changeToC=()=>{
const degreeCountContainer = document.getElementById("degree");
if (state.isF){
state.isF=false;
degreeCountContainer.textContent = Math.floor((state.degree -32)/1.8);
}

}
const changeToF=()=>{
const degreeCountContainer = document.getElementById("degree");
if (! state.isF){
state.isF=true;
}
degreeCountContainer.textContent=state.degree;

Choose a reason for hiding this comment

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

👍 Nice work doing the temperature conversion.

Comment on lines +70 to +92
// const plusTemp = (e) => {
// const degreeCountContainer = document.getElementById("degree");
// state.degree += 1;
// degreeCountContainer.textContent=state.degree;

// const temperature = document.querySelector("#degree");
// if (temperature > 80) {
// temperature.style.color = 'red';

// }
// state.clickCount=0;
// };
// const minusTemp = () => {
// const degreeCountContainer = document.getElementById("degree");
// state.degree -= 1;
// degreeCountContainer.textContent = state.degree;

// const temperature = document.querySelector("#degree");
// if (temperature < 40) {
// temperature.style.color = 'blue';
// }
// state.clickCount=0;
// };

Choose a reason for hiding this comment

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

Remove commented out code.

Comment on lines +67 to +68
e.target.id ==='plus'? state.degree++: state.degree--;
degreeCountContainer.textContent=state.degree;

Choose a reason for hiding this comment

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

Always good practice to add spaces around operators.

Suggested change
e.target.id ==='plus'? state.degree++: state.degree--;
degreeCountContainer.textContent=state.degree;
e.target.id === 'plus'? state.degree++ : state.degree--;
degreeCountContainer.textContent = state.degree;

Comment on lines +102 to +135
const getTemp=() => {
//console.log(loc)
const loc= document.getElementById('name');

return axios
.get('http://127.0.0.1:5000/location', {
params:{
q: loc.value,
},
})
.then((response) => {
console.log(response);
const lat=response.data[0].lat;
const lon=response.data[0].lon;
return axios.get('http://127.0.0.1:5000/weather', {
params:{
lat: lat,
lon: lon,
},
})
})

.then((response) => {

const temp=response.data.current.temp;
state.degree=Math.floor((temp-273.15)*1.8+32)
document.getElementById("degree").textContent=state.degree;
return state.degree;
})
.catch((response)=>{
console.log(response);
console.log('ERROR');
})
}

Choose a reason for hiding this comment

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

👍

Comment on lines +137 to +144
// const showRealWeather = ()=> {
// const degreeCountContainer = document.getElementById("degree");
// let theText = document.getElementById("name").value;

// // let temp=getTemp(theText);
// // state.isF=true;
// // degreeCountContainer.textContent=temp;
// }

Choose a reason for hiding this comment

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

Remove commented-out code.

Comment on lines +151 to +165
const changeBackground=() =>{
const sky = document.getElementById('select-sky');
if (sky.value == 'sunny'){
document.getElementById("body").style.backgroundImage= "url('/assets/sunny.gif')";
}else if (sky.value == 'rainy'){

document.getElementById("body").style.backgroundImage ="url('/assets/rainy.gif')";
}else if (sky.value == 'snowy'){
document.getElementById("body").style.backgroundImage='url("/assets/snow.gif")';
}else if (sky.value == 'cloudy'){
document.getElementById("body").style.backgroundImage="url('/assets/cloudy.webp')";
}
console.log('changeBackgroundCalled')

}

Choose a reason for hiding this comment

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

There's a lot of repetitive syntax in the conditionals. We can dry up our code by changing the value of a variable in the conditionals and THEN setting that variable to the body's background image.

const changeBackground = () => { 
    const sky = document.getElementById('select-sky');
    let image = '';
    if (sky.value == 'sunny'){
        image = "url('./assets/sunny.gif')";
    } else if (sky.value == 'rainy'){
        image = "url('./assets/rainy.gif')";
    } else if (sky.value == 'snowy'){
        image = "url('./assets/snow.gif')";
    } else if (sky.value == 'cloudy'){
        image = "url('./assets/cloudy.webp')";
    }
    document.getElementById("body").style.backgroundImage = image;    
}

Choose a reason for hiding this comment

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

Love the gifs as a background!

.sky{
font-size: 1.5em;
}
#div1 {

Choose a reason for hiding this comment

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

Use more descriptive names for ids.

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