Submission for assignment 4

[nguyenhoaikhang37]

// Demo link https://df-frontend-2023-ass4-eight.vercel.app

// Any notes for reviewers If the code lacks clarify or if there are any issue with the application, please don't hesitate to provide feedback. I'm open to suggestions and eager to improve. Thanks a lot <3

[ngolapnguyen]

Nice work 👍

Requirements

• [x] All core functionalities present
• [x] Proper Tailwind implementation
• [x] Book detail page
• [x] 404 page
• [x] Redirect after deletion
• [x] Bonus: Tailwind dark/light mode
• [x] Bonus: URL state sync

Result: ✅

Feedbacks

1. Conventionally we have been using UpperCamelCase as the format for component files. Why did you switch to snake-case? 🤔

2. TextField and TextFieldWithLabel seem... kind of random. 2 questions: a. Why don't you render label conditionally inside TextField? Why is there a need for a dedicated component? b. What do you think about building a generic FormField wrapper component that handles the label (and error message) part? So we can use it like this:

   <FieldWrapper label="Text">
     <TextField />
   </FieldWrapper>  
   <FieldWrapper label="Text">
     <SelectField />
   </FieldWrapper>  

3. Bug in the query params syncing:

4. I can't add book:

5. The below logic can only get books from BOOK_LIST. It will not work for books we manually added.

6. For handling query params, you can also look for some 3rd-party packages like query-string. It'll help us manage the query params better.

Current implementation is fine for current use-case, but it won't hold for more complicated flows. The createQueryString isn't helpful enough, either. You have to re-implement most of the logic in the useEffect for searchValue. Anyhow, the query syncing logic should definitely be optimized 🤔

7. ...

[ngolapnguyen]

I'll give you a 🔥 for all the experimental stuff you've done in this assignment. Good job 💯

[nguyenhoaikhang37]

Hello @ngolapnguyen, thank you for your quality review <3

1. I will respond to a few things about the above review
2. I have used two ways to format component file names. In this project, I just tried using snake-case writing and I found it easier to read ...
3. I was confused when using 2 components with the same purpose. I will correct it your way, thanks... Oh, I think it will be helpful for me in my next project, thanks

And so

Current implementation is fine for current use-case, but it won't hold for more complicated flows. The createQueryString isn't helpful enough, either. You have to re-implement most of the logic in the useEffect for searchValue. Anyhow, the query syncing logic should definitely be optimized 🤔

Can you give me some tips on working with query params, tksss

[ngolapnguyen]

@nguyenhoaikhang37 You can start by using something like query-string to simplify parsing objects into query params. Or write your own utility method. Either way is fine.

Another thing is just how you can have a generic approach to syncing query params, so that we can minimize duplication. Something like this should work:

const [filters, setFilters] = useState({
  query: '',
  category: '',
  author: ''
})

const [pagination, setPagination] = useState({
  page: 1,
  limit: 5
})

// Update query params when filters and pagination changes
useEffect(() => {
  replace(pathname, qs.stringify({ ...filters, ...pagination });
}, [filters, pagination])

You don't have to follow my approach. The important thing is to figure out a way that can reduce your effort.