Finally finished my Weather App. Feedback is appreciated!

Hi, guys!

Going forward through this interesting journey step by step, project by project. And happily, now one less project separates me from the ‘Front End Certificate’.

I’ve spent about 8 hours on this one and here is result:

What do you think about it?

I do like the overall effect of what you did with the project.

You have two 404 errors and one mixed content warning in the console.

index.html:117 GET

index.html:11 GET

index.html:117 GET 404 ()

index.html:1 Mixed Content: The page at ‘’ was loaded over HTTPS, but requested an insecure image ‘’. This content should also be served over HTTPS.

Also, I noticed one small issue when the screen is not a mobile width, I see the following when viewing Fahrenheit.


1 Like

Also, I noticed a way you could simply your setIcon function

You could replace:

  let isNight = sunrise > time || sunset < time ? true : false;

with the following:

  let period = sunrise > time || sunset < time ? 'night' : 'day';
  setIcon(weather, period);

This would allow you to get rid of that giant case statement in your setIcon function and replace with the following code. See my comments below:

function setIcon(weather, period) {
  let iconUrl = '';
  // the following object has property names of each weather condition and values of nested objects
  // containing an icon name for 'day' vs 'night'   
  const weatherIcons = {
    Clear: {day: 'sun', night: 'moon'},
    Rain: {day: 'rain', night: 'rain'},
    Drizzle: {day: 'drizzle', night: 'drizzle'},
    Clouds: {day: 'cloudy', night: 'cloudy'},
    "Partly Cloudy": {day: 'cloudy-day', night: 'cloudy-night'},
    Snow: {day: 'snowing', night: 'snowing'},
    Thunderstorm: {day: 'storm', night: 'storm'},
    Atmosphere: {day: 'fog', night: 'fog'} 
  // should Atmosphere above be Fog?  Earlier code showed a conversion to Fog
  // but I left your case statement logic in place using the weatherIcons object

  $('#icon').fadeOut(10, function () {
    $(this).attr('src', iconUrl + weatherIcons[weather][period] +'.png').fadeIn(100);

Oh, that’s because I edited it in the editor on my laptop and then published a final result to CodePen, but somehow forgot to remove includings of script.js and styles.css files. That’s what about 404 errors.

And what about Mixed Content, I changed http to https in the image url and now this error doesn’t exist too.

Thank you for paying attention to these errors. I completely forgot to check them.

Talking about the issue when viewing Fahrenheit, I could play with font-size and some padding’s, but decided to just remove the space between the number and the degree symbol.

And I have a guideline question related to Fahrenheit. I know that Celsius degrees are always meant to be displayed as integers. But I don’t know about Fahrenheit, is it right to display them as floats?

That is funny, because I always thought Celsius was supposed to be displayed as a float. In the United States,I have only seen Fahrenheit displayed as an integer.

Wow, that sounds really funny, because in Ukraine situation is quite opposite :slight_smile:

I don’t think there’s any hard convention in that. I think Celsius might be more commonly taken out to decimals because it has a smaller scale.

But in general for weather, I don’t recall seeing decimal points - most people don’t need that much precision. Is it 46.4 degrees out or 46.2? I need to know what to wear!

In the US, the main place we see Celsius (and the metric system in general is for science and technical things. In those fields we tend to need more precision so we use more significant digits. I think that’s why Americans sometimes associate Celsius with decimals.