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.

Sign up for GitHub

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

panthers Maria Fernanda #25

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

panthers Maria Fernanda #25

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
59 changes: 59 additions & 0 deletions index.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,65 @@
<link rel="stylesheet" href="styles/index.css" />
</head>
<body>
<header class="weather-header">
<h1>Weather Report</h1>

</header>

<section class="temperature_section">
<h1>Temperature</h1>
<div class="temperature_content">
<button id ="increase_temperature">↑</button>
<button id="decrease_temperature">↓</button>
<button type="button" id="realtime-temp">Get Realtime Temperature</button>
</div>
<span id="temperatureDisplay" class="orange">72</span>
Copy link
Contributor

Choose a reason for hiding this comment

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

Rather than hard-coding a value for UI elements (which just happens to match the initial value backing the control), I like to leave the initial markup blank, and update the UI on first load from the initial backing values in the JS code. So like here, leave off the 72, then when the document loads, update the UI in the JS to show the initial temperature value. This would protect us from changing the default starting JS value, but having it be out of sync with what the UI is showing.

I would tend to do this for any of the configurable elements (temperature, ground, sky, city).

</section>

<section class="sky_section">
<h2>Sky selection</h2>

<select id="selectSky">
<option >Sunny</option>
<option >Cloudy</option>
<option >Rainy</option>
<option >Snowy</option>
</select>


</section>
Comment on lines +28 to +39
Copy link
Contributor

Choose a reason for hiding this comment

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

Be careful with indentation here


<div>
<h2> Weather Garden</h2>
</div>
<div>
<p id ="sky">☁️ ☁️ ☁️ ☀️ ☁️ ☁️

</p>
<p
id ="garden_section"> 🌸🌿🌼🌷🌻🌿☘️🌱🌻🌷
</p>



<section class="city-section">
<h1>City Name</h1>
<form action="/action_page.php" method="get" target="_blank">
<input type="text" id="city-name-input" name="city-name-input">
<input type="reset" value="Clear" id="reset-city-name">
</form>
<button id="reset-city-name-2">Reset</button>


<div >
<h2 id="city-display">✨Atlanta✨</h2>
Copy link
Contributor

Choose a reason for hiding this comment

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

Avoid using hard-coded values in our markup

</div>


</section>


</header>
<script src="./src/index.js"></script>
<script src="./node_modules/axios/dist/axios.min.js"></script>
</body>
Expand Down
148 changes: 148 additions & 0 deletions src/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,148 @@
const getTempFromCoordinates = () => {
const cityInput = document.getElementById('city-display').textContent;
console.log(cityInput);
Copy link
Contributor

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

axios
.get('http://127.0.0.1:5000/location', {
params: {
q: cityInput,
},
})
.then((response) => {
console.log('success', response);
const return_obj = {
lat: response.data[0].lat,
lon: response.data[0].lon,
};
axios
.get('http://127.0.0.1:5000/weather', {
params: {
lat: return_obj['lat'],
lon: return_obj['lon'],
},
})
.then((weatherResponse) => {
console.log(weatherResponse);
const cityTemp = weatherResponse.data['main']['temp'];
let tempDisplay = document.getElementById('temperatureDisplay');
console.log(cityTemp);
const tempFahrenheit =
(parseInt(cityTemp) - parseInt(273.15)) * 1.8 + 32;
console.log(tempFahrenheit);
tempDisplay.textContent = tempFahrenheit;

changeTempColor(tempDisplay);
const garden = gardenChange(tempDisplay);

const gardenContainer = document.getElementById('garden_section');
gardenContainer.textContent = garden;
});
})
Comment on lines +4 to +39
Copy link
Contributor

Choose a reason for hiding this comment

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

Consider breaking this function up a bit. It's taking on a lot responsibilty. Whenever we see a function that is more than 10 lines long we should consider seprating concerns. This is completely preferential but just a note. More on this here:

https://dmitripavlutin.com/the-art-of-writing-small-and-plain-functions/

https://medium.com/@janakachathuranga/coding-best-practices-javascript-write-small-functions-7d2567bd6328

Copy link
Contributor

Choose a reason for hiding this comment

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

There's an issue with wave 04 (calling the apis), the requirements specify that your code should get the weather of the currently displayed city name & when the   element is clicked updates and displays the realtime temperature of the currently displayed city name. Currently your code is not providing the updated temperature.

.catch((error) => {
console.log(error);
});
};

const increaseTemp = () => {
const currentTemp = document.getElementById('temperatureDisplay');
currentTemp.textContent = parseInt(currentTemp.textContent) + 1;
changeTempColor(currentTemp);
const gardenContainer = document.getElementById('garden_section');

const garden = gardenChange(currentTemp);
gardenContainer.textContent = garden;
};
const decreaseTemp = () => {
const currentTemp = document.getElementById('temperatureDisplay');
currentTemp.textContent = parseInt(currentTemp.textContent) - 1;
changeTempColor(currentTemp);
const gardenContainer = document.getElementById('garden_section');

const garden = gardenChange(currentTemp);
gardenContainer.textContent = garden;
};

