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

NW-6 | AREEB-SATTAR | JS2| [TECH ED] Reading List| WEEK-3 #221

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

NW-6 | AREEB-SATTAR | JS2| [TECH ED] Reading List| WEEK-3 #221

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
08f06b2
Updated HTML
areebsattar Jan 24, 2024
0ce9cfa
Written function and update css
areebsattar Jan 24, 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
32 changes: 18 additions & 14 deletions week-3/reading-list/index.html
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,15 +1,19 @@
<!DOCTYPE >
<!DOCTYPE>
<html lang="en_US">
<head>
<meta charset="utf-8" />
<meta name="viewport" content="width=device-width, initial-scale=1.0" />
<link rel="stylesheet" href="style.css" />
<title>Title here</title>
</head>
<body>
<div id="content">
<ul id="reading-list"></ul>
</div>
<script src="script.js"></script>
</body>
</html>

<head>
<meta charset="utf-8" />
<meta name="viewport" content="width=device-width, initial-scale=1.0" />
<link rel="stylesheet" href="style.css" />
<script defer src="script.js"></script>
Copy link

Choose a reason for hiding this comment

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

Nice use of the defer attribute!

<title>Reading List</title>
</head>

<body>
<div id="content">
<ul id="reading-list"></ul>
</div>

</body>

</html>
21 changes: 21 additions & 0 deletions week-3/reading-list/script.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,23 @@
function readingList(books) {
const readingList = document.getElementById("reading-list");

for (const item of books) {
const list = document.createElement("li");

list.setAttribute("class", item.alreadyRead ? "teal" : "red");
Copy link

Choose a reason for hiding this comment

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

I'm going to be really picky, but hopefully this is a useful note!

As much as possible, it's good to keep your styling separate from logic. You could use class names like "read" and "unread", then only use CSS to decide what the colour should be.

This is really useful for modifying your site in the future, as all colour choices are together in the CSS. There is also a scenario where your site might have different themes, for example, a light and dark mode. The colours for read and unread books might not always be the same in each mode.

Copy link
Author

Choose a reason for hiding this comment

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

That makes sense, I will keep that in mind, Thank you


const p = document.createElement("p");
p.innerText = `${item.title} by ${item.author}`;

readingList.appendChild(list);
list.appendChild(p);

const image = document.createElement("img");
image.src = item.bookCoverImage;
list.appendChild(image);
}
}

// for the tests, do not modify this array of books
const books = [
{
Expand All @@ -21,3 +41,4 @@ const books = [
},
];

readingList(books);
6 changes: 5 additions & 1 deletion week-3/reading-list/style.css
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,11 @@ body {
}

.red {
background-color: red;
background-color: orangered;
}

.teal {
background-color: teal;
}

.addArticle {
Expand Down