Snow Leopards - Arika A. Ja H.

[arikaagnew]

No description provided.

[anselrognlie]

Looks good! Try to think about how to refactor the API call helpers so that they're a little more independent and reusable. I have some suggestions in the comments. There's also quite a bit of code based on similar if/else if statements that we could think about making more data-driven, simplifying our code, and reducing the likelihood of typos, so check my comments for more details.

[anselrognlie]

Nice use of grid to layout your page.

[anselrognlie]

Some other approaches to avoid going down the road of positioning (this way lies madness) could be to use a grid for the contents of the weatherContainer and give all the growth to a middle row, leaving minimally sized rows to hold the sky and ground.

As it is, the absolute setting here is modified by this element being nested in a relative element higher up, which becomes the baseline against which child absolute positioning takes place.

Some layouts are absolutely achieved most directly using position rules, but if we have other layout methods, it can leave things open to future modifications if we avoid them. My personal feeling is that position rules tend to be very finicky and prone to breakage, though that may just be me.

[anselrognlie]

Be sure to validate your html when possible. The errors/warnings can help us write cleaner HTML. Even if our markup looks fine in the browser we're using, there's no guarantee invalid HTML will do so on other browsers, and it could even break in the same browser since there are no layout guarantees made for invalid HTML.

[anselrognlie]

Missing space between the attributes. Prefer to omit spaecs around the = for html attributes.

        <section class="container" id="weatherContainer">

[anselrognlie]

It looks like you were planning to bring in Font Awesome for some icons, but I don't see the needed links/imports.

[anselrognlie]

Prefer not grabbing values out of the UI for further processing. Rather, have a variable that's used both to update the UI display, and can be referenced to perform related operations (like getting the weather for the city).

[anselrognlie]

Rather then calling the weather api call directly, consider returning the lat/lon from this .then callback so that it would be available to act as the input to a subsequent .then callback. We could then make a separate function whose responsibility is to combine the city->lat/lon function with the lat/lon->weather function.

Assuming getLongLat was a function taking a city name and returning a promise to an object shaped like {lat: 0, lon: 0}, and getWeather was a function taking lat and lon values, and returning a promise to a temperature, we could write a third function getWeatherForCity as follows:

const getWeatherForCity = (cityName) => {
  return getLongLat(cityName)
    .then(({lat, lon}) => {
      return getWeather(lat, lon);
    })
};

We could then use this as

getWeatherForCity(state.city)
  .then(temp => {
      // code currently in getWeather below
      document.getElementById('startingTemp').textContent = Math.round(fahrenheit);
      state.startingTemp = Math.round(fahrenheit);
      updateColor();
      updateLandscape();
  })

[anselrognlie]

Be sure to return here as mentioned above.

[anselrognlie]

Rather than updating the application immediately here, consider returning the temperature from the .then callback. This would make it available to a subsequent .then callback that could be chained outside this helper. In that case, we also need to be sure to return the end of the promise chain itself from the helper, so that additional actions can be chained to run after this helper completes.

[anselrognlie]

Consider either storing these element references in your state for later use, or even making global variables to hold them, along with any other elements needed for the operation of the app.