const changeTempColor = (temperatureDisplay) => {
if (parseInt(temperatureDisplay.textContent) >= 80) {
temperatureDisplay.style.color = 'red';
} else if (parseInt(temperatureDisplay.textContent) >= 70) {
temperatureDisplay.style.color = 'orange';
} else if (parseInt(temperatureDisplay.textContent) >= 60) {
temperatureDisplay.style.color = 'yellow';
} else if (parseInt(temperatureDisplay.textContent) >= 50) {
temperatureDisplay.style.color = 'darkgreen';
} else if (parseInt(temperatureDisplay.textContent) <= 49) {
temperatureDisplay.style.color = 'mediumblue';
}
};

const gardenChange = (temperatureDisplay) => {
let garden = '';
if (parseInt(temperatureDisplay.textContent) >= 80) {
garden = '🌵__🐍_🦂_🌵🌵__🐍_🏜_🦂';
} else if (parseInt(temperatureDisplay.textContent) >= 70) {
garden = '🌸🌿🌼__🌷🌻🌿_☘️🌱_🌻🌷';
} else if (parseInt(temperatureDisplay.textContent) >= 60) {
garden = '🌾🌾_🍃_🪨__🛤_🌾🌾🌾_🍃';
} else if (parseInt(temperatureDisplay.textContent) >= 50) {
garden = '🌲🌲⛄️🌲⛄️🍂🌲🍁🌲🌲⛄️🍂🌲';
} else if (parseInt(temperatureDisplay.textContent) <= 49) {
garden = '🌲🌲⛄️🌲⛄️🍂🌲🍁🌲🌲⛄️🍂🌲';
}
return garden;
};

const updateSky = () => {
const inputSky = document.getElementById('selectSky').value;
const skyContainer = document.getElementById('sky');
let sky = '';

if (inputSky === 'Cloudy') {
sky = '☁️☁️ ☁️ ☁️☁️ ☁️ 🌤 ☁️ ☁️☁️';
} else if (inputSky === 'Sunny') {
sky = '☁️ ☁️ ☁️ ☀️ ☁️ ☁️';
} else if (inputSky === 'Rainy') {
sky = '🌧🌈⛈🌧🌧💧⛈🌧🌦🌧💧🌧🌧';
} else if (inputSky === 'Snowy') {
sky = '🌨❄️🌨🌨❄️❄️🌨❄️🌨❄️❄️🌨🌨';
}
skyContainer.textContent = sky;
};

const displayCityName = () => {
const inputCity = document.getElementById('city-name-input').value;
const displayCityContainer = document.getElementById('city-display');

displayCityContainer.textContent = '✨ ' + inputCity + ' ✨';
Copy link
Contributor

Choose a reason for hiding this comment

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

You could also use concatentation here like so:

  displayCityContainer.textContent = `✨ ${inputCity} ✨`;

};

const resetCityName = () => {
const inputCity = document.getElementById('city-name-input');
const cityNameDisplay = document.getElementById('city-display');
inputCity.value = 'Atlanta';
cityNameDisplay.textContent = '✨Atlanta✨';
console.log(inputCity);
console.log(inputCity.value);
};

const registerEventHandlers = () => {
const increaseTempButton = document.getElementById('increase_temperature');
increaseTempButton.addEventListener('click', increaseTemp);

const decreaseTempButton = document.getElementById('decrease_temperature');
decreaseTempButton.addEventListener('click', decreaseTemp);

const cityInputForm = document.getElementById('city-name-input');
cityInputForm.addEventListener('input', displayCityName);

updateSky();
const skySelect = document.getElementById('selectSky');
skySelect.addEventListener('change', updateSky);

const resetButton = document.getElementById('reset-city-name-2');
resetButton.addEventListener('click', resetCityName);

const realtimeTempButton = document.getElementById('realtime-temp');
realtimeTempButton.addEventListener('click', getTempFromCoordinates);
};
Comment on lines +127 to +146
Copy link
Contributor

Choose a reason for hiding this comment

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

👍🏾


document.addEventListener('DOMContentLoaded', registerEventHandlers);
25 changes: 25 additions & 0 deletions styles/index.css
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
.weather-header {
display: flex;
flex-direction: column;
grid-column: auto / span 2;
color: white;

}
body {
background-color: rgb(136, 195, 241);
}

.orange {
color: rgb(255, 145, 49);
}
#temperatureDisplay{
display: flex;
font-size: xx-large;
Comment on lines +1 to +17
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice work with the styling, though I think you could have pushed yourself to add some CSS grid styling

flex-direction: column;
grid-column: 4;
};

.headerCityName::before,
.headerCityName::after{
content: "✨";
};