Ellie & Laura Ada C23

[lavendercaterpillar]

No description provided.

[yangashley]

Nice job on weather-report, team!

[yangashley]

👍🤗

[yangashley]

Suggested change

	"user strict"
	"use strict"

[yangashley]

Nice job using constant variables to reference the endpoints.

Suggested change

	const locationURL = 'http://127.0.0.1:5000/location'
	const weatherURL = 'http://127.0.0.1:5000/weather'
	const LOCATION_URL = 'http://127.0.0.1:5000/location'
	const WEATHER_URL = 'http://127.0.0.1:5000/weather'

[yangashley]

Remove debugging print statements after you're done using them to keep your project from getting bloated with code that isn't necessary to the implementation.

Suggested change

	console.log('renderTemp temp= ', temp)
	console.log('currentTemp state= ', state.currentTemp)

[yangashley]

As a way to further organize your project, I'd prefer that the elements that you access to make updates to the page get declared here and set all at once during page load, like we did in this example: https://github.com/Ada-Activities/Pick-Me-Up-Pup/blob/c23-solution/src/index.js#L4-L10

[yangashley]

Suggested change

	// // console.log('in realtime set, before get real time', state.currentTemp)
	// state.currentTemp = getRealTimeTemp();
	// temp.textContent = state.currentTemp;
	// // console.log('in realtime set after real time, state of current temp', state.currentTemp)
	// renderTemp(temp);
	// return

All commented out code that is not necessary to the implementation should be cleaned up before a PR.

At work, opening a PR for review is asking for your code to be merged into main. A team will most likely not allow any extra lines of code to be merged into main because it bloats the code base and makes it unorganized and difficult to maintain.

It can also be confusing to read this and need to figure out, why was this left in? should i uncomment it?

It could get accidentally uncommented which could result in a bug.

[yangashley]

Suggested change

	// .then(response => {
	// const { lat, lon } = response;
	// return axios.get(weatherURL, {
	// params: {
	// lat: lat,
	// lon: lon
	// }
	// })
	// })

Remove commented out code

[yangashley]

It looks like these two functions are meant to run during page load because I don't see them encapsulated in a function. It's not immediately evident to me that there's code that should run on page load because it's kind of sprinkled throughout this js file.

For example, I see a stand alone function call on lines 180-181, line 207 that executes on page load.

I'd prefer a method onLoaded that executes everything required for setting the page up.

const onLoaded = () => {
  registerEventHandlers();
  loadControls();
};

document.addEventListener("DOMContentLoaded", onLoaded); 

[yangashley]

Suggested change

// cityOutput.textContent = inputFieldValue;

Delete uneccessary code

[yangashley]

Could loop through an object to set these values instead of having a lot of conditional logic here.