LONDON | 25-ITP-MAY | TEWODROS BEKERE | SPRINT-3 | Reading-List-App #732
Conversation
…ying books and also improve CSS.
iteddy16
added
📅 Sprint 3
iteddy16
changed the title
There was a problem hiding this comment.
Have you run the test? All tests fails at the moment. Please refactor the code accordingly!
| </head> | ||
| <body> | ||
| <div id="content"> | ||
| <ul id="reading-list"></ul> |
There was a problem hiding this comment.
Hint: You will need to use this id reading-list to match the test cases.
There was a problem hiding this comment.
I can't see the changed files and new commits!
|
|
||
| const bookItem = document.createElement("div"); | ||
| bookItem.className = "book"; | ||
| bookItem.style.backgroundColor = book.alreadyRead ? "green" : "red"; |
There was a problem hiding this comment.
This works fine but it's best to keep styles in the css file rather then modifying in the DOM. I would recommend using class to change the colour. Can you refactor the code accordingly?
| bookItem.style.backgroundColor = book.alreadyRead ? "green" : "red"; | ||
|
|
||
| const bookInfo = document.createElement("div"); | ||
| bookInfo.innerHTML = `<strong>${book.title}</strong> by <strong>${book.author}</strong>`; |
There was a problem hiding this comment.
innerHTML has a potential security issue. It's better to avoid using it. FYI, find more in this link. Can you refactor the code with an alternative property?
day-lee
added
Reviewed
|
Good job! I've left a few comment. Please let me know if you need any help. |
…ying books and also improve CSS.
Learners, PR Template
Self checklist
Changelist
Briefly explain your PR.
Questions
Ask any questions you have for your reviewer.