Cape Town | 26-ITP-May | Enice Mutanda | Sprint 2 | Book Library - #567
Cape Town | 26-ITP-May | Enice Mutanda | Sprint 2 | Book Library#567Enice-Codes wants to merge 2 commits into
Conversation
correct invalid HTML in book-library form - Add proper page title - Split malformed charset/viewport meta tag into two valid tags - Fix invalid input types (title/author) to type="text" - Properly associate checkbox label with for/id
- Fix Robinson Crusoe title typo - Use myLibrary instead of undefined library in submit() - Pass author.value instead of duplicate title.value - Add missing closing parenthesis in render() loop - Fix delButton variable name mismatch - Correct "clicks" event typo to "click" - Fix read/unread status logic
Enice-Codes
added
the
Needs Review
This comment has been minimized.
This comment has been minimized.
github-actions
Bot
removed
the
Needs Review
This comment has been minimized.
This comment has been minimized.
Enice-Codes
added
the
NotCoursework
cjyuan
changed the title
cjyuan
left a comment
cjyuan
left a comment
There was a problem hiding this comment.
Could you also 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.
| @@ -53,14 +51,17 @@ <h1>Library</h1> | |||
| name="pages" | |||
| required | |||
| /> | |||
There was a problem hiding this comment.
Why not make this input element to accept only positive whole number?
| <input | ||
| type="author" | ||
| type="text" | ||
| class="form-control" | ||
| id="author" | ||
| name="author" | ||
| required | ||
| /> |
There was a problem hiding this comment.
The browser checks the input elements against the specified constraints when a user submits a form.
Without <form>, the browser won't enforce the "required" constraint.
| <label class="form-check-label" for="check"> | ||
| <input | ||
| type="checkbox" | ||
| class="form-check-input" | ||
| id="check" | ||
| value="" | ||
| />Read | ||
| name="check" | ||
| /> | ||
| Read | ||
| </label> |
There was a problem hiding this comment.
The 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.
| @@ -1 +1 @@ | |||
| let myLibrary = []; | |||
There was a problem hiding this comment.
Could we declare myLibrary in a way that prevents it from being accidentally reassigned?
| let book1 = new Book("Robinson Crusoe", "Daniel Defoe", "252", true); | ||
| let book2 = new Book( | ||
| "The Old Man and the Sea", | ||
| "Ernest Hemingway", | ||
| "127", | ||
| true | ||
| ); |
There was a problem hiding this comment.
Why represent pages using two different types of data?
cjyuan
added
Reviewed
2 participants
Self checklist
Changelist
index.html*
<title><meta>tag into two valid tagstype="title",type="author") totype="text"idusingforscript.js
submit()referencing an undefinedlibraryvariable — now correctly usesmyLibrarysubmit()passingtitle.valuetwice instead ofauthor.valuewhen creating a new bookrender()for-loop (syntax error)delButton/delButvariable names causing a reference error"clicks"typo in event listener — corrected to"click"so the delete button works