Manchester | 26-ITP-Jan | Mehroz Munir | Sprint 2| Book Library#433
Manchester | 26-ITP-Jan | Mehroz Munir | Sprint 2| Book Library#433MehrozMunir wants to merge 3 commits intoCodeYourFuture:mainfrom
Conversation
MehrozMunir
added
📅 Sprint 2
There was a problem hiding this comment.
According to https://validator.w3.org/, there is an error in your index.html. Can you fix it?
| @@ -1,14 +1,13 @@ | |||
| let myLibrary = []; | |||
There was a problem hiding this comment.
Could consider declaring it using const.
| const book = new Book( | ||
| titleInput.value, | ||
| authorInput.value, | ||
| pagesInput.value, |
There was a problem hiding this comment.
.value is a string, and a user can enter a value such as "1.4e2" (a numerical value equals to 140) in the input field.
There was a problem hiding this comment.
Yes, that's true. I have resolved this issue by converting this back to a number when rendering this value, as shown here
pagesCell.textContent = Number(myLibrary[i].pages); //line 81 of script.js
| readStatus = "No"; | ||
| } | ||
| changeBut.innerText = readStatus; | ||
| myLibrary[i].check ? (readStatus = "Yes") : (readStatus = "No"); |
There was a problem hiding this comment.
Normally way to use ? : is:
readStatus = myLibrary[i].check ? "Yes" : "No";
condition ? v1 : v2 is an expression. It evaluates to v1 if condition is true, and to v2 if condition is false.
cjyuan
added
Reviewed
|
@cjyuan Thank you for your useful feedback. I have made the changes you mentioned in the comments. Can you please review the code now. |
MehrozMunir
added
the
Needs Review
| const deleteCell = row.insertCell(4); | ||
| titleCell.textContent = myLibrary[i].title; | ||
| authorCell.textContent = myLibrary[i].author; | ||
| pagesCell.textContent = Number(myLibrary[i].pages); |
There was a problem hiding this comment.
This may work for this app, but it is a dangerous practice.
A better place to ensure pages are number is in the Book constructor (line 60).
There was a problem hiding this comment.
Yes thanks for highlighting that.
I have done that now.
this.pages = Number(pages);
cjyuan
added
Complete
2 participants
Learners, PR Template
Self checklist
Changelist
I have tried to make the book library website more interactive, added validation for the input fields, and resolved the bugs in rendering and adding data.