ITP_LONDON | PRISCILLA_EMEBO |Module-Data-Groups | Reading list app #509
Conversation
jenny-alexander
added
Review in progress
This comment was marked as duplicate.
This comment was marked as duplicate.
Sorry, something went wrong.
cjyuan
removed
the
Review in progress
|
@jenny-alexander Sorry. I didn't play by the rule and changed the label. I didn't thought anyone would still be giving review at this stage. I just hid my review. |
cjyuan
added
the
Review in progress
No worries @cjyuan, glad for the help as I was trying to do a few reviews before the work day starts. 🙂 |
| books.forEach((book) => { | ||
| const listItem = document.createElement("li"); | ||
| listItem.className = "book-item"; | ||
| listItem.style.backgroundColor = book.alreadyRead ? "#c8f7c5" : "#f7c5c5"; |
There was a problem hiding this comment.
I like the way you used a ternary operator here to determine the background color. 👍
However, it's generally preferred to keep CSS styling within your CSS styling files. This will make it easier to maintain styling as your application grows. Can you make this change?
https://www.w3schools.com/html/html_classes.asp -> look at the Multiple Classes section
jenny-alexander
added
Reviewed
| const image = document.createElement("img"); | ||
| image.src = book.bookCoverImage; | ||
| image.alt = `${book.title} cover`; | ||
| image.style.width = "100px"; |
There was a problem hiding this comment.
Here is another place where using a class name (for styling) is preferred to styling the image within the javascript code.
|
Thank you for the review. I'll make the necessary changes |
Learners, PR Template
Self checklist
Changelist
Briefly explain your PR.
Questions
Ask any questions you have for your reviewer.