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 – Helene Westrin #421

Open
wants to merge 17 commits into
base: master
Choose a base branch
from

Conversation

HeleneWestrin
Copy link

@HeleneWestrin HeleneWestrin changed the title Weatherington – a weather app Weather app – Helene Westrin Sep 29, 2024
@JennieDalgren JennieDalgren self-assigned this Oct 4, 2024
Copy link
Contributor

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

You've done a fantastic job with the weather app! 🎉

HTML/CSS

  • Good job with the structure and nice little favicon too.
  • Great use of css variables!

JavaScript

  • Nice job with the fetching from the API as well as using some mock data. Clever!
  • Nice to see some error handling/empty state ⭐
  • Love the way you have used helper functions.

Clean Code

  • Your JavaScript is well-organized and modular, with helper functions like formatTemperature and getWeekdayAbbreviation.
  • The comments you’ve provided are generally helpful, although keeping them concise, sometimes your code is very explanatory itself.
  • Don't mix arrow functions with function keyword, be consistent

Changes Requested
No changes are necessary.

Keep up the great work, and I look forward to seeing your next project! 😊

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