Submission for assignment 5

Requirements

[x] All core functionalities present
[x] Edit form
[x] Login page
[x] Forms built by react-hook-form
[x] Validation by zod
[x] Bonus: Password Strength Meter

Result: ✅

Feedbacks

Overall good migration & form implementation 👍 Some nitpicks:

Users should be redirected into the dashboard after login:

There's something buggy here. First time I logged in, I wasn't redirected. It worked on the second time, though 🤔

You should consider moving these logic outside of the forms:

Current implementation assumes that by going back once, you're back to homepage. This might hold true for this project, but it's not a good practice to build a component with assumptions. It reduces the flexibility of the component.

By moving the logic out, we leave the responsibility to the parent components:

<DeleteBookDialog
  ...
  onDelete={() => {
    deleteBook();
    push('/');
  })
/>

Now you can use DeleteBookDialog everywhere & be able to control the navigation. Or you can keep the deleteBook call inside, and use an onAfterDelete props to handle what happens after:

<DeleteBookDialog
  ...
  onAfterDelete={() => {
    push('/');
  })
/>

Some files' locations are... questionable:

You can keep these in the same place as the pagination component. E.g:

src/components/common/pagination
  pagination.tsx
  pagination.css
  ...

This useMemo will run every renders, because handleLogin and handleLogout are not memoized:

Some points:

Eslint has a rule to prevent this (I don't remember exactly, but probably eslint-plugin-react-hooks provide the rule?)
You can wrap those methods in useCallback to memoize them
Normally we shouldn't have to memoize a context provider's value, you memoize the methods & stuff sufficiently 🤔

... (Please refer to my feedbacks for previous assignments)

nguyenhoaikhang37 / df-frontend-2023

Submission for assignment 5 #5

Requirements

Feedbacks