Feedback + help on my Local Weather App

Code Feedback
1

Hi, I just completed the Local Weather challenge. It took me a while, but I learned at ton along the way.

I’d love some help to finish polishing the project:

  1. The wind speed button only changes from mph to km/h once, and stays stuck there. I’ve used the same code than for the temperature button, which works fine to switch from C to F back and forth, without success.
  2. I can’t figure our how to use the CSS background-size: cover; property to have the background image take the full screen and not repeat. I haven’t been able to figure out how to style the background when the image is referenced in JS.
  3. I tried to keep the JS code clean, but I’m not yet familiar with the all the best practices, so any feedback on how the code is laid out or written would be greatly appreciated.

Thanks for checking it out!

2

Background repeats. Also, why use nonfunctional buttons to display your four weather conditions?

3

Thanks! The background issue is the point I was trying to make on #2. I edited it to clarify. I used those buttons because two of them can be click to convert the units, so it’s homogeneous, but I agree that the info could be displayed better without those buttons.

4

Hi, the wind speed button issue is due to a case typo - on line 45 of your JS you have written

$("#windspeedMph").html(windSpeedMph + " mph");

instead of

$("#windSpeedMph").html(windSpeedMph + " mph");

(uppercase S)

Hope that helps.

5

Also, regarding the background image size/repeat, since you are only setting the background-image property with your JS, you can declare the following in your CSS and these won’t be overwritten.

body {
  background-size: cover;
  background-repeat: no-repeat;
}

I think that’s what you were trying to achieve?

Re: point 3, what you have looks OK to me. I’d suggest maybe looking into how you can split your code down in to separate functions to maybe make it easier to read? You could have a function getImageUrl(weatherId) which returns the URL string, for example, and perhaps you could use a javascript switch statement to avoid all those if {} else {} statements. But I don’t see anything wrong with what you have done, it’s just an alternative suggestion.

6

Ah, yes, I was pretty sure I have case typo somewhere. Thanks, I got to be more careful there.

7

Thanks for the help with the background. It wasn’t working earlier because I had it declared as “background-image” instead of “body”. That makes sense. And thanks for the pointers with the code. Now I can move on to the Wikipedia viewer challenge!