Uh oh!
There was an error while loading. Please reload this page.
London | 26-ITP-May | Jorvan White | Sprint 2 | Book library - #548
London | 26-ITP-May | Jorvan White | Sprint 2 | Book library#548JorvanW wants to merge 17 commits into
Conversation
… and made sure it can produce error with not inputting author
This comment has been minimized.
This comment has been minimized.
cjyuan
left a comment
There was a problem hiding this comment.
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.
JorvanW
commented
Aug 16, 2026
I think I have made some relevant changes following the link given |
Uh oh!
There was an error while loading. Please reload this page.
| <button | ||
| type="button" | ||
| class="btn btn-primary" | ||
| onclick="submit();" |
There was a problem hiding this comment.
Could you look up the trade-off between:
- Assigning event listener in HTML
- Assigning event listener in JS via
.addEventListener()
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
| } 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); |
There was a problem hiding this comment.
What if user enter title as " C++ Programming ", and page count as "30e1", "00030", or "12.345"?
There was a problem hiding this comment.
user is no longer able to input anything in page count that isn't a whole positive number
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
… instead clear all at once
…the book delete before the alert message is passed
JorvanW
commented
Aug 17, 2026
I have made relevant changes from the comments given. Please let me know if there are other issues to work on. |
JorvanW
commented
Aug 17, 2026
At the time of writing this, I believe github is down and causing some problem with loading |
cjyuan
commented
Aug 17, 2026
Changes are good. Could you also address the remaining three "unresolved" comments left in my previous review? Two of which are related to |
JorvanW
commented
Aug 18, 2026
Hello, I could be wrong but I have addressed these comments that are not resolved yesterday but because github was down they weren't synced properly. Please let me know if there are still mistakes I have overlooked in case its a fault on my end. Many Thanks |
cjyuan
commented
Aug 18, 2026
JorvanW
commented
Aug 18, 2026
fixed changes mentioned from comments |
| const bookForm = document.getElementById("bookForm"); | ||
| const bookTableBody = document.getElementById("bookTableBody"); | ||
| submitButton.addEventListener("click", submit); |
There was a problem hiding this comment.
Could consider placing all code that runs once on page load in a single function. For example, you could put it inside the page load callback or create a function named init() or setup() and call it once when the page loads.
This makes it easier to locate and manage all the code that runs once when the app starts.

Self checklist
Changelist
Created Pull Request from Data Flows Book Library Task