R-Jim / df-frontend-2023

https://df-frontend-2023-assignment1.vercel.app
0 stars 0 forks source link

Submission for assignment 2 #9

Closed R-Jim closed 11 months ago

R-Jim commented 12 months ago
zlatanpham commented 11 months ago

Hello @R-Jim, good work!

Requirements

Final result: ✅ passed 70% of the requirements

Feedback

We also have some comments for your work:

  1. Be consistent in variable declaration. For example, use const instead of var for books and setBooks https://github.com/R-Jim/df-frontend-2023/blob/d1ae7dbbf9ebcb2d6a12a2a86d5f07c3c7cea59e/assignment-2/src/page/main/Main.js#L33-L34

  2. Never directly mutate the state in React. Use methods that return a new array or object. That's important because direct mutation doesn't trigger a new render for "memo" components. https://github.com/R-Jim/df-frontend-2023/blob/d1ae7dbbf9ebcb2d6a12a2a86d5f07c3c7cea59e/assignment-2/src/page/main/Main.js#L74-L77

  3. The search is currently case-sensitive. Consider making it case-insensitive for a better use experience https://github.com/R-Jim/df-frontend-2023/blob/d1ae7dbbf9ebcb2d6a12a2a86d5f07c3c7cea59e/assignment-2/src/page/main/Main.js#L59

    const filteredBooks = books.filter(({ name }) => name.toLowerCase().includes(searchBookName.toLowerCase()));
  4. It'd be better to not have multiple components in one file. Consider dedicating one file per component https://github.com/R-Jim/df-frontend-2023/blob/d1ae7dbbf9ebcb2d6a12a2a86d5f07c3c7cea59e/assignment-2/src/component/form/Input.js#L1-L23

  5. I'd recommend moving JSX render code into the Pagination component. If there is only logic inside a component without HTML construction, consider writing a hook instead. https://github.com/R-Jim/df-frontend-2023/blob/d1ae7dbbf9ebcb2d6a12a2a86d5f07c3c7cea59e/assignment-2/src/component/table/Pagination.js#L25-L27