London|25-ITP-September|Alexandru Pocovnicu|Sprint 3|Reading list #852
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…ased on read status
alexandru-pocovnicu
added
the
Needs Review
jennethydyrova
added
Review in progress
jennethydyrova
suggested changes
Nov 22, 2025
Contributor
jennethydyrova
left a comment
jennethydyrova
left a comment
There was a problem hiding this comment.
Hi @alexandru-pocovnicu! Very good work on this exercise. There are few comments I left that will make you code cleaner.
| ]; | ||
|
|
||
| function readingList() { | ||
| const unorderedList = document.querySelector("#reading-list"); |
Contributor
There was a problem hiding this comment.
Is this a good name for the variable? Another thing, what if you code cannot find this dom element? How can you guard your function against it?
Sprint-3/reading-list/script.js
Outdated
Comment on lines
29
to
31
| const title = book.title; | ||
| const author = book.author; | ||
| newImage.src = book.bookCoverImage; |
Contributor
There was a problem hiding this comment.
This is okay but can you check how to destructure an object to get field values? This will make you code cleaner.
Author
There was a problem hiding this comment.
thank you, this was interesting
Sprint-3/reading-list/script.js
Outdated
Comment on lines
34
to
35
| newList.appendChild(paragraph); | ||
| newList.appendChild(newImage); |
Contributor
There was a problem hiding this comment.
Can you think of another way to append both paragraph and newImage in one line?
Sprint-3/reading-list/style.css
Outdated
Comment on lines
163
to
164
| #reading-list img { | ||
| size: ; |
jennethydyrova
added
Reviewed
Author
|
Thank You. |
alexandru-pocovnicu
added
Needs Review
jennethydyrova
approved these changes
Nov 23, 2025
| for (const book of books) { | ||
| const newList = document.createElement("li"); | ||
| const newImage = document.createElement("img"); | ||
| const { title, author, bookCoverImage } = book; |
Contributor
There was a problem hiding this comment.
You could do something more interesting. Just fyi
for (const {title, author, bookCoverImage, alreadyRead} of books) {}
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Learners, PR Template
Self checklist
Changelist
In this PR I have looped through an array, created DOM elements and appended them to the DOM