search

R-Jim / df-frontend-2023

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

Submission for assignment 3 #11

Closed R-Jim closed 11 months ago

R-Jim commented 12 months ago

Link https://df-frontend-2023-pearl.vercel.app/

zlatanpham commented 11 months ago

Hello @R-Jim, good work!

Requirements

Final result: ✅ passed 70% of the requirements

Feedback

We also have some comments for your work:

  1. You can utilize the (books || []).map(...) syntax to avoid using additional lines of code for new assignments and branching. https://github.com/R-Jim/df-frontend-2023/blob/7c4a63a252979a1d7c375a5cfa19aef0ed2de2fe/assignment-3/src/main/Main.tsx#L34-L57
  2. It's conventional to return null for an empty render https://github.com/R-Jim/df-frontend-2023/blob/7c4a63a252979a1d7c375a5cfa19aef0ed2de2fe/assignment-3/src/component/table/Table.tsx#L68-L70
  3. As per feedback received in previous assignments, it is recommended to improve accessibility support by using the labebl element along with appropriate attributes that link to the relevant input. https://github.com/R-Jim/df-frontend-2023/blob/7c4a63a252979a1d7c375a5cfa19aef0ed2de2fe/assignment-3/src/component/form/InputText.tsx#L23-L24
  4. Should name props instead of params as it's a convention in writing React code https://github.com/R-Jim/df-frontend-2023/blob/7c4a63a252979a1d7c375a5cfa19aef0ed2de2fe/assignment-3/src/component/table/Table.tsx#L20
  5. Since all of the props are external to the component, it would be more optimal to manage the logic externally as well. https://github.com/R-Jim/df-frontend-2023/blob/7c4a63a252979a1d7c375a5cfa19aef0ed2de2fe/assignment-3/src/component/table/Table.tsx#L47-L52
  6. I have observed that several components are constructed by means of a mere tag and the rendering of its child elements. If the sole intention of the component is to act as a wrapper, it is better to utilize the HTML version rather than creating a React component for it. https://github.com/R-Jim/df-frontend-2023/blob/main/assignment-3/src/component/bar/Action.tsx#L7-L13