-
Notifications
You must be signed in to change notification settings - Fork 198
Week 2 Project PR #57
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
base: main
Are you sure you want to change the base?
Conversation
Changed small thing with the walkthrough link
armanhalizadeh
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great job on the project this week. The site is snappy and responsive and looks great. Also great job implementing what we talked about last week. All variable names/method names are clear and easy to read, no dead code and more compartmentalization of code.
For the upcoming week lets focus on compartmentalizing with components even more and better tag usage.
| } | ||
|
|
||
| return ( | ||
| <div> |
There was a problem hiding this 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 good job of using semantic classes throughout your project, but we can take it a step further for future projects. Whenever you find yourself using
Here is the link to a list of semantic tags that we discussed in our 1:1: https://www.w3schools.com/html/html5_semantic_elements.asp
| <div className="App"> | ||
|
|
||
| <header> | ||
| <Header/> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great job on building out these components to hold the relevant code. A great implementation of what we discussed last week, creating more classes/methods to allow for cleaner code and more reuse. I think we can take it even a step farther, for example the sort method you have above is only used in the SortBy component so we can probably move the logic into that jsx file and pass down the relevant props.
|
Great job on implementing some of the stretch features as well!! |
No description provided.