phuctinh542001 / df-frontend-2023-soranpt

https://df-frontend-2023-soranpt-assignment-1.vercel.app
0 stars 0 forks source link

Submission for assignment 3 #3

Open phuctinh542001 opened 1 year ago

phuctinh542001 commented 1 year ago

Link demo: https://df-frontend-2023-soranpt-assignment-3.vercel.app/ Link repo: https://github.com/phuctinh542001/df-frontend-2023-soranpt/tree/main/assignment-3

chinhld12 commented 1 year ago

Hi @phuctinh542001 , nice work!

Requirements

Final result: ✅ passed

Feedback

  1. Some places on your code still loosely implemented with Typescript image

https://github.com/phuctinh542001/df-frontend-2023-soranpt/blob/ecfea1f86e9f24cb6f1adbd8124ab6aa727b4a30/assignment-3/src/components/common/DataTable/DataTable.tsx#L12-L13 https://github.com/phuctinh542001/df-frontend-2023-soranpt/blob/ecfea1f86e9f24cb6f1adbd8124ab6aa727b4a30/assignment-3/src/components/common/Pagination/PaginationItem.tsx#L6-L7 https://github.com/phuctinh542001/df-frontend-2023-soranpt/blob/ecfea1f86e9f24cb6f1adbd8124ab6aa727b4a30/assignment-3/src/components/common/Pagination/PaginationItem.tsx#L6-L7 https://github.com/phuctinh542001/df-frontend-2023-soranpt/blob/ecfea1f86e9f24cb6f1adbd8124ab6aa727b4a30/assignment-3/src/contexts/BooksContext.tsx#L8-L9

For checking any place of usage with any can change this to true for Typescript checking all implicit places https://github.com/phuctinh542001/df-frontend-2023-soranpt/blob/ecfea1f86e9f24cb6f1adbd8124ab6aa727b4a30/assignment-3/tsconfig.json#L15-L16

-   const [currentData, setCurrentData] = useState<Book[] | []>([])
+   const [currentData, setCurrentData] = useState<Book[]>([])
  1. No need to use "`" for single variable styles, can consider usingclassnamespackage to join your variables inclassName` https://github.com/phuctinh542001/df-frontend-2023-soranpt/blob/ecfea1f86e9f24cb6f1adbd8124ab6aa727b4a30/assignment-3/src/components/common/Loading/Loading.tsx#L5-L6 ...

  2. No need to create a context for only the update dispatch reducer for the book state, using book context is enough https://github.com/phuctinh542001/df-frontend-2023-soranpt/blob/ecfea1f86e9f24cb6f1adbd8124ab6aa727b4a30/assignment-3/src/contexts/BooksContext.tsx#L6-L7

  3. Seem you did not migrate this JS file to Typescript https://github.com/phuctinh542001/df-frontend-2023-soranpt/blob/ecfea1f86e9f24cb6f1adbd8124ab6aa727b4a30/assignment-3/src/utils/localStorage.js#L1-L20