London | 26-ITP-May | Dipa Sarker | Sprint 2 | Book Library Project by Dipa-Sarker · Pull Request #521 · CodeYourFuture/Module-Data-Flows

Dipa-Sarker · 2026-08-06T20:33:08Z

Learners, PR Template

Self checklist

I have titled my PR with Region | Cohort | FirstName LastName | Sprint | Assignment Title
My changes meet the requirements of the task
I have tested my changes
My changes follow the style guide

Changelist

Changes made

Fixed bugs preventing books from being added correctly to the library.
Corrected book information handling, including title, author, and read status.
Improved the rendering of book cards on the page.
Added/fixed event handling for user interactions.
Debugged JavaScript issues using browser developer tools and verified the application behaviour.
Testing
Tested adding new books through the form.
Confirmed that book details display correctly.
Verified that the read status updates properly.
Checked that the application works correctly in the browser using Live Server.

The changes were tested locally and the application is working as expected.

cjyuan

Can you check if any of this general feedback can help you further improve your code?
https://github.com/CodeYourFuture/Module-Data-Flows/blob/general-review-feedback/debugging/book-library/feedback.md

Doing so can help me speed up the review process. Thanks.

Dipa-Sarker · 2026-08-09T23:22:02Z

Thank you for giving me the general feedback file. I made correction according to this.

cjyuan

Changes look solid. I just have a few suggestions.

cjyuan · 2026-08-10T02:53:22Z

@@ -1,17 +1,18 @@
 let myLibrary = [];


Can we declare myLibrary in a way that prevents it from being accidentally reassigned?

we can use const instead of let.

cjyuan · 2026-08-10T02:56:37Z

+const titleInput = document.getElementById("title"); //rename all variable names
+const authorInput = document.getElementById("author");
+const pagesInput = document.getElementById("pages");
+const checkInput = document.getElementById("check");
+const bookForm = document.getElementById("bookForm");


Note: Normal practice is to keep variable/constant declaration at beginning of the file (before function definition).

okay, moved all the variables at the very beginning.

cjyuan · 2026-08-10T03:03:31Z

+bookForm.addEventListener("submit", function (event) { //after user submits the book form 
+// this will run submit () function to add book
+  event.preventDefault();
+  submit();
+});


Common practice is to keep all the "code that runs once on page load" in the same place (e.g., in a function named init() or setup()) and then call the function on page load.

Code that setups event listener usually fall into this category.

cjyuan · 2026-08-10T03:07:39Z

-
-    changeBut.addEventListener("click", function () {
+    const changeButton = document.createElement("button"); //correct changeBut > changeButton
+    changeButton.id = i;


Is there any need to assign an id attribute to this element?
In addition, "0", "1", ..., do not make good ID value.

ok, we don't need button id, because we have event listener to response with change button. Besides, only id ="0", "1",...etc don't mean anything. I deleted this line.

cjyuan · 2026-08-10T03:08:32Z

+      const deletedTitle = myLibrary[i].title; //save the book title which should have to delete
      myLibrary.splice(i, 1);
      render();
+      alert(`You've deleted title: ${deletedTitle}`);


alert() is a blocking function call. As a result, invoking it prevents the browser from updating the UI until the dialog is dismissed.

If time permits, research for approaches that allows the UI to update before displaying the alert dialog. (This is an optional change).

Ok, I got your point. I did two things.

Use confirm() before deleting.

Put a normal message element in the HTML instead of using alert() after deletion.

cjyuan

Changes look good. Well done.

cjyuan · 2026-08-11T12:57:36Z

+/*window.addEventListener("load", function () {
  //delete e as it is never used
  populateStorage();
  render();
-});
+}); */


In practice, it is better to remove any unused code instead of wrap them in a comment in order to keep the code clean.

cjyuan · 2026-08-11T12:58:30Z

+       const confirmed = confirm( //use confirm () to handle popup message for ok/cancel
+      `Are you sure you want to delete "${deletedTitle}"?`
+       );


Indentation is off.

Dipa-Sarker · 2026-08-11T14:00:09Z

Thank you for your feedback!

fix book library app bugs

956cf0e

Dipa-Sarker added the Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. label Aug 7, 2026

cjyuan reviewed Aug 7, 2026

View reviewed changes

cjyuan added Reviewed Volunteer to add when completing a review with trainee action still to take. and removed Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. labels Aug 7, 2026

made correction according to general feedback

6a246e7

Dipa-Sarker added Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. and removed Reviewed Volunteer to add when completing a review with trainee action still to take. labels Aug 9, 2026

cjyuan reviewed Aug 10, 2026

View reviewed changes

cjyuan added Reviewed Volunteer to add when completing a review with trainee action still to take. and removed Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. labels Aug 10, 2026

correct reviewers comments

586658c

Dipa-Sarker added Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. and removed Reviewed Volunteer to add when completing a review with trainee action still to take. labels Aug 11, 2026

cjyuan reviewed Aug 11, 2026

View reviewed changes

cjyuan added Complete Volunteer to add when work is complete and all review comments have been addressed. and removed Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. labels Aug 11, 2026

Uh oh!

Uh oh!

Conversation

Dipa-Sarker commented Aug 6, 2026

Learners, PR Template

Changelist

Uh oh!

cjyuan left a comment

Choose a reason for hiding this comment

Uh oh!

Dipa-Sarker commented Aug 9, 2026

Uh oh!

cjyuan left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cjyuan left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Dipa-Sarker commented Aug 11, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants