Glasgow | 26-ITP-May | Tuan Nguyen | Sprint 2 | Book-Library-Project - #556
Glasgow | 26-ITP-May | Tuan Nguyen | Sprint 2 | Book-Library-Project#556Jacknguyen4438 wants to merge 9 commits into
Conversation
…eing added correctly.
…utton in the library table
cjyuan
left a comment
There was a problem hiding this comment.
Could 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.
|
Hello @cjyuan, thank you very much for your time on review my PR. I have make needed change after reading feedback.md and do validation check. I have fix the issue with meta tag with incorrect attribute, input and label. For JS file I updated the JavaScript code by removing an unnecessary render() call during page load, improving variable names to make it clearer that they reference DOM elements, trimming user input before validation, and converting the page count from a string to a number. If further change need to be make please let me know thank you. |
Use const for myLibrary and improve page count validation by rejecting negative and decimal values.
By moving shared variables to the top of the file and refactoring table rendering to clear and update the tbody more efficiently.
By adding proper form support for browser validation and applying consistent code formatting and indentation.
By improving Bootstrap form markup, handling form submission through addEventListener, and updating delete notification behaviour.
|
Hello @cjyuan, thank you so much on your feed back on the mistake I make on the code, I have making fix and change base on the feed and it ready for your review again. if further improvement needed please let know. |
cjyuan
left a comment
There was a problem hiding this comment.
Changes look good. Well done.
| bookForm.addEventListener("submit", function (event) { | ||
| event.preventDefault(); | ||
| 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.
| Number.isNaN(pagesValue) || | ||
| pagesValue < 1 || | ||
| !Number.isInteger(pagesValue) |
There was a problem hiding this comment.
One of these checks is optional (redundant).
There was a problem hiding this comment.
Thank you for the feed back I see that there is better way of doing this validation check I will test this and mesage you back as soon as possible.
Learners, PR Template
Self checklist
Changelist
Hello and thank you for checking this course work PR. In this PR I have fixed all major issues with in the project, the book library is now is fully function and displayed properly. If further fix and change need to be made please let me know thank you.
Questions
No Question.