Glasgow | 26-ITP-May |Chan Yat Long |Sprint 2 | Book Library by Motorcycle-lab · Pull Request #560 · CodeYourFuture/Module-Data-Flows

Motorcycle-lab · 2026-08-17T00:03:02Z

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

The book project

Questions

If I can self learn all backend and frameworks by myself, build a porfolio by myself and become a full-stack engineer.

cjyuan

Could you fix the checkboxes in the PR description?
Coud 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.

cjyuan · 2026-08-18T17:09:17Z

Did you push your changes to GitHub?

Motorcycle-lab · 2026-08-19T09:36:49Z

Sorry sir. I am a bit confusing because If you clicked on my file change, I have already updated the file.

cjyuan · 2026-08-19T13:17:18Z

Could you go through the "general feedback" document again to see what else you could also improve?

cjyuan

Have you also checked `index.html for errors or possible improvements?

cjyuan · 2026-08-21T09:31:01Z

+      alert("Are you sure you want to delete the book ?");
      myLibrary.splice(i, 1);
      render();


alert() won't let a user choose Yes or No. Besides, this message is not quite the same as the original message.
The intent of the original message is to inform the user which book was deleted.

cjyuan · 2026-08-21T09:36:58Z

+  const table = document.getElementById("display");
+  table.innerHTML = "";
  let rowsNumber = table.rows.length;
  //delete old table
-  for (let n = rowsNumber - 1; n > 0; n-- {
-    table.deleteRow(n);
-  }
+  const tobody = table.querySelector("tbody") || table;
+  tobody.innerHTML = "";


Why clear both <table> and <tbody>?

Where is rowsNumber used?

Where is tobody used?

cjyuan · 2026-08-21T09:37:46Z

According to https://validator.w3.org/, there are errors in your index.html. Can you fix these errors?

cjyuan · 2026-08-21T09:38:42Z

    <div id="demo" class="collapse">
+<form id="bookform">


Good idea to introduce <form>. Why not use a formatter to keep the code consistently formatted?

cjyuan · 2026-08-21T09:39:15Z

        <input
          type="number"
          class="form-control"
          id="pages"
          name="pages"
-          required
        />


Why remove required?

Why not make this input element to accept only positive whole number?

cjyuan · 2026-08-21T09:39:51Z

@@ -61,13 +59,9 @@ <h1>Library</h1>
            value=""
          />Read
        </label>


Currently this checkbox is not showing.

The issue is related to Bootstrap 4.4.1. Could you use AI to find a way to fix the issue? Mentioning "Bootstrap 4.4.1" might help.

cjyuan · 2026-08-21T09:42:05Z

-          class="btn btn-primary"
-          onclick="submit();"
-        />
+        <button type="button" class="btn btn-primary mt-3" onclick="processbook()">Submit</button>


Could you look up the trade off between

Assigning event listener in HTML

Assigning event listener in JS using .addEventListener()

cjyuan · 2026-08-21T09:49:03Z

-        <input
-          type="submit"
-          value="Submit"
-          class="btn btn-primary"
-          onclick="submit();"
-        />
+        <button type="button" class="btn btn-primary mt-3" onclick="processbook()">Submit</button>


Changing the submit button to a regular button means the browser won't check the constraints such as required when the user clicks the button. If your intention is to prevent the browser from submitting the form, which would cause the page to reload, you can call .preventDefault() on the event object passed to the event listener.

Arthur added 2 commits July 29, 2026 13:46

update book

d0f5bcd

fix all the bug

ef308cb