dnhn / df-fe-23

https://dffe23-dn-7.vercel.app
1 stars 0 forks source link

Assignment 2 #4

Open dnhn opened 11 months ago

dnhn commented 11 months ago

https://dffe23-dn-2.vercel.app

trankhacvy commented 11 months ago

Hello @dnhn, well done!

Requirements

Final result: ✅ passed

Feedback

We also have some comments for your work:

  1. The CSS code is related to the Button component, so please consider moving the import to the Button component file. https://github.com/dnhn/df-fe-23/blob/ee8f4c0f074ceb930d2407191534b6f825b25666/assignment-2/src/App.js#L10

  2. Instead of placing all files in the books folder, we can create separate folders for different components to improve organization. For example, we can have folders like book-table, book-table-toolbar, book-table-pagination, and so on. This will help keep our project more organized and easier to manage.

  3. Please consider splitting the state into multiple states to make the code clearer. https://github.com/dnhn/df-fe-23/blob/ee8f4c0f074ceb930d2407191534b6f825b25666/assignment-2/src/components/books/BooksContext.js#L37

  4. After new book added, it appear at the last page so user need to move to last page to see it. It's not a good UX. Consider move the newly book to the first page.

  5. This is unnecessary; we can use it directly in the useState function. https://github.com/dnhn/df-fe-23/blob/ee8f4c0f074ceb930d2407191534b6f825b25666/assignment-2/src/components/books/TableForm.js#L15C1-L15C1

  6. I provided feedback about the differences in your UI in assignment 1, but I noticed that you've repeated the same issue in this assignment. I'm not sure if this was intentional or not, but please ensure that you follow the requirements. While we welcome new ideas, it's important to communicate them with us beforehand.

dnhn commented 11 months ago
  1. I provided feedback about the differences in your UI in assignment 1, but I noticed that you've repeated the same issue in this assignment. I'm not sure if this was intentional or not, but please ensure that you follow the requirements. While we welcome new ideas, it's important to communicate them with us beforehand.

@trankhacvy thanks for your feedback. At the beginning, the dialogs weren’t mentioned in the instructions, so I assumed the image provided was just an example, and we could design the form as we wanted. I’ll update this in the next assignment.