C24-Jesica & Xi by xiwu5 · Pull Request #8 · Ada-C24/weather-report

xiwu5 · 2025-12-05T22:22:54Z

No description provided.

…ing purpose

… value instead of object for function

…eInput's value(avoid touch the object directly)

…readability

Copilot

Pull request overview

This pull request implements a weather report application with real-time temperature fetching and interactive UI components. The implementation appears to be a student project (C24-Jesica & Xi) that creates a weather-themed application with temperature controls, sky conditions, and city selection.

Key Changes:

Implementation of weather report application with temperature display and controls
Integration with weather API using Axios for real-time data
Interactive UI with city name selection, sky conditions, and visual weather garden

Reviewed changes

Copilot reviewed 5 out of 6 changed files in this pull request and generated 8 comments.

Show a summary per file

File	Description
package.json	Added axios and dotenv dependencies with version issues
package-lock.json	Generated lockfile for new dependencies with incorrect versions
index.html	Added complete HTML structure for weather application UI
styles/index.css	Added comprehensive CSS styling for weather components
src/index.js	Implemented JavaScript functionality with minor naming issue
coworking_agreement.md	Completed team agreement with spelling errors and future dates

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2025-12-05T22:28:25Z

 ## Accessibility Needs
 *What does each team member need access to in order to succeed and show up the best they can?*

+Jesica: Clear commuincation and transperancy of where each team memeber is at via Slack or meeting up outside of classroom hours. Attend Office hours for extra help when stuck in a wave.


Spelling error: "transperancy" should be "transparency".

Suggested change

Jesica: Clear commuincation and transperancy of where each team memeber is at via Slack or meeting up outside of classroom hours. Attend Office hours for extra help when stuck in a wave.

Jesica: Clear commuincation and transparency of where each team memeber is at via Slack or meeting up outside of classroom hours. Attend Office hours for extra help when stuck in a wave.

Copilot · 2025-12-05T22:28:25Z

 ## Accessibility Needs
 *What does each team member need access to in order to succeed and show up the best they can?*

+Jesica: Clear commuincation and transperancy of where each team memeber is at via Slack or meeting up outside of classroom hours. Attend Office hours for extra help when stuck in a wave.


Spelling error: "memeber" should be "member".

Suggested change

Jesica: Clear commuincation and transperancy of where each team memeber is at via Slack or meeting up outside of classroom hours. Attend Office hours for extra help when stuck in a wave.

Jesica: Clear commuincation and transperancy of where each team member is at via Slack or meeting up outside of classroom hours. Attend Office hours for extra help when stuck in a wave.

Copilot · 2025-12-05T22:28:25Z

 ## Collaboration vs. Individual Work Expectations
 *Clarify your collaboration expectations - does your group want to write code together all of the time? Or divide work to do independently, then come together to share accomplishments? What tools and technologies can help your collaboration?*

+Coding together for the first 2 waves so we can both be on the same page. Once we have a throught plan we can split the work up wether thats splitting 1 wave or all the waves together. 


Spelling error: "throught" should be "thought".

Copilot · 2025-12-05T22:28:26Z

 ## Collaboration vs. Individual Work Expectations
 *Clarify your collaboration expectations - does your group want to write code together all of the time? Or divide work to do independently, then come together to share accomplishments? What tools and technologies can help your collaboration?*

+Coding together for the first 2 waves so we can both be on the same page. Once we have a throught plan we can split the work up wether thats splitting 1 wave or all the waves together. 


Spelling error: "wether thats" should be "whether that's".

Copilot · 2025-12-05T22:28:26Z

 ## Accessibility Needs
 *What does each team member need access to in order to succeed and show up the best they can?*

+Jesica: Clear commuincation and transperancy of where each team memeber is at via Slack or meeting up outside of classroom hours. Attend Office hours for extra help when stuck in a wave.


Spelling error: "commuincation" should be "communication".

Copilot · 2025-12-05T22:28:26Z


 ## One Team Communication Skill to Improve
 *What is a teamwork-related skill you want to work on?*
+Jesica: I would like to work on explaining code better to make sure my partner understand where I cam coming from or how did i get to the solution. 


Spelling error: "cam" should be "am".

Copilot · 2025-12-05T22:28:26Z

