Skip to content

Sheffield | May-2025 | Waleed-Yahay Salih-Taha | Sprint 2 | Book Library #286

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
Bluejay600 wants to merge 18 commits into from
Closed

Sheffield | May-2025 | Waleed-Yahay Salih-Taha | Sprint 2 | Book Library #286

Show file tree
Hide file tree
Changes from 10 commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
c830178
bugs are fixed now
aidazhukova Mar 28, 2025
c3b7f29
fixed the if condition's syntax of the missing bracket.
Bluejay600 Aug 9, 2025
e99d6aa
added a title to the web page and a languagge element to the html ope…
Bluejay600 Aug 9, 2025
ff68704
Merge branch 'book-library' of https://github.com/Bluejay600/Module-D…
Bluejay600 Aug 9, 2025
3869805
updated html code
Bluejay600 Aug 16, 2025
300dc39
updated css.
Bluejay600 Aug 16, 2025
6b2c9a9
updated the script file.
Bluejay600 Aug 16, 2025
c21b787
updated and validated html file.
Bluejay600 Aug 17, 2025
5fd2034
updated the input values not to be empty spaces.
Bluejay600 Aug 17, 2025
808cd28
updated css file.
Bluejay600 Aug 17, 2025
6473c1f
Cache DOM elements with clear suffixes
Bluejay600 Aug 18, 2025
60240cb
Grouped all setup codes in window.onload
Bluejay600 Aug 18, 2025
196a070
improved function to addbook
Bluejay600 Aug 18, 2025
2e0943e
Fix delete alert order (show confirmation only after deletion succeed…
Bluejay600 Aug 18, 2025
555c393
updated html code.
Bluejay600 Aug 19, 2025
410104f
fixes js code
Bluejay600 Aug 19, 2025
b77a783
Improve maintainability / readability
Bluejay600 Aug 19, 2025
f032d63
Edited input validation to prevent decimal values (e.g., 12.5 pages).…
Bluejay600 Aug 19, 2025
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
77 changes: 24 additions & 53 deletions debugging/book-library/index.html
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,20 +1,14 @@
<!DOCTYPE html>
<html>
<html lang="en">
<head>
<title> </title>
<meta
charset="utf-8"
name="viewport"
content="width=device-width, initial-scale=1.0"
/>
<title>Book library</title>
<meta charset="utf-8"/>
<meta name="viewport" content="width=device-width, initial-scale=1.0"/>
<script src="https://ajax.googleapis.com/ajax/libs/jquery/3.4.1/jquery.min.js"></script>
<script src="https://cdnjs.cloudflare.com/ajax/libs/popper.js/1.16.0/umd/popper.min.js"></script>
<script src="https://maxcdn.bootstrapcdn.com/bootstrap/4.4.1/js/bootstrap.min.js"></script>
<link
rel="stylesheet"
href="https://maxcdn.bootstrapcdn.com/bootstrap/4.4.1/css/bootstrap.min.css"
/>
<link rel="stylesheet" type="text/css" href="style.css" />
<link rel="stylesheet" href="https://maxcdn.bootstrapcdn.com/bootstrap/4.4.1/css/bootstrap.min.css"/>
<link rel="stylesheet" type="text/css" href="style.css"/>
</head>

<body>
Expand All @@ -23,51 +17,28 @@ <h1>Library</h1>
<p>Add books to your virtual library</p>
</div>

<button data-toggle="collapse" data-target="#demo" class="btn btn-info">
<button data-toggle="collapse" data-target="#demo" aria-controls="demo" class="btn btn-info">
Add new book
</button>

<div id="demo" class="collapse">
<div class="form-group">
<form id="bookForm" class="form-group">
<label for="title">Title:</label>
<input
type="title"
class="form-control"
id="title"
name="title"
required
/>
<label for="author">Author: </label>
<input
type="author"
class="form-control"
id="author"
name="author"
required
/>
<input type="text" class="form-control" id="title" name="title" required />

<label for="author">Author:</label>
<input type="text" class="form-control" id="author" name="author" required />

<label for="pages">Pages:</label>
<input
type="number"
class="form-control"
id="pages"
name="pages"
required
/>
<label class="form-check-label">
<input
type="checkbox"
class="form-check-input"
id="check"
value=""
/>Read
</label>
<input
type="submit"
value="Submit"
class="btn btn-primary"
onclick="submit();"
/>
</div>
<input type="number" class="form-control" id="pages" name="pages" required />

<div class="form-check">
<input type="checkbox" class="form-check-input" id="check" />
<label class="form-check-label" for="check">Read</label>
</div>

<button type="button" class="btn btn-primary mt-2" id="submitBtn">Submit</button>
</form>
</div>

<table class="table" id="display">
Expand All @@ -77,7 +48,7 @@ <h1>Library</h1>
<th>Author</th>
<th>Number of Pages</th>
<th>Read</th>
<th></th>
<th>Action</th>
</tr>
</thead>
<tbody>
Expand All @@ -93,4 +64,4 @@ <h1>Library</h1>

<script src="script.js"></script>
</body>
</html>
</html>
99 changes: 37 additions & 62 deletions debugging/book-library/script.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,19 +1,14 @@
let myLibrary = [];

window.addEventListener("load", function (e) {
window.addEventListener("load", function () {
populateStorage();
render();
});

function populateStorage() {
if (myLibrary.length == 0) {
let book1 = new Book("Robison Crusoe", "Daniel Defoe", "252", true);
let book2 = new Book(
"The Old Man and the Sea",
"Ernest Hemingway",
"127",
true
);
let book2 = new Book("The Old Man and the Sea","Ernest Hemingway","127", true );
myLibrary.push(book1);
myLibrary.push(book2);
render();
Expand All @@ -25,79 +20,59 @@ const author = document.getElementById("author");
const pages = document.getElementById("pages");
const check = document.getElementById("check");
Copy link
Copy Markdown
Contributor

@cjyuan cjyuan Aug 17, 2025

Choose a reason for hiding this comment

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

Using descriptive and consistent suffixes (like El, Input, Btn, Form, etc.) for variables that store DOM elements can improve code readability and maintainability.

Copy link
Copy Markdown
Author

@Bluejay600 Bluejay600 Aug 19, 2025

Choose a reason for hiding this comment

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

I have Consistented suffixes for DOM elements (El, Btn, Form, etc.)


document.getElementById("submitBtn").addEventListener("click", addBook);
Copy link
Copy Markdown
Contributor

@cjyuan cjyuan Aug 17, 2025

Choose a reason for hiding this comment

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

It is better to group all the code that's to be executed once on page load together in the window.onload callback function (around line 4)

Copy link
Copy Markdown
Author

@Bluejay600 Bluejay600 Aug 19, 2025

Choose a reason for hiding this comment

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

now I Grouped all setup code in window.onload


//check the right input from forms and if its ok -> add the new book (object in array)
//via Book function and start render function
function submit() {
if (
title.value == null ||
title.value == "" ||
pages.value == null ||
pages.value == ""
) {
alert("Please fill all fields!");
return false;
} else {
let book = new Book(title.value, title.value, pages.value, check.checked);
library.push(book);
function addBook() {
if (!title.value.trim() || !author.value.trim() || !pages.value.trim()) {
alert("Please fill all fields with valide values (no empty spaces)!");
return;
}
let book = new Book(title.value.trim(), author.value.trim(), pages.value.trim(), check.checked);
myLibrary.push(book);
Copy link
Copy Markdown
Contributor

@cjyuan cjyuan Aug 17, 2025

Choose a reason for hiding this comment

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

  • For better performance (reduce number of function calls) and reducing the chance of using raw input accidently, we could stored the pre-processed/sanitized/normalized input in some variables first, and reference the variables in other part of the function.

  • pages.value.trim() is a string, without proper validation, sanitization, and pre-processing, the following page numbers are possible:

image

Copy link
Copy Markdown
Author

@Bluejay600 Bluejay600 Aug 19, 2025

Choose a reason for hiding this comment

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

Pre-processed and sanitizes the input before using it multiple times

render();
document.getElementById("bookForm").reset();
}
Comment on lines +50 to 60
Copy link
Copy Markdown
Contributor

@cjyuan cjyuan Aug 19, 2025

Choose a reason for hiding this comment

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

Why not use the VSCode extension prettier to auto format/indent the code?

}

function Book(title, author, pages, check) {
function Book(title, author, pages, read) {
this.title = title;
this.author = author;
this.pages = pages;
this.check = check;
this.read = read;
}

function render() {
let table = document.getElementById("display");
let rowsNumber = table.rows.length;
//delete old table
for (let n = rowsNumber - 1; n > 0; n-- {
table.deleteRow(n);
}
//insert updated row and cells
let length = myLibrary.length;
for (let i = 0; i < length; i++) {
let row = table.insertRow(1);
let titleCell = row.insertCell(0);
let authorCell = row.insertCell(1);
let pagesCell = row.insertCell(2);
let wasReadCell = row.insertCell(3);
let deleteCell = row.insertCell(4);
titleCell.innerHTML = myLibrary[i].title;
authorCell.innerHTML = myLibrary[i].author;
pagesCell.innerHTML = myLibrary[i].pages;
let tbody = document.querySelector("#display tbody");
tbody.innerHTML = "";

myLibrary.forEach((book, i) => {
let row = tbody.insertRow();

//add and wait for action for read/unread button
row.insertCell(0).textContent = book.title;
row.insertCell(1).textContent = book.author;
row.insertCell(2).textContent = book.pages;

// read/unread button
let wasReadCell = row.insertCell(3);
let changeBut = document.createElement("button");
changeBut.id = i;
changeBut.className = "btn btn-success";
wasReadCell.appendChild(changeBut);
let readStatus = "";
if (myLibrary[i].check == false) {
readStatus = "Yes";
} else {
readStatus = "No";
}
changeBut.innerText = readStatus;

changeBut.addEventListener("click", function () {
myLibrary[i].check = !myLibrary[i].check;
changeBut.textContent = book.read ? "Yes" : "No";
changeBut.addEventListener("click", () => {
book.read = !book.read;
render();
});
wasReadCell.appendChild(changeBut);

//add delete button to every row and render again
// delete button
let deleteCell = row.insertCell(4);
let delButton = document.createElement("button");
delBut.id = i + 5;
deleteCell.appendChild(delBut);
delBut.className = "btn btn-warning";
delBut.innerHTML = "Delete";
delBut.addEventListener("clicks", function () {
alert(`You've deleted title: ${myLibrary[i].title}`);
delButton.className = "delete-btn";
delButton.textContent = "Delete";
delButton.addEventListener("click", () => {
alert(`You've deleted title: ${book.title}`);
myLibrary.splice(i, 1);
render();
Copy link
Copy Markdown
Contributor

@cjyuan cjyuan Aug 17, 2025

Choose a reason for hiding this comment

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

The alert message is shown before the book is actually deleted; the deletion only occurs after the alert dialog is dismissed. This introduces a risk that the operation may not complete (e.g., if the user closes the browser before dismissing the alert).

In general, it’s better to display a confirmation message only after the associated operation has successfully completed.

Copy link
Copy Markdown
Author

@Bluejay600 Bluejay600 Aug 19, 2025

Choose a reason for hiding this comment

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

Fixed the delete alert order (show confirmation only after deletion succeeds, not before)

});
}
deleteCell.appendChild(delButton);
});
}
119 changes: 107 additions & 12 deletions debugging/book-library/style.css
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,19 +1,114 @@
.form-group {
width: 400px;
height: 300px;
align-self: left;
padding-left: 20px;
/* General page styling */
body {
background: #f7f9fc;
font-family: "Segoe UI", Tahoma, Geneva, Verdana, sans-serif;
color: #333;
margin: 0;
padding: 0;
}

.btn {
display: block;
/* Jumbotron header */
.jumbotron {
background: linear-gradient(135deg, #4facfe, #00f2fe);
color: white;
padding: 3rem 2rem;
border-radius: 0 0 20px 20px;
box-shadow: 0 4px 10px rgba(0, 0, 0, 0.15);
}

.form-check-label {
padding-left: 20px;
margin: 5px 0px 5px 0px;
/* Add Book button */
button.btn-info {
margin: 1rem 0;
background: #03cceb;
border: none;
width: 15%;
height: 30px;
transition: background 0.3s ease, transform 0.2s ease;
}

button.btn-info {
margin: 20px;
button.btn-info:hover {
background: #4facfe;
transform: translateY(-2px);
}

/* Form styling */
#demo {
background: #fff;
padding: 1.5rem;
margin: 1rem 0;
border-radius: 12px;
box-shadow: 0 2px 8px rgba(0, 0, 0, 0.1);
}

#bookForm label {
font-weight: 600;
margin-top: 0.5rem;
}

#bookForm input[type="text"],
#bookForm input[type="number"] {
border-radius: 8px;
border: 1px solid #ccc;
padding: 0.5rem;
margin-bottom: 0.8rem;
}

#bookForm input:focus {
border-color: #17a2b8;
box-shadow: 0 0 5px rgba(23, 162, 184, 0.4);
}

#submitBtn {
width: 10%;
border-radius: 8px;
padding: 0.6rem;
font-weight: 600;
}

/* Table styling */
.table {
background: #fff;
border-radius: 12px;
overflow: hidden;
margin-top: 2rem;
box-shadow: 0 3px 12px rgba(0, 0, 0, 0.08);
}

.table th {
background: #343a40 !important;
color: #fff;
text-align: center;
vertical-align: middle;
}

.table td {
text-align: center;
vertical-align: middle;
}

/* Delete button */
.delete-btn {
background: #e74c3c;
border: none;
color: white;
padding: 6px 12px;
border-radius: 6px;
cursor: pointer;
font-size: 0.85rem;
transition: background 0.3s ease;
}

.delete-btn:hover {
background: #c0392b;
}

/* Checkbox styling */
.form-check-input {
transform: scale(1.2);
margin-right: 8px;
}

/* Smooth transitions */
* {
transition: all 0.2s ease-in-out;
}