Skip to content
This repository was archived by the owner on Dec 18, 2024. It is now read-only.

NW6 | Orlando_Morales | JS2-Module-Reading-list | Week3 #199

Open
OrlandoMoralesKuan wants to merge 2 commits into
base: main
Choose a base branch
from
Open

NW6 | Orlando_Morales | JS2-Module-Reading-list | Week3 #199

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
5e40123
Changing the title to reading-list HTML
OrlandoMoralesKuan Jan 18, 2024
de85239
reading-list javascript is finished
OrlandoMoralesKuan Jan 18, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion week-3/reading-list/index.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
<meta charset="utf-8" />
<meta name="viewport" content="width=device-width, initial-scale=1.0" />
<link rel="stylesheet" href="style.css" />
Copy link
Contributor

@pseudopilot pseudopilot May 15, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see this line but don't see any .css file here. Was it the part of the task to style the page or not? I'm not really sure but I see one of the requirements:

The page is visually appealing

Maybe you just missed to add it to commit?

<title>Title here</title>
<title>Reading list app</title>
</head>
<body>
<div id="content">
Expand Down
35 changes: 35 additions & 0 deletions week-3/reading-list/script.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,3 +21,38 @@ const books = [
},
];

function renderBooks(book) {
Copy link
Contributor

@pseudopilot pseudopilot May 15, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As this function renders only one book I would suggest renaming it to renderBook to avoid confusion.

const bookElement = document.createElement("li");

bookElement.style.backgroundColor = book.alreadyRead ? "green" : "red";

const titleElement = document.createElement("h3");
titleElement.textContent = book.title;
bookElement.appendChild(titleElement);

const authorElement = document.createElement("p");
authorElement.textContent = book.author;
bookElement.appendChild(authorElement);

const imageElement = document.createElement("img");
imageElement.src = book.bookCoverImage;
bookElement.appendChild(imageElement);

return bookElement;
}

function readingList() {
Copy link
Contributor

@pseudopilot pseudopilot May 15, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One more naming recommendation. Functions should describe action. This allow us to distinguish functions and variables just even by names. I would suggest rename it to buildReadingList or renderReadingList.

const readingList = document.getElementById("reading-list");

readingList.innerHTML = "";

books.forEach((book) => {
const bookElement = renderBooks(book);

readingList.appendChild(bookElement);
});
}

window.onload = function () {
readingList(books);
};