SeaTurtles - Adriana G.

[adrianajg]

Hello!

Please note that I replaced the landscape requirement with a thermometer icon that changes depending on the temperature. I also replaced the drop-down menu for weather conditions to icons, and instead used the drop-down menu to select the state (I did this purely for aesthetics as I liked how the weather icons looked at the bottom of the screen.)

Please also note that there is SO much refactoring that needs to be done. I didn't have time to delete comments or fix the many bugs that exist, but this is what I have so far.

Improvements I would make if I had more time:

• clean up code in general
• add background boxes around the temperature elements on the second and third "pages" or scroll regions like I have on the first page
• Fix city and state displays for cities and states that have multiple words (for example New York vs Charlotte).
• Make chosen weather default on third page be the same as the searched weather results on the second page

Feature I wanted to add:

• API call to use user's IP address to set default location for that user.

[kelsey-steven-ada]

Great work Adriana! I've left some comments & suggestions, please take a look when you have time and let me know if there's anything I can clarify =]

[kelsey-steven-ada]

We want to clean up unused code & comments before opening our PRs to make sure they don't make it into the main/deployment branch of our projects.

[kelsey-steven-ada]

I like the idea of pulling element lookups to the top of the page so they're stored in a variable at the time we need them!

It looks like there's a mix styles across the file with these 3 elements being looked up & stored up here and other elements looked up directly in the functions. For consistency, we would want to pick one style to use across the project, with the caveat that if there is an element that is only conditionally aded to the DOM based on user events, that specific item may need to be looked up later inside a function when it is guaranteed to exist.

[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.

const updateTempColor = () => {

    ...

    if (STATE.currTemp < 50) {
        updateChosenTempTextAndImage('cold', './assets/thermometer-icons/cold.png');
    } else if ...

}

...

const updateChosenTempTextAndImage = (category, imagePath) => {
    chosenTempText.id = category;
    chosenTherm.src = imagePath;
}

[kelsey-steven-ada]

I would strongly recommend using CSS to set padding & margins to create space between elements over adding empty block elements like divs

[kelsey-steven-ada]

I would recommend replacing the divs here with elements that have more semantic meaning like section. This applies to most of the div uses I see across the HTML. We should reserve div for when no other semantic element makes sense or we need a container for styling that does not otherwise have meaning to the layout.

[kelsey-steven-ada]

Really neat paged layout!

[kelsey-steven-ada]

Nice use of .then chaining to fire the calls off in sequence! This function is a bit long, if we wanted to divide it up for clarity, we could move the /weather endpoint call to its own function and call the new function from a .then block here.

[kelsey-steven-ada]

Really nice use of helper functions to divide up the actions we want to take into smaller portions that are easier to read and test.

[kelsey-steven-ada]

Love the neon effects on the temperature!

[kelsey-steven-ada]

I like how the thermometers turned out! Very cute ^_^