Completed the Weather App takehome project :)

Well, I wanted to put this where someone might enjoy it… sharing this with family and friends I just get “whats the big deal, I have a weather app on my phone”, haha.

I’m not completely happy with the visual layout, but figure my time might be better spent starting the next course or project rather than spending a few more days tweeking margin widths, font styles, and colors. If there is any blaring “oh, you should never do that” or “oh, its so much better to do this” items in the code, please let me know.

-Browser geolocation to get initial location
-Clickable F/C button for instant conversion, selection saved for next session
-Wind indicator graphic
-Search bar for getting weather from other locations (can search by city, state)
-Tab bar to store up to 5 recent searches
-Tabs are saved in local storage and restored on next visit
-Tabs can be deleted by clicking ‘x’
-Rainviewer animated weather map (not the best choice, but easy to implement)

This looks great - well done!

One little glitch - if I do a few successive searches for different locations, I’m sometimes seeing NaN mph for the wind gust value.
It looks as if the wind gust data is occasionally missing from the weather data.

Maybe you could handle that by displaying some sort of “not available” message?

1 Like

KittyMac, thanks… I actually noticed that, and almost put an inline if, but figured the lay person would get the meaning… but since I was called out on it, haha:)

Now a non-existant gust should show: " – mph ". Definitely an improvement, thanks again.

Oops - sorry about that !

Seriously though, I think you did a great job.
I just finished the React part of the course - by the time I’d finished React and Redux I was feeling a bit shell-shocked!

So, I’m pretty impressed by your Weather App. :smile:

1 Like

After I finished most of the code on this app it occurred to me I might have missed an opportunity by not doing it with REACT instead… it would have been interesting, but I’m not about to re-write it, lol.

You’ve got something that’s working nicely - no reason to change it.

And the wind gust speed looks a lot better now, thanks for changing that.

1 Like

Definitely like the look of your weather app! I do have a couple of suggestions:

  1. Make sure to use const instead of let when a variable’s value is not going to be modified. You are using let many times where const should be used. Using the applicable const and let keywords helps someone reviewing your code to know what should and should not be modified later in your code.

  2. You have several sections where you are updating the innerHTML property for an element. You are concatenating several strings with variables but it starts to look a bit messy. My suggestion is to use Template Literals to make the code more readable and look more like HTML. Also, I suggested defining all the elements used on the page in your global variables section, so you do not have to use document.getElementById('elementID') in the functions that reference them.

For example, in your drawTemps function, I moved the declaration of the elements updated in this section to the global variables section as follows:

const currentTempElem = document.getElementById('current-temp');
const unitTextElem = document.getElementById('f-or-c');
const tempsElem = document.getElementById('temps');
const humidityElem = document.getElementById('humidity');

Then , in the latter part of the function where the elements are updated, I used Template Literals where applicable:

I changed the following:

  document.getElementById('current-temp').innerHTML = '<span class="current-temp" style="color: '+theColor+'">'+temp+'°</span> <span class="feels"><br>Feels Like <span class="value">' + convTemps(feels_like) + '°</span></span>';
  document.getElementById('f-or-c').innerHTML = is_f?"F":"C";
  document.getElementById('temps').innerHTML = 'Today\'s High: <span class="value">' + convTemps(temp_max) + '°</span><br>Today\'s Low: <span class="value">' + convTemps(temp_min) + "°</span>";
  document.getElementById('humidity').innerHTML = 'Humidity: <span class="value">' + humidity + ' % </span><br>Pressure: <span class="value">' + pressure + ' hpa</span>';


  currentTempElem.innerHTML = `
    <span class="current-temp" style="color: ${theColor}">${temp}°</span> <span class="feels">
    Feels Like <span class="value">${convTemps(feels_like)}°</span></span>
  unitTextElem.innerHTML = is_f?"F":"C";
  tempsElem.innerHTML = `
    Today's High: <span class="value">${convTemps(temp_max)}°</span>
    Today's Low: <span class="value">${convTemps(temp_min)}°</span>
  humidityElem.innerHTML = `
    Humidity: <span class="value">${humidity} %</span>
    Pressure: <span class="value">${pressure} hpa</span>

I suggestion doing something similar for the section of your getWeatherInfo function that updates the innerHTML property of various elements.

  1. Related to the getWeatherInfo function, I also suggest having it only “getting the weather data”. Functions should be limited to one task if possible. Instead of getting the data and displaying to the DOM in the same function, I suggest that the function return the data as an object and then pass that object to another function to do the actual updates to the DOM. Having a separate function for displaying will make it easy for someone (a future team member) to go and update the one function’s display logic if so needed.

Wow, thanks for that indepth look, all really great points for me to focus on… yeah, while writing just to get it working I revert to base level messy coding, but never really cleaned it up afterward… Definitely looks better with the Template Literals (which I had completely forgot about) and declaring the constants. I think most of my let declarations are due to the logic that the values would need to change everytime someone enters a new location, but I guess as the scope of those variables are in the function, they aren’t being changed, their being re-declared, so yeah, should have used const.

Thanks again :slight_smile: