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

Changes from 1 commit
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
12 changes: 12 additions & 0 deletions debugging/book-library/script.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,5 +58,17 @@ function render() {
for (let n = rowsNumber - 1; n > 0; n--) {
table.deleteRow(n);
}
// 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);

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