mirageruler / df-frontend-2023

learning FE stuffs from dwarves
https://df-frontend-2023-rust.vercel.app
0 stars 1 forks source link

Submission for assignment 1 #1

Open mirageruler opened 1 year ago

mirageruler commented 1 year ago
tienan92it commented 1 year ago

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

Requirements

Final result: ✅ passed 60% of requirements

Feedback

  1. Recommend to use rem em for font-size instead of px https://github.com/mirageruler/df-frontend-2023/blob/2c1354360991f6dc64e18c26566e26af5509a59a/assignment-1/style.css#L231 https://github.com/mirageruler/df-frontend-2023/blob/2c1354360991f6dc64e18c26566e26af5509a59a/assignment-1/style.css#L222

  2. Inconsistent tag id, mixing between camel cases and snake cases. https://github.com/mirageruler/df-frontend-2023/blob/2c1354360991f6dc64e18c26566e26af5509a59a/assignment-1/index.html#L13 https://github.com/mirageruler/df-frontend-2023/blob/2c1354360991f6dc64e18c26566e26af5509a59a/assignment-1/index.html#L34

  3. Using inline events like onclick, oninput, onchange... in HTML is a bad practice. For your HTML, it's no problem because each event is used for a single element. But when you have to reuse them for multiple elements, your code will be hard to read and maintain. Consider addEventListener instead. Research Why is using inline events in HTML a bad practice for further information. https://github.com/mirageruler/df-frontend-2023/blob/2c1354360991f6dc64e18c26566e26af5509a59a/assignment-1/index.html#L51 https://github.com/mirageruler/df-frontend-2023/blob/2c1354360991f6dc64e18c26566e26af5509a59a/assignment-1/index.html#L71 https://github.com/mirageruler/df-frontend-2023/blob/2c1354360991f6dc64e18c26566e26af5509a59a/assignment-1/index.html#L47

  4. Avoid using innerHTML to manipulate element, it's a bad practice. Consider textContent, appendChild, setHTML instead. Research keyword Why is using innerHTML in HTML is bad idea and cross-site scripting vulnerabilities. https://github.com/mirageruler/df-frontend-2023/blob/2c1354360991f6dc64e18c26566e26af5509a59a/assignment-1/script.js#L11-L17 https://github.com/mirageruler/df-frontend-2023/blob/2c1354360991f6dc64e18c26566e26af5509a59a/assignment-1/script.js#L22-L28

  5. Make modals overlay fullscreen to prevent the unexpected interactions from background elements. This leads to a bug: when User clicks on delete button and while deleting modals is opening, continue click on delete button of other books. Then after delete book confirming, all books that relate to clicked delete buttons will be removed.

    Screenshot 2023-09-25 at 17 50 54