Code Review #27

mayaajike · 2024-06-18T22:47:39Z

No description provided.

mellyeliu

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.

mellyeliu · 2024-06-18T23:15:35Z

src/MovieCard.jsx

+        }
+    }
+
+    const className = Number(props.rating) < 5 ? 'bad' : Number(props.rating) < 7.5 ? 'okay' : 'good'


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'}}

mellyeliu · 2024-06-18T23:18:17Z

src/SideBar.jsx

+import './SideBar.css';
+
+const SideBar = (props) => {
+    const likedMovies = props.likedList;


typically we like to destructure the props to access them:
const {likedList, watchedList, showing, etc} = props;

mellyeliu · 2024-06-18T23:18:36Z

src/SideBarCard.jsx

+import React from 'react';
+import './SideBarCard.css';
+
+const SideBarCard = (props) => {


similarly, prop destructuring preferred

mellyeliu · 2024-06-18T23:19:41Z

src/MovieCard.jsx

+                const updatedWatchedList = props.watchedList.filter(movie => movie.id !== props.id);
+                props.setWatchedList(updatedWatchedList);
+            }
+        }else {


in the future, let's run lint to fix some of these spacing issues

mellyeliu · 2024-06-18T23:21:13Z

src/App.css

+}
+
+#searchTabButton {
+  background-color: #B19CD9;


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

ireneluvsbear

Nice work!!

ireneluvsbear · 2024-06-19T00:16:49Z

src/App.jsx

 }

-export default App
+export default App;


you can do export default component App(){...} more to read on React flow component syntax https://fburl.com/6hxctx6w

ireneluvsbear · 2024-06-19T00:18:22Z

src/App.jsx

+
+  return (
+    <>
+    <header className="header">


this should have an indent. Somehow format is important for code readebility. things like indentation, empty line,...

ireneluvsbear · 2024-06-19T00:20:35Z

src/Modal.css

+    color: white;
+}
+
+.body-no-scroll {


let's try to keep the styles name format consistent, bodyNoScroll

ireneluvsbear · 2024-06-19T00:27:35Z

src/App.jsx

+      // loadButton.style.display = 'none';
+    } else if (selectedValue === "rating-asc"){
+      sortByRatingAsc();
+    }


can use 'switch case' here when multiple if else

hanluc · 2024-06-20T16:47:40Z

src/App.jsx

+    }else {
+      setMovies(results);
+    }
+


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?

hanluc · 2024-06-20T16:48:17Z

src/App.jsx

+  const loadMore = () => {
+    setPage(page => page + 1);
+};
+// setModal(!modal);


Why do we need this line?

hanluc · 2024-06-20T16:50:54Z

src/SideBar.jsx

@@ -13,7 +14,7 @@ const SideBar = (props) => {
            <div className="side-bar-content">
                <ul className="side-bar-list">
                    <div className="liked-movies-div">
-                        <li className="liked-movies">Liked Movies</li>
+                        <li className="conatiner-names">Liked Movies</li>


hanluc · 2024-06-20T16:51:42Z

src/Modal.css

+    height: 100%;
+    overflow: hidden;
+    background-color: rgba(225, 225, 225, 0.5);
+    /* margin-top: 15px; */


remove this unused line?

hanluc · 2024-06-20T16:53:27Z

src/App.jsx

  const apiKey = import.meta.env.VITE_API_KEY;

+  console.log(likedList);


let's remove the console log stuff before committing.

hanluc · 2024-06-20T16:59:43Z

src/MovieCard.jsx

+        } else if (likeCount >= 1) {
+            const heart = document.getElementById(props.id).querySelector('#like-count');
+            setLikeCount(likeCount - 1);
+            heart.innerText = `♡`;


The function is called "increaseLikes", but the likeCount is decreased here. Do you want to update the logic or rename the function?

mayaatmeta and others added 26 commits June 12, 2024 21:52

Added new Components

6bebcf1

Added App styling

5d03bde

Added Modal styling

065f2a2

Added MovieCard styling

d63990f

Added MovieCard Component

e5d78ad

Added MovieList styling

026f9f2

Added MovieList Component

883bfe1

Added Search Component

7916f26

Added api key

c3d439a

hid api key

091f138

edited file name

7472cab

Added Modal Component

147c02e

Working Modal

2f3a39b

Modal updated with runtime

c6f4bf5

Modal with runtime ✅

97585ba

Modal Component✅

c9ad1e3

Added like and watched features

116414d

Filtering and Sorting feature✅

677bb79

Footer added

e7fe6ff

Working sidebar, no movies added.

80ebbb4

Adding liked songs to side bar feature ✅

04a9369

Adding watched movies to sidebar feature✅

d7d123c

Updated modal with poster background and embedded youtube trailer

ef5479d

final css changes made

0ef6526

Update README.md

58adaa7

Update README.md

7b94bce

mellyeliu reviewed Jun 18, 2024

View reviewed changes

ireneluvsbear reviewed Jun 19, 2024

View reviewed changes

hanluc reviewed Jun 20, 2024

View reviewed changes

src/App.jsx Outdated

const loadMore = () => {

setPage(page => page + 1);

};

// setModal(!modal);

Copy link

hanluc Jun 20, 2024

Choose a reason for hiding this comment

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

Why do we need this line?

hanluc reviewed Jun 20, 2024

View reviewed changes

mayaatmeta and others added 3 commits June 20, 2024 16:46

style updates

6247b27

Merge branch 'main' of https://github.com/mayaajike/flixster-starter

e7f81f1

Update README.md

7d53211

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Code Review #27

Code Review #27

mayaajike commented Jun 18, 2024

mellyeliu left a comment •

edited

Loading

mellyeliu Jun 18, 2024

mellyeliu Jun 18, 2024

mellyeliu Jun 18, 2024

mellyeliu Jun 18, 2024

mellyeliu Jun 18, 2024

ireneluvsbear left a comment

ireneluvsbear Jun 19, 2024

ireneluvsbear Jun 19, 2024

ireneluvsbear Jun 19, 2024

ireneluvsbear Jun 19, 2024

hanluc Jun 20, 2024 •

edited

Loading

hanluc Jun 20, 2024

hanluc Jun 20, 2024

hanluc Jun 20, 2024

hanluc Jun 20, 2024

hanluc Jun 20, 2024 •

edited

Loading

		const apiKey = import.meta.env.VITE_API_KEY;

		console.log(likedList);

Code Review #27

Are you sure you want to change the base?

Code Review #27

Conversation

mayaajike commented Jun 18, 2024

mellyeliu left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ireneluvsbear left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hanluc Jun 20, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hanluc Jun 20, 2024 • edited Loading

Choose a reason for hiding this comment

mellyeliu left a comment •

edited

Loading

hanluc Jun 20, 2024 •

edited

Loading

hanluc Jun 20, 2024 •

edited

Loading