Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Weather App. Team: Darius, Therese, Tavan, Kasia, Jasmin #11

Open
wants to merge 32 commits into
base: main
Choose a base branch
from

Conversation

JasminHed
Copy link

Copy link

@JennieDalgren JennieDalgren left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great job with this app!

You have some things to fix before getting approved on this project:

  • Stick to one way of writing functions: const name = () => {} or function name() {}
  • The icons doesnt seems to fit the weather? It cant be right that all cities have the same forecast?
  • I can't see any conditional change of the design? Either day/night or based on weather?

@JasminHed JasminHed requested a review from JennieDalgren March 24, 2025 17:53
@JasminHed
Copy link
Author

Hello,
I have now updated our code to meet your requests and Therese has double checked our functions.
🌟

Copy link

@JennieDalgren JennieDalgren left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants