victhangnguyen / df-frontend-2023

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

Submission for assignment 1 #4

Open victhangnguyen opened 1 year ago

victhangnguyen commented 1 year ago

// Demo link: https://df-frontend-2023-bay.vercel.app/pages/createTopic.html // Any notes for reviewers: Dự án có làm thêm phần Bonus: Storage, và phần Setting em sẽ cập nhật sau bên React. UXUI: Responsive, Điền thông tin Book, Tạo Sách, Tạo Topic, và Search theo Tên.

tienan92it commented 1 year ago

Hello @victhangnguyen , great work!. Below is some feedback for your assignment.

Requirements

Final result: ✅ passed 80% of requirements

Feedback

  1. Should use <button /> instead of <span /> for clickable elements. https://github.com/victhangnguyen/df-frontend-2023/blob/bbd281dda6e386f0568a88aa4c138418283dcdbc/assignment-1/index.html#L124

  2. Seem like you are using many !important keywords in css. Keep in mind it's really a bad practice, using !important only if you want to override styles of the css libraries. https://github.com/victhangnguyen/df-frontend-2023/blob/bbd281dda6e386f0568a88aa4c138418283dcdbc/assignment-1/style.css#L683-L690 https://github.com/victhangnguyen/df-frontend-2023/blob/bbd281dda6e386f0568a88aa4c138418283dcdbc/assignment-1/style.css#L713 https://github.com/victhangnguyen/df-frontend-2023/blob/bbd281dda6e386f0568a88aa4c138418283dcdbc/assignment-1/style.css#L821-L868

  3. Avoid using innerHTML to manipulate element, it's a bad practice. Consider alternative solutions like textContent, appendChild, setHTML, sanitizer... instead. Research keyword Why is using innerHTML in HTML is bad idea and cross-site scripting vulnerabilities. https://github.com/victhangnguyen/df-frontend-2023/blob/bbd281dda6e386f0568a88aa4c138418283dcdbc/assignment-1/scripts/tableBook.js#L20-L30

  4. It's better to wrap JSON.parse inside a try-catch to prevent unexpected error from parsing data https://github.com/victhangnguyen/df-frontend-2023/blob/bbd281dda6e386f0568a88aa4c138418283dcdbc/assignment-1/scripts/storage.js#L12

  5. Searching book list doesn't use latest books. It's because your bookDocArr in tableBook.js isn't updated after a book is added or deleted. Add book listener and delete book listener work well because your condition always return true in cases of adding and deleting and get latest for rendering bookDocs = Book._getBooks(); but searching. Keep in mind that even if you update bookDocArr inside a listener, other listeners still remain the old value at the time you init these listeners. https://github.com/victhangnguyen/df-frontend-2023/blob/bbd281dda6e386f0568a88aa4c138418283dcdbc/assignment-1/scripts/tableBook.js#L11 https://github.com/victhangnguyen/df-frontend-2023/blob/bbd281dda6e386f0568a88aa4c138418283dcdbc/assignment-1/scripts/tableBook.js#L35-L37 https://github.com/victhangnguyen/df-frontend-2023/blob/bbd281dda6e386f0568a88aa4c138418283dcdbc/assignment-1/scripts/tableBook.js#L90-L97 https://github.com/victhangnguyen/df-frontend-2023/blob/bbd281dda6e386f0568a88aa4c138418283dcdbc/assignment-1/scripts/tableBook.js#L99-L111 https://github.com/victhangnguyen/df-frontend-2023/blob/bbd281dda6e386f0568a88aa4c138418283dcdbc/assignment-1/scripts/tableBook.js#L53-L71