London | ITP-May-2025 | Mo Muchunu | Module-Data-Flows | Sprint 2 | Book Library #243

Mo-Muchunu · 2025-07-27T06:33:42Z

Learners, PR Template

Self checklist

I have committed my files one by one, on purpose, and for a reason
I have titled my PR with Region | Cohort | FirstName LastName | Sprint | Assignment Title
I have tested my changes
My changes follow the style guide
My changes meet the requirements of this task

Changelist

Fixed rendering bug that prevented books from displaying on page load.
Corrected input handling to prevent missing values.
Improved form validation and reset inputs after submission.
Fixed broken delete functionality and removed placeholder rows for cleaner display.

Questions

… for deletions.

cjyuan

Good job in fixing the errors in the HTML file.
Code is working good. I just have a bunch of suggestions.

cjyuan · 2025-08-02T13:44:51Z

debugging/book-library/script.js

+  authorValue.length < minLength ||
+  !/^(?=(?:.*[A-Za-z]){3,})[A-Za-z ]+$/.test(titleValue) ||                                       // Validate input for minimum length and character type
+  !/^(?=(?:.*[A-Za-z]){3,})[A-Za-z ]+$/.test(authorValue)


These rules seem to be a bit restrictive. A lot of book titles have digits or symbols like "&", ",", "-".

The indentation level of this if-block is off.

cjyuan · 2025-08-02T13:45:40Z

debugging/book-library/script.js

+    title.value == null ||                                                                        // Clear form fields after successful submission
    title.value == "" ||
+    author.value == null ||                                                                       
+    author.value == "" ||


Can .value be null? (Do we need to check someInputElement.value == null?)

Are these checks still needed?

cjyuan · 2025-08-02T13:55:03Z

debugging/book-library/script.js

+    const book = new Book(title.value, author.value, pages.value, check.checked);                 // Correct 'author' key
+    myLibrary.push(book);                                                                         // Correct array name


With the current check, the page number shown in this screenshot is possible.

The title and author may contain leading and trailing space characters.

Should the page number be stored as a string or as a number

cjyuan · 2025-08-02T15:12:55Z

debugging/book-library/script.js

+  for (let n = rowsNumber - 1; n > 0; n--) {                                                      // Add missing closing bracket
    table.deleteRow(n);
  }


Instead of deleting the table rows one by one, can you think of a more efficient way to remove all rows (except the <th>...</th>) in the table?

cjyuan · 2025-08-02T15:21:22Z

debugging/book-library/script.js

    titleCell.innerHTML = myLibrary[i].title;
    authorCell.innerHTML = myLibrary[i].author;
    pagesCell.innerHTML = myLibrary[i].pages;


Please note that when setting the text content of an HTML element, there are subtle but important differences between using .innerHTML, innerText, and textContent.

cjyuan · 2025-08-02T15:25:44Z

debugging/book-library/script.js

    let readStatus = "";
-    if (myLibrary[i].check == false) {
-      readStatus = "Yes";
-    } else {
-      readStatus = "No";
-    }
+    readStatus = myLibrary[i].check ? "Yes" : "No";                                               // Correct read status logic for not read


We could also write

const readStatus = myLibrary[i].check ? "Yes" : "No";

cjyuan · 2025-08-02T15:34:57Z

debugging/book-library/script.js

+
+    delButton.addEventListener("click", function () {                                             // Click not clicks
+      const index = parseInt(this.getAttribute("data-index"), 10);                                // Get index from button's data attribute and convert from string to number (base 10)
+      if (confirm(`You're about to delete "${myLibrary[index].title}". Continue?`)) {             // Require user confirmation before deletion


Nice touch in changing alert to confirm!

cjyuan · 2025-08-02T15:41:28Z

debugging/book-library/script.js

-      alert(`You've deleted title: ${myLibrary[i].title}`);
-      myLibrary.splice(i, 1);
-      render();
+    const delButton = document.createElement("button");


Did you notice that the two variables, changeBut and delButton, use different naming conventions?

cjyuan · 2025-08-02T15:54:39Z

debugging/book-library/script.js

+    delButton.className = "btn btn-warning";                                                      // Correct delButton variable name
+    delButton.innerHTML = "Delete";
+    delButton.setAttribute("data-index", i);                                                      // Add data attribute to identify the book index
+    deleteCell.appendChild(delButton);
+
+    delButton.addEventListener("click", function () {                                             // Click not clicks
+      const index = parseInt(this.getAttribute("data-index"), 10);                                // Get index from button's data attribute and convert from string to number (base 10)


Why use "data-index" attribute to store the index with delButton, but not with changeBut?

Mo-Muchunu force-pushed the Sprint-2/feature/book-library branch from 3c0fb60 to 8ddf6f9 Compare July 27, 2025 06:48

Fixed book display bug, form validation, and delete button

0e6cabf

Mo-Muchunu force-pushed the Sprint-2/feature/book-library branch from 8ddf6f9 to 0e6cabf Compare July 27, 2025 07:02

Mo-Muchunu added the Needs Review Participant to add when requesting review label Jul 27, 2025

Mo-Muchunu mentioned this pull request Jul 29, 2025

ITP-May-2025 | Data Flows | Mo Muchunu Mo-Muchunu/My-Coursework-Planner#85

Open

Mo-Muchunu added 2 commits July 31, 2025 05:55

Improved form validation, added regex checks and confirmation prompts…

afee38d

… for deletions.

Removed whitespace

fd12d23

cjyuan added the Review in progress This review is currently being reviewed. This label will be replaced by "Reviewed" soon. label Aug 2, 2025

cjyuan reviewed Aug 2, 2025

View reviewed changes

cjyuan added Reviewed Volunteer to add when completing a review and removed Needs Review Participant to add when requesting review Review in progress This review is currently being reviewed. This label will be replaced by "Reviewed" soon. labels Aug 2, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

London | ITP-May-2025 | Mo Muchunu | Module-Data-Flows | Sprint 2 | Book Library #243

London | ITP-May-2025 | Mo Muchunu | Module-Data-Flows | Sprint 2 | Book Library #243

Uh oh!

Mo-Muchunu commented Jul 27, 2025

Uh oh!

cjyuan left a comment

Uh oh!

cjyuan Aug 2, 2025

Uh oh!

cjyuan Aug 2, 2025

Uh oh!

cjyuan Aug 2, 2025

Uh oh!

cjyuan Aug 2, 2025

Uh oh!

cjyuan Aug 2, 2025

Uh oh!

cjyuan Aug 2, 2025

Uh oh!

cjyuan Aug 2, 2025

Uh oh!

cjyuan Aug 2, 2025

Uh oh!

cjyuan Aug 2, 2025

Uh oh!

Uh oh!

		const book = new Book(title.value, author.value, pages.value, check.checked); // Correct 'author' key
		myLibrary.push(book); // Correct array name

Uh oh!

London | ITP-May-2025 | Mo Muchunu | Module-Data-Flows | Sprint 2 | Book Library #243

Are you sure you want to change the base?

London | ITP-May-2025 | Mo Muchunu | Module-Data-Flows | Sprint 2 | Book Library #243

Uh oh!

Conversation

Mo-Muchunu commented Jul 27, 2025

Learners, PR Template

Changelist

Questions

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!

Uh oh!