Conversation
There was a problem hiding this comment.
Overall, great work this week! It was awesome to see you complete so many stretch features.
What’s working well:
- Clear component names: You’ve done an excellent job naming your components in a clear and descriptive way. This really helps in understanding the purpose of each component at a glance, making the codebase much more navigable.
- Organized CSS files: Thanks for separating styles into files by component, it enhances maintainability and makes it easier to manage styles as the project grows.
- Good parent-child structures: You demonstrated a good understanding of component architecture, making the data flow and component responsibilities clear and logical.
- Effective use of state and props: You’ve demonstrated a solid grasp of managing state and props for displaying the movies in the sidebar.
Areas for improvement:
- Destructuring props: To make your components even cleaner, let's destructuring props at the beginning of your functional components. This not only reduces the props. repetition throughout the component but also makes the code cleaner and easier to read
- CSS Class Repetition: I noticed some repetition in the CSS ids. Let's combine styles (eg. button styles) into one unified class
- Combining commits: Try to combine related changes into fewer commits. This practice can help in keeping the project history more streamlined and meaningful. Tools like git rebase can be really handy for combining commits and cleaning up your commit history before merging.
| } | ||
| } | ||
|
|
||
| const className = Number(props.rating) < 5 ? 'bad' : Number(props.rating) < 7.5 ? 'okay' : 'good' |
There was a problem hiding this comment.
instead of allocating a variable for this, we typically like to do this inline:
<div className={{Number(props.rating) < 5 ? 'bad' : Number(props.rating) < 7.5 ? 'okay' : 'good'}}
| import './SideBar.css'; | ||
|
|
||
| const SideBar = (props) => { | ||
| const likedMovies = props.likedList; |
There was a problem hiding this comment.
typically we like to destructure the props to access them:
const {likedList, watchedList, showing, etc} = props;
| import React from 'react'; | ||
| import './SideBarCard.css'; | ||
|
|
||
| const SideBarCard = (props) => { |
There was a problem hiding this comment.
similarly, prop destructuring preferred
| const updatedWatchedList = props.watchedList.filter(movie => movie.id !== props.id); | ||
| props.setWatchedList(updatedWatchedList); | ||
| } | ||
| }else { |
There was a problem hiding this comment.
in the future, let's run lint to fix some of these spacing issues
| } | ||
|
|
||
| #searchTabButton { | ||
| background-color: #B19CD9; |
There was a problem hiding this comment.
we reuse background-color: #B19CD9; quite a few times here, in the future let's try to capture repeated styles in a separate class so we can avoid repetition
| } | ||
|
|
||
| export default App | ||
| export default App; |
There was a problem hiding this comment.
you can do export default component App(){...} more to read on React flow component syntax https://fburl.com/6hxctx6w
|
|
||
| return ( | ||
| <> | ||
| <header className="header"> |
There was a problem hiding this comment.
this should have an indent. Somehow format is important for code readebility. things like indentation, empty line,...
src/Modal.css
Outdated
| color: white; | ||
| } | ||
|
|
||
| .body-no-scroll { |
There was a problem hiding this comment.
let's try to keep the styles name format consistent, bodyNoScroll
| // loadButton.style.display = 'none'; | ||
| } else if (selectedValue === "rating-asc"){ | ||
| sortByRatingAsc(); | ||
| } |
There was a problem hiding this comment.
can use 'switch case' here when multiple if else
| }else { | ||
| setMovies(results); | ||
| } | ||
|
|
There was a problem hiding this comment.
Overall looks good to me! The website looks great to me and functioning well!
Just a few nit on spacing and newlines. I noticed that a lot of functions have series of consecutive newlines, can we delete the newlines to make file structure cleaner?
src/App.jsx
Outdated
| const loadMore = () => { | ||
| setPage(page => page + 1); | ||
| }; | ||
| // setModal(!modal); |
| <ul className="side-bar-list"> | ||
| <div className="liked-movies-div"> | ||
| <li className="liked-movies">Liked Movies</li> | ||
| <li className="conatiner-names">Liked Movies</li> |
src/Modal.css
Outdated
| height: 100%; | ||
| overflow: hidden; | ||
| background-color: rgba(225, 225, 225, 0.5); | ||
| /* margin-top: 15px; */ |
src/App.jsx
Outdated
| const [watchedList, setWatchedList] = useState([]); | ||
| const apiKey = import.meta.env.VITE_API_KEY; | ||
|
|
||
| console.log(likedList); |
There was a problem hiding this comment.
let's remove the console log stuff before committing.
| } else if (likeCount >= 1) { | ||
| const heart = document.getElementById(props.id).querySelector('#like-count'); | ||
| setLikeCount(likeCount - 1); | ||
| heart.innerText = `♡`; |
There was a problem hiding this comment.
The function is called "increaseLikes", but the likeCount is decreased here. Do you want to update the logic or rename the function?
5 participants
No description provided.