Kunzite: Angelica and Alyssa

[alyssa-tn]

No description provided.

[spitsfire]

Good job, Angelica and Alyssa. I've left some feedback on how you can refactor your JavaScript in order to make it more organized.

Things to consider:

• When assigning variables, don't forget your semicolon, even if the value is a function.
• Collect all your addEventListener('DOMContentLoaded') calls into one function in order to dry up repetitive code.

[spitsfire]

Suggested change

	'use strict'
	'use strict';

[spitsfire]

Looks like there are several elements scattered around the file, along with some repetitive code for registering event listeners individually.

Let's streamline this by creating one function that will contain any elements that need an event listener:

const registerEventHandler = () => {
    const skyOptions = document.getElementById('skySelect');
    skyOptions.addEventListener('change', changeSky);

    const decreaseTempButton = document.getElementById('decreaseTempControl');
    decreaseTempButton.addEventListener('click', decreaseTemp);
    // or: decreaseTempHandler(); since you have a function already built for this
    // so on and so forth
};

// then, likely placed at the bottom of the file
document.addEventListener('DOMContentLoaded', registerEventHandler);

[spitsfire]

The rest of the variables that grab the values we wish to calculate or re-render, let's collect them together for the sake of organization, or collect them all into one object of key-value pairs.

Just like the event and event listeners we registered in registerEventHandler, we can create an object of initial state:

const state = {
    tempValue: 0,
    currentCity: 'Seattle',
    landscape: null
};

// then inside the registerEventHandler
state.tempValue = parseInt(document.getElementById('tempValue').textContent);
state.landscape = document.getElementById('landscape');
// so on and so forth

This isn't necessary, but it would create some structure and organization for other coders when reading your code.

[spitsfire]

Another way to format these two methods (maybe combine them into one) would be to have an object that has temp values as keys, and then you could have values be a list. The list could hold background color and background image, like:

const tempDisplayObject = {
    80: ['red',  "🌵__🐍_🦂_🌵🌵__🐍_🏜_🦂"], 
    70: ['orange', "🌸🌿🌼__🌷🌻🌿_☘️🌱_🌻🌷"]
} 

When you have an object, you don't need to do if/else if/else if/ else which can introduce bugs and also adds complexity to the method. If you have an object, than you can iterate over the object and check the keys with state.tempValue and then get the correct values to set the color and landscape emojis.

[spitsfire]

👍 Nice job separating out the axios promises into individual functions!

[spitsfire]

Suggested change

[spitsfire]

Suggested change

	}
	};

[spitsfire]

Suggested change

	}
	};

[spitsfire]

Suggested change

	}
	};

All assignments should have a semicolon at the end

[spitsfire]

Does the promise this function returns need a then and catch here?