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.
|
I think I have made some relevant changes following the link given |
| <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()
| } 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
… instead clear all at once
…the book delete before the alert message is passed
|
I have made relevant changes from the comments given. Please let me know if there are other issues to work on. |
|
At the time of writing this, I believe github is down and causing some problem with loading |
|
Changes are good. Could you also address the remaining three "unresolved" comments left in my previous review? Two of which are related to |
|
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 |
|
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