Skip to content
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

London | Emmanuel Gessessew | Module-Data-Flows| WEEK 2 Book library project #124

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

London | Emmanuel Gessessew | Module-Data-Flows| WEEK 2 Book library project #124

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
adf10a0
Fix book submission and use 'myLibrary' instead of 'library'.
Emmanuelgessessew Jan 5, 2025
4d2e8f8
Fix render to delete only old rows.
Emmanuelgessessew Jan 5, 2025
7239a23
Insert rows and cells for each book
Emmanuelgessessew Jan 5, 2025
c4d3ae4
Add delete button to each row and implement delete functionality
Emmanuelgessessew Jan 5, 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
3 changes: 3 additions & 0 deletions .vscode/settings.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
{
"liveServer.settings.port": 5502
Copy link

Choose a reason for hiding this comment

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

There is debate as to whether you should commit your .vscode file to version control. Generally it is things related to your own personal setup, and you don't want to force other users to share them. Google .gitignore to discover how to exclude files or folders from version control.

Sometimes you do want local configuration to be shared, but only do it intentionally!

}
71 changes: 21 additions & 50 deletions debugging/book-library/script.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ window.addEventListener("load", function (e) {

function populateStorage() {
if (myLibrary.length == 0) {
let book1 = new Book("Robison Crusoe", "Daniel Defoe", "252", true);
let book1 = new Book("Robinson Crusoe", "Daniel Defoe", "252", true);
Copy link

Choose a reason for hiding this comment

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

This is a good correction. Having accurate test data in software helps users and testers take it more seriously. So well done for bothering to correct this.

Copy link
Author

Choose a reason for hiding this comment

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

thanks

let book2 = new Book(
"The Old Man and the Sea",
"Ernest Hemingway",
Expand All @@ -25,24 +25,25 @@ 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
// Check the right input from forms and if it's ok, add the new book (object in array)
// via Book function and start render function
function submit() {
if (
title.value == null ||
title.value == "" ||
author.value == null ||
author.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);
let book = new Book(title.value, author.value, pages.value, check.checked);
myLibrary.push(book); // Use myLibrary here, not library
render();
}
}

function Book(title, author, pages, check) {
this.title = title;
this.author = author;
Expand All @@ -53,51 +54,21 @@ 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-- {
// Delete old table rows except for the header
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;
// Insert updated rows and cells
let length = myLibrary.length;
for (let i = 0; i < length; i++) {
let row = table.insertRow(1);
Copy link

Choose a reason for hiding this comment

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

What does the 1 argument do here? How does the result differ if you leave it out? Which do you prefer?

Copy link
Author

Choose a reason for hiding this comment

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

The 1 argument makes the row insert as the second row, below the header. Without it, the row is added at the end of the table. I prefer using 1 to keep new rows consistently under the header.

Copy link

Choose a reason for hiding this comment

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

If that's what you want, that's fine. It does of course reverse the order of the items...

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);

changeBut.addEventListener("click", function () {
myLibrary[i].check = !myLibrary[i].check;
render();
});
titleCell.innerHTML = myLibrary[i].title;
authorCell.innerHTML = myLibrary[i].author;
pagesCell.innerHTML = myLibrary[i].pages;
Copy link

Choose a reason for hiding this comment

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

I'm pretty sure your code wasn't working when you made this commit, because there are a whole bunch of missing } at the end (which show up in the next commit). It's generally considered prudent to have committed code in a reasonable state of "cleanness" as others may check it out and try to use it. Less of an issue on a branch that only you are using, but something to bear in mind


//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();
});
}
}