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.

Sign up for GitHub

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

Jump to bottom

Joyce Kuo Weather App #423

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

Joyce Kuo Weather App #423

wants to merge 14 commits into from

Conversation

JoyceKuode
Copy link

@JoyceKuode
Add base HTML, CSS, and JavaScript files
75a1a1e
@JoyceKuode
Complete step 2 - present city name, temperature, and weather
c791226
@JoyceKuode
Add sunrise and sunset times and create a function to convert to read…
a0c9205 
…able time format
@JoyceKuode
Add forecast URL and update HTML with 5 day forecast
66a30ae
@JoyceKuode
Add ellipse to top container and edit CSS for responsiveness
da45dc8
@JoyceKuode
Edit forecast container to match design
8c05749
@JoyceKuode
Add search icon and input field with function to toggle search/close …
0b7af8d 
…icon and display weather of searched city
@JoyceKuode
Add arrow icon to cycle through 5 cities and their weather data on click
51635e3
@JoyceKuode
Fix positioning of arrow icon
ca29873
@JoyceKuode
Add background images and function that displays image based on weath…
71e968e 
…er ID and local time
@JoyceKuode
Edit CSS to make more of the background image visible
a2ef0a6
@JoyceKuode
Add feature that triggers warm/cold colors based on the current tempe…
4d11814 
…rature
@JoyceKuode
Fix layout issue to make sunrise and sunset times visible on all screens
52e5bc8
@JoyceKuode
Edit README file
d2beb30
HIPPIEKICK
HIPPIEKICK approved these changes Oct 3, 2024
View reviewed changes
Copy link
Contributor

@HIPPIEKICK HIPPIEKICK left a comment

Choose a reason for hiding this comment

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

HTML/CSS

  • Well structured HTML file! Just remember to put the script within the body. Also, consider using more semantic elements instead of just divs
  • Real nice work with copying the design! Love the color overlay!
  • Would be super nice if the arrow to change city had a pointer cursor on hover
  • Try to keep the JS to the JS file (using event listeners instead of the onclick method)

JavaScript

  • Well structured and easy-to-follow JS 👍 A lot of well-named and well-structured functions. The capitalizeFirstLetter function is beautiful!

Clean Code

  • Be consistent with if you're using single or double quotes

No changes requested, just keep up the good work! 🥳

@HIPPIEKICK HIPPIEKICK self-assigned this Oct 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@HIPPIEKICK HIPPIEKICK HIPPIEKICK approved these changes

Assignees

@HIPPIEKICK HIPPIEKICK

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@JoyceKuode @HIPPIEKICK