C22- Sphinx - Weixi He, Nikki Vaughan / Phoenix - Ajar Duishembieva

[vnikki13]

No description provided.

[kelsey-steven-ada]

Nice work folks, let me know if you have any question on the feedback! =]

[kelsey-steven-ada]

The Weather Garden's sky will only appear once a user selects an option other than "Sunny" for the first time, even though it looks like a default of "Sunny" is selected. Where could we call this function so the sky is displayed along with the garden when it is updated?

[kelsey-steven-ada]

I suggest using something like a h2 or a p element around the inner span here.

span is much like div where it does not inherently have semantic meaning, so we want to reserve it for places where we need an inline element to wrap something for styling-purposes only. More info: https://developer.mozilla.org/en-US/docs/Web/HTML/Element/span

[kelsey-steven-ada]

There are some divs across the HTML like this one that I might suggest replacing with section or article elements because it looks they are meaningfully sectioning content. That way we only leave divs where they are wrapping content for styling purposes.

I suggest taking a look at each div across the file and asking: if it is doing more work than styling, is there any semantic element that could be a better fit?

[kelsey-steven-ada]

Since this line is repeated without change in each if/else block, we can remove it from these blocks and perform the operation just once before the if/else chain starts.

[kelsey-steven-ada]

Since we already set a default city, what could it look like to set a default temperature so that a user would have a weather and sky to look at and easily navigate between when the page loads up?

[kelsey-steven-ada]

increaseTemp and decreaseTemp might not be operating exactly as you expect since the color change function gets called before we actually change the temperature value.

When you manually increase the temperature starting at 68, I have to go to 71 before I see the next set of garden emojis appear. The same happens when I decrease from 71, it doesn't change back to the previous garden until the temperature reads 68.

[kelsey-steven-ada]

Since increaseTemp and decreaseTemp are very similar, another approach we could take is to combine them into 1 function:

const setTemp = (changeInTemp) => {
  value = parseInt(temp.textContent) || 0;
  changeColor(value)
  temp.innerText += changeInTemp;
};

Then where we register the handlers, we could call our new function with a parameter by doing so inside an anonymous function:

  increment.addEventListener('click', () => { setTemp(1) });
  decrement.addEventListener('click', () => { setTemp(-1) });

[kelsey-steven-ada]

We should remove console.log statements from our Javascript before submission just like with print statements in our Python code.

[kelsey-steven-ada]

When we have large if-statements like this where each if-block performs the same operations with slightly different values, it can be worth making a helper function that handles the updates to reduce places we're repeating code and could accumulate small typos. With the helper function, each of the ifs here would call that function passing the desired parameters rather than setting attributes themselves.

[kelsey-steven-ada]

Commented out code should be removed before opening PRs.