+  skyElement.textContent = getSkyDisplay(option);
+};
+
+const skySelectDropDwon = () => {


Function name typo: "skySelectDropDwon" should be "skySelectDropDown" (missing "o" and incorrect capitalization).

Copilot · 2025-12-05T22:28:26Z

+  }
+
+  updateSkyDisplay(skySelectElement.value);
+}


Avoid automated semicolon insertion (97% of all statements in the enclosing script have an explicit semicolon).

yangashley · 2025-12-11T21:37:07Z

@@ -0,0 +1,242 @@
+'use strict';


👍 Nice job remembering to add 'use strict'

yangashley · 2025-12-11T21:37:36Z

@@ -0,0 +1,242 @@
+'use strict';
+
+const BASE_URL = 'https://ada-weather-report-proxy-server.onrender.com';


👌 Good work using a constant value to refer to the proxy-server URL.

yangashley · 2025-12-11T21:40:50Z

+let tempElement = null;
+let landscapeElement = null;
+let skyElement = null;
+let headerCityElement = null;
+let cityNameInputElement = null;
+let cityNameResetElement = null;
+let increaseTempControlElement = null;
+let decreaseTempControlElement = null;
+let currentTempButtonElement = null;
+let skySelectElement = null;
+let gardenContent = null;


Would prefer that these variables that reference elements are stored in the state object instead of having a bunch of global variables that are declared using let that could be accidentally changed (which could introduce bugs).

You can see an example of how we did this in pick-me-up-pup here.

yangashley · 2025-12-11T21:42:55Z

+const getElementBySelector = (selector) => {
+  const element = document.querySelector(selector);
+  if (element) {
+    return element;
+  } else {
+    return null;
+  }
+};


This helper function is unnecessary because it duplicates behavior that querySelector already has. Prefer to directly use querySelector without this helper.

Per the MDN docs "The Document method querySelector() returns the first Element within the document that matches the specified CSS selector, or group of CSS selectors. If no matches are found, null is returned."

yangashley · 2025-12-11T21:44:47Z

+const addCounter = () => {
+  state.counter += 1;
+  displayCurrentTemp();
+};
+
+const subtractCounter = () => {
+  state.counter -= 1;
+  displayCurrentTemp();
+};


The difference between these addCounter and subtractCounter are + and -. Can you think of a way to write this logic with just one function that increase or decrease the temperature without repeating logic.

yangashley · 2025-12-11T21:49:21Z

