Skip to content
Closed
Show file tree
Hide file tree
Changes from 16 commits
Commits
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
18 changes: 5 additions & 13 deletions debugging/book-library/index.html
Original file line number Diff line number Diff line change
@@ -1,11 +1,9 @@
<!DOCTYPE html>
<html>
<head>
<title> </title>
<title>My Library</title>
<meta
charset="utf-8"
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>
Expand All @@ -31,15 +29,15 @@ <h1>Library</h1>
<div class="form-group">
<label for="title">Title:</label>
<input
type="title"
type="text"

Choose a reason for hiding this comment

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

Right now you need to manually validate the form. Do you know what change you need to make to the HTML and script to make the browser validate the form inputs automatically?

Copy link
Author

Choose a reason for hiding this comment

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

lines 47 & 56

I have removed the some of the manual validations i used earlier in the script and replaced it with an autotmatic error handler in my html for the inputs pattern="[a-zA-Z\s.'-]+" , required min="1"

class="form-control"
id="title"
name="title"
required
/>
<label for="author">Author: </label>
<input
type="author"
type="text"
class="form-control"
id="author"
name="author"
Expand Down Expand Up @@ -72,7 +70,7 @@ <h1>Library</h1>

<table class="table" id="display">
<thead class="thead-dark">
<tr>
<tr class="clickable">
<th>Title</th>
<th>Author</th>
<th>Number of Pages</th>
Expand All @@ -81,13 +79,7 @@ <h1>Library</h1>
</tr>
</thead>
<tbody>
<tr>
<td></td>
<td></td>
<td></td>
<td></td>
<td></td>
</tr>
<!-- Removed empty <tr> to allow render() to populate rows -->
</tbody>
</table>

Expand Down
132 changes: 64 additions & 68 deletions debugging/book-library/script.js
Original file line number Diff line number Diff line change
@@ -1,22 +1,15 @@
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
);
myLibrary.push(book1);
myLibrary.push(book2);
render();
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);
myLibrary.push(book1, book2);
}
}

Expand All @@ -25,22 +18,31 @@ const author = document.getElementById("author");
const pages = document.getElementById("pages");
const check = document.getElementById("check");

//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 == ""
) {
const titleValue = title.value.trim();
const authorValue = author.value.trim();
const pagesValue = pages.value.trim();

if (!titleValue || !authorValue || !pagesValue) {

Choose a reason for hiding this comment

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

Are there any values in the form you might want to validate more carefully than just "is there data here"?

Copy link
Author

Choose a reason for hiding this comment

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

yes there are a couple of validation rules i should have added.

line 32

I added a conditon if (!/^[a-zA-Z\s.'-]+$/.test(authorValue) to validate authors value, limiting it to only letters, spaces, periods, apostrophes, and hyphens.

line 38

I added another condition for page number if (!Number.isInteger(pagesNumber) || pagesNumber <= 0) to validate only postive numbers.

alert("Please fill all fields!");
return false;
} else {
let book = new Book(title.value, title.value, pages.value, check.checked);
library.push(book);
render();
}

// Prevent numbers in Author field
if (/\d/.test(authorValue)) {
alert("Author name cannot contain numbers!");
return false;
}

let book = new Book(titleValue, authorValue, Number(pagesValue), check.checked);
myLibrary.push(book);
render();

// Reset form
title.value = "";
author.value = "";
pages.value = "";
check.checked = false;
Comment on lines +54 to +57
Copy link
Contributor

Choose a reason for hiding this comment

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

We could also use .reset() of a form element to clear the form.

}

function Book(title, author, pages, check) {
Expand All @@ -51,53 +53,47 @@ function Book(title, author, pages, check) {
}

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;

//add and wait for action for read/unread button
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 () {
const table = document.getElementById("display");
const tbody = table.querySelector("tbody");

// Clear all existing rows
tbody.innerHTML = "";

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

row.insertCell(0).textContent = book.title;
row.insertCell(1).textContent = book.author;
row.insertCell(2).textContent = book.pages;

// Read/unread button
const wasReadCell = row.insertCell(3);
const readToggleButton = document.createElement("button");
readToggleButton.className = "btn btn-success";
readToggleButton.textContent = book.check ? "Yes" : "No";
wasReadCell.appendChild(readToggleButton);

readToggleButton.addEventListener("click", () => {
myLibrary[i].check = !myLibrary[i].check;
render();
});

//add delete button to every row and render again
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}`);
myLibrary.splice(i, 1);
render();
// Delete button
const deleteCell = row.insertCell(4);
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you think of the pros and cons of these two approaches for creating cells within a row?

  • Keeping all the cell creation code in one location, like the original code does.
  • Scattering the cell creation code across different locations, like what you did.

const deleteButton = document.createElement("button");
deleteButton.className = "btn btn-warning";
deleteButton.textContent = "Delete";
deleteCell.appendChild(deleteButton);

deleteButton.addEventListener("click", () => {
myLibrary.splice(i, 1); // delete immediately
render(); // update the table
alert(`Deleted "${book.title}" successfully.`); // optional confirmation

Choose a reason for hiding this comment

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

When you are deleting something like a file on your PC, do you normally expect to be notified after, or is there an interaction that would work that is better than alert?

Copy link
Author

Choose a reason for hiding this comment

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

line 22 - 35

Instead of blocking alerts, i have used a notification function function showNotification(message) which gives a non interrputing visually nicer notification instead of popup alert.

});
}
});
}

// Make header row toggle the form
document.querySelector(".thead-dark tr").addEventListener("click", function () {
$("#demo").collapse("toggle");
});
4 changes: 4 additions & 0 deletions debugging/book-library/style.css
Original file line number Diff line number Diff line change
Expand Up @@ -17,3 +17,7 @@
button.btn-info {
margin: 20px;
}

.clickable {
cursor: pointer;
}