VanLeDinh96 / df-frontend-2023

https://df-frontend-2023-woad.vercel.app
0 stars 0 forks source link

Submission for assignment 2 #2

Open VanLeDinh96 opened 11 months ago

VanLeDinh96 commented 11 months ago

Demo link

tienan92it commented 11 months ago

Hello @VanLeDinh96 , well done!. Below is some feedback for your assignment.

Requirements

Final result: ✅ passed 60% of requirements

Feedback

  1. Should use React Context to handle dark / light theme effect and store value in local storage to keep theme state after page refreshing. https://github.com/VanLeDinh96/df-frontend-2023/blob/3541110825a804bd0aa72456ff26367cfac33127/assignment-2/src/components/header/Header.js#L6-L11

  2. throw error inside catch is a block whole, catch will be useless in this case. Just log the error or handle error properly. https://github.com/VanLeDinh96/df-frontend-2023/blob/3541110825a804bd0aa72456ff26367cfac33127/assignment-2/src/components/dashboard/Dashboard.js#L25-L28 https://github.com/VanLeDinh96/df-frontend-2023/blob/3541110825a804bd0aa72456ff26367cfac33127/assignment-2/src/components/dashboard/Dashboard.js#L59-L62

  3. Error occurs only when parse json from a string, for parse json to a string, it doesn't have any error. So don't need to wrap JSON.stringify inside a try-catch. https://github.com/VanLeDinh96/df-frontend-2023/blob/3541110825a804bd0aa72456ff26367cfac33127/assignment-2/src/components/dashboard/Dashboard.js#L57-L62

  4. Mange books by unique id to avoid bug from deleting books with same name. https://github.com/VanLeDinh96/df-frontend-2023/blob/3541110825a804bd0aa72456ff26367cfac33127/assignment-2/src/components/dashboard/Dashboard.js#L74

  5. Wrap updateData inside a useEffect to track book changes and don't need to call updateData every action handlers. https://github.com/VanLeDinh96/df-frontend-2023/blob/3541110825a804bd0aa72456ff26367cfac33127/assignment-2/src/components/dashboard/Dashboard.js#L56-L65

useEffect(() => {
    localStorage.setItem(dataKey, JSON.stringify(data));
    setPage(0); // Handle pagination to avoid reseting page to zero after data changes
}, [data])

const addItem = newItem => {
    const updatedData = [...data, newItem];
    setData(data);
    setIsOpen(false);
}

const deleteItem = item => {
    const idx = data.findIndex(i => i.name === item.name);
    if (idx === -1) {
        return;
    }
    const updatedData = [...data];
    updatedData.splice(idx, 1);
    setData(data);
    setIsOpen(false);
}
  1. Again, mange books by unique id to avoid bug from duplicated keys. Ref: Duplicated keys bug https://github.com/VanLeDinh96/df-frontend-2023/blob/3541110825a804bd0aa72456ff26367cfac33127/assignment-2/src/components/dashboard/Dashboard.js#L99-L107