phuctinh542001 / df-frontend-2023-soranpt

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

Submission for assignment 2 #2

Open phuctinh542001 opened 1 year ago

phuctinh542001 commented 1 year ago

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

chinhld12 commented 1 year ago

Hi @phuctinh542001 , great work, your codebase looks clean 👍 !

Requirements

Final result: ✅ passed

Feedback

  1. Instead of utilizing an array of actions, which can make the code less clear in terms of their intended interactions, you can opt for an object with more meaningful and descriptive keys to enhance code readability and maintainability. It would be preferable to avoid using an anonymous function within the JSX callback function for improved application performance.

Example:

const updateItem = (item) => {
 // Return an inner function to callback on click
  return () => {
    // Do the update
  }
}

const handleActions = {
  updateItem: updateItem,
  deleteItem: ...,
 }

 // In the DataTable CTA, you can use the callback closure without employing an anonymous function.

  onClick={handleActions.updateItem(item)}
  onClick={handleActions.deleteItem(item)}

https://github.com/phuctinh542001/df-frontend-2023-soranpt/blob/ecfea1f86e9f24cb6f1adbd8124ab6aa727b4a30/assignment-2/src/pages/Books.js#L139-L140 https://github.com/phuctinh542001/df-frontend-2023-soranpt/blob/ecfea1f86e9f24cb6f1adbd8124ab6aa727b4a30/assignment-2/src/components/common/DataTable/DataTable.js#L51-L52

  1. This close button should be used with button element https://github.com/phuctinh542001/df-frontend-2023-soranpt/blob/ecfea1f86e9f24cb6f1adbd8124ab6aa727b4a30/assignment-2/src/components/common/Modal/Modal.js#L18-L20

  2. In your code, there are instances where else ... return blocks are used, but it may not be necessary. You can find more information on this coding practice and its benefits by reading this resource or conducting further research using the keywords "no else return." This principle encourages writing cleaner and more concise code by avoiding unnecessary else branches when they don't contribute to clarity or logic.

  3. This code block can be refactored to be simpler. https://github.com/phuctinh542001/df-frontend-2023-soranpt/blob/ecfea1f86e9f24cb6f1adbd8124ab6aa727b4a30/assignment-2/src/components/common/SwitchTheme/SwitchTheme.js#L12-L18

to:

setTheme((prev) => {
  const nextTheme = prev === 'light' ? 'dark': 'light';
  setToLocalStorage('theme', nextTheme); 
  return nextTheme;
})
  1. These code blocks can be refactored to a pure function method to get initial book data

https://github.com/phuctinh542001/df-frontend-2023-soranpt/blob/ecfea1f86e9f24cb6f1adbd8124ab6aa727b4a30/assignment-2/src/contexts/BooksContext.js#L45-L53

  1. These code blocks can be refactored to use in the initial state of useState in the ThemeContext

https://github.com/phuctinh542001/df-frontend-2023-soranpt/blob/ecfea1f86e9f24cb6f1adbd8124ab6aa727b4a30/assignment-2/src/pages/Books.js#L35-L51

// contexts/ThemeContext.js

  const [theme, setTheme] = useState(() => {
    // Use anonymous function to get theme from localstorage if not return default is 'light' theme

     const localTheme = getFromLocalStorage('theme') || 'light'; 
     return localTheme
  });