-
-
Notifications
You must be signed in to change notification settings - Fork 235
London | 26-ITP-Jan | Angela McLeary | Sprint 2 | Book Library #415
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from 9 commits
6fe2b67
b647506
dc2969c
6146e7f
731ac4c
377bd24
200d98c
6d570e5
ff6b347
376af2f
1d6d91b
366d541
957ce0b
b6bb86f
c87d77d
89af4d0
1a97cd5
6822947
5350f6e
9228c2f
5062215
10ae61f
ece0823
6b1fb53
380c723
820a4e1
f170475
94388ba
eb6981c
adfb8c0
d53e21d
e4c4826
e29da34
d89ea46
6fff30c
eab8a1f
851e2ce
4e49d1d
c96752d
dc383b1
2474cd9
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -31,14 +31,18 @@ function submit() { | |
| if ( | ||
| title.value == null || | ||
| title.value == "" || | ||
| author.value == null || | ||
| author.value == "" || | ||
| pages.value == null || | ||
| pages.value == "" | ||
| pages.value == "" || | ||
| pages.value <= 0 || | ||
| pages.value != parseInt() | ||
| ) { | ||
| alert("Please fill all fields!"); | ||
| alert("Please fill all fields with valid input!"); | ||
| return false; | ||
| } 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); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Using raw input value without properly pre-processing them is dangerous. Here are some input that can "mess up" the app:
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can you address this comment? Currently some input values can still "mess up" the app.
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hi @cjyuan, Thank you for your feed back. I’ve updated the validation. I now trim all input values before checking them, and I added a stricter regex (/^[1-9]\d*$/) for the pages field so it only accepts whole positive numbers without leading zeros. This prevents unsafe or misleading inputs like " " or "3e-2" from being treated as valid. Thanks for the heads‑up! |
||
| myLibrary.push(book); | ||
| render(); | ||
| } | ||
| } | ||
|
|
@@ -54,7 +58,7 @@ function render() { | |
| let table = document.getElementById("display"); | ||
| let rowsNumber = table.rows.length; | ||
| //delete old table | ||
| for (let n = rowsNumber - 1; n > 0; n-- { | ||
| for (let n = rowsNumber - 1; n > 0; n--) { | ||
| table.deleteRow(n); | ||
| } | ||
| //insert updated row and cells | ||
|
|
@@ -77,9 +81,9 @@ function render() { | |
| wasReadCell.appendChild(changeBut); | ||
| let readStatus = ""; | ||
| if (myLibrary[i].check == false) { | ||
| readStatus = "Yes"; | ||
| } else { | ||
| readStatus = "No"; | ||
| } else { | ||
| readStatus = "Yes"; | ||
| } | ||
| changeBut.innerText = readStatus; | ||
|
|
||
|
|
@@ -89,12 +93,12 @@ function render() { | |
| }); | ||
|
|
||
| //add delete button to every row and render again | ||
| let delButton = document.createElement("button"); | ||
| let delBut = document.createElement("button"); | ||
| delBut.id = i + 5; | ||
| deleteCell.appendChild(delBut); | ||
| delBut.className = "btn btn-warning"; | ||
| delBut.innerHTML = "Delete"; | ||
| delBut.addEventListener("clicks", function () { | ||
| delBut.addEventListener("click", function (index) { | ||
| alert(`You've deleted title: ${myLibrary[i].title}`); | ||
| myLibrary.splice(i, 1); | ||
| render(); | ||
|
AngelaMcLeary marked this conversation as resolved.
Outdated
|
||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.