+  return '🌲🌲⛄️🌲⛄️🍂🌲🍁🌲🌲⛄️🍂🌲';
+};
+
+const changeCurrentTempColor = (temp) => {


This name could be a little more descriptive. When I read changeCurrentTempColor I make the assumption that this function changes the color of the temperature display (like 91 would be red). This function does 2 things though because it also updates the landscape.

A name like changeCurrentTempColorAndLandscape would be more descriptive.

yangashley · 2025-12-11T21:50:50Z

+    setTemperatureAndDisplay(temp);
+  } catch (err) {
+    console.error('Failed to fetch temperature:', err);
+    setTemperatureAndDisplay(DEFAULT_TEMPERATURE);


Nice job coming up with a default fallback plan for scenario where the call to get the current temp fails.

yangashley · 2025-12-11T21:56:05Z

+  if (option == 'Sunny') {
+    skyElement.textContent = '☁️ ☁️ ☁️ ☀️ ☁️ ☁️';
+    gardenContent.style.backgroundColor = 'rgb(221, 255, 255)';
+  } else if (option == 'Cloudy') {
+    skyElement.textContent = '☁️☁️ ☁️ ☁️☁️ ☁️ 🌤 ☁️ ☁️☁️';
+    gardenContent.style.backgroundColor = 'lightgrey';
+  } else if (option == 'Rainy') {
+    skyElement.textContent = '🌧🌈⛈🌧🌧💧⛈🌧🌦🌧💧🌧🌧';
+    gardenContent.style.backgroundColor = 'lightblue';
+  } else if (option == 'Snowy') {
+    skyElement.textContent = '🌨❄️🌨🌨❄️❄️🌨❄️🌨❄️❄️🌨🌨';
+    gardenContent.style.backgroundColor = 'lightsteelblue';
+  }


What if the adjectives that describe the weather like "Sunny", "Rainy", etc were keys in an object and the color and sky content were values?

If you had an object that held this data then you could refactor this implementation so you wouldn't need to chain many if-statements (which is brittle and difficult to maintain).

You could use option to get the corresponding sky and garden values:

let option = skySelectElement.value; const sky_data = { "Rainy": ['☁️ ☁️ ☁️ ☀️ ☁️ ☁️', 'rgb(221, 255, 255)'], // rest of your data }; skyElement.textContent = sky_data[option][0]; gardenContent.style.backgroundColor = sky_data[option][1];

yangashley · 2025-12-11T22:03:21Z

+  displayCurrentTemp();
+};
+
+document.addEventListener('DOMContentLoaded', registerEventHandlers);


Nice job organizing your code! I like that you have a function that 'loads' all the elements, like you do in initializeElements and I like that you have a function that registers all the event handlers.

The function registerEventHandlers does more than just registering event handlers though because it also performs some initial logic that needs to happen when the page loads (initializeElements and displayCurrentTemp).

I'd prefer this initial set up logic to be in a separate helper function, maybe something like setUpUI. And also introduce a function like onLoaded that would call setUpUI and registerEventHandlers. You can see an example of how we do this here in pick-me-up-pup.

const setUpUI = () => { // steps to carry out when the page has loaded initializeElements(); displayCurrentTemp(); }; const onLoaded = () => { setUpUI(); registerEventHandlers() }; document.addEventListener('DOMContentLoaded', onLoaded);

yangashley · 2025-12-11T22:05:01Z

+  if (cityNameInputElement) {
+    const initialCityName = getCityInputValue();
+    // Set initial city name header
+    updateCityNameHeader(initialCityName);
+    cityNameInputElement.addEventListener('input', () => {
+      const cityName = getCityInputValue();
+      updateCityNameHeader(cityName);
+    });
+  }


I'd prefer this logic to be in a function so that you can pass a named function to addEventListener on line 229 (instead of passing an anonymous function like you currently do).

Having named functions that you can pass to addEventListener makes your code easier to understand and maintain.

jguall421 and others added 21 commits December 3, 2025 10:35

Signed Agreement and Installed

4679c72

Temp button is interactive

2c477cd

initialize temperature display

9ce6ad0

wrapup code as a renderTemp() helper, set default temp as 65 for test…

80ed8b9

…ing purpose

Wave02: temp ranges change text color done!

420ca88

Wave 2 completed added change landscape

9d6d7d4

city name practice

289779e

Wave03:fixed bug for cityname display by add on symbol to id and pass…

4ead082

… value instead of object for function

delete extra comment

2a72b4b

Wave03: improve readability of cityname handler

8b86786

refactor variable names and methods to remain consistency

3368aae

wave06: cityname reset button done! Add a helper to deal with citynam…

f1b3618

…eInput's value(avoid touch the object directly)

write a helper for helpers to reduce the total number of helpers

16023a9

installed axios and dotEn

b7911fc

Merge branch 'main' of https://github.com/jguall421/weather-report

3a611f5

create .enc file

25f1a53

Merge branch 'main' into wave-4

1698d98

Wave 4: API and EventHandle Button Working for CurrentTemp

25d25bd

Wave 05: skySelect structure setted up

9090b1e

add chenge to eventhandle

c5ce279

DRY code by adding helpers and refactoring variable names to improve …

9365a75

…readability

Copilot AI review requested due to automatic review settings December 5, 2025 22:22

Copilot started reviewing on behalf of xiwu5 December 5, 2025 22:23 View session

Copilot AI reviewed Dec 5, 2025

View reviewed changes

jguall421 added 2 commits December 5, 2025 16:19

added more css/color

9d24f91

resolve merge

6272ed1

yangashley reviewed Dec 11, 2025

View reviewed changes

	Jesica: Clear commuincation and transperancy of where each team memeber is at via Slack or meeting up outside of classroom hours. Attend Office hours for extra help when stuck in a wave.
	Jesica: Clear commuincation and transparency of where each team memeber is at via Slack or meeting up outside of classroom hours. Attend Office hours for extra help when stuck in a wave.

		@@ -0,0 +1,242 @@
		'use strict';

		const BASE_URL = 'https://ada-weather-report-proxy-server.onrender.com';

Conversation

xiwu5 commented Dec 5, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Key Changes:

Reviewed changes

Uh oh!

Copilot AI Dec 5, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 5, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 5, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 5, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 5, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 5, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 5, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 5, 2025

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants