Submission for assignment 2

[tranduybau]

Link:

https://df-frontend-2023-vxcm.vercel.app/

Note for team:

Great job, thank you for the project!

[zlatanpham]

Hello @tranduybau, good work!

Requirements

• [x] Render book list with pagination
• [x] Create book
• [x] Delete book
• [ ] Search book by title
• [x] Standard use of React component & state
• [x] Light/dark mode

Final result: ✅ passed 70% of the requirements

Feedback

We also have some comments for your work:

1. If the search function has not been implemented, it is better to exclude it from the UI. https://github.com/tranduybau/df-frontend-2023/blob/659f11c1a4836180b92205326cf52ce13a0c9a16/assignment-2/src/App.jsx#L99
2. The onChangePage function currently seems to expect the event directly. However, in most pagination implementations, you'd likely want to pass the page number directly rather than the event object. https://github.com/tranduybau/df-frontend-2023/blob/659f11c1a4836180b92205326cf52ce13a0c9a16/assignment-2/src/App.jsx#L36
3. If there is only one page, it'd better to hide the pagination. Also, consider disabling the next and prev button if the current page is the last and first respectively.
4. Instead of doing theme === Theme.DARK && 'dark', it's clearer to use a ternary: theme === Theme.DARK ? 'dark' : ''. https://github.com/tranduybau/df-frontend-2023/blob/659f11c1a4836180b92205326cf52ce13a0c9a16/assignment-2/src/App.jsx#L93
5. You are using uncontrolled inputs in the form. Take the note and switch to controlled inputs and pass onChange event in the following assignments instead.
6. Use of switch(true) is unconventional. You might want to simplify this logic using conditional rendering or more straightforward if-else. https://github.com/tranduybau/df-frontend-2023/blob/659f11c1a4836180b92205326cf52ce13a0c9a16/assignment-2/src/components/Pagination/index.jsx#L8
7. Avoid magic numbers. For example, the number 5 is hard-coded in multiples places in the pagination. Consider using a constant like ITEMS_PER_PAGE. https://github.com/tranduybau/df-frontend-2023/blob/659f11c1a4836180b92205326cf52ce13a0c9a16/assignment-2/src/components/Pagination/index.jsx#L35
8. It's a best practice to avoid inner functions that return JSX. Try to create a new component to prevent the component from multiple time initiations https://github.com/tranduybau/df-frontend-2023/blob/659f11c1a4836180b92205326cf52ce13a0c9a16/assignment-2/src/components/Pagination/index.jsx#L7

[tranduybau]

Hi @zlatanpham , thank for your feedback. Here is some replying from me:

1. I updated function onSearch, also used useDebounce for better experience.
2. About the onChangePage function, I used the event object as a parameter because of don't want to create another function for format the parameter:

   const onChange = (event) => onChangePage(event.target.value)

   This is wasted of time, also increase the number of lines of code. The event object is a native one from HTML.

3. I updated that.
4. Just like topic 2, the code from me is shorter but keeping the same meaning. I prefer the short line of codes way.
5. I used the uncontrolled form because of not rendering the form again when user types something. It is also another way to working with form, without save any form's state with a variable. For me, the less code, the better performance :)
6. The switch (true) is not a unconventional, but it is a rule from SonarLint for using too many conditions in if-else state.
7. I will remember that. Thanks!
8. I updated

[tranduybau]

I had a face to face meeting with @zlatanpham, thank you for that. We can close the issue now ✅