-
-
Notifications
You must be signed in to change notification settings - Fork 263
London | 26-ITP-May | Chun Yan Wong | Sprint 2 | Book library #525
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
Open
cywong-dev
wants to merge
26
commits into
CodeYourFuture:main
Choose a base branch
from
cywong-dev:book-library
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+98
−57
Open
Changes from all commits
Commits
Show all changes
26 commits
Select commit
Hold shift + click to select a range
286b82e
Fix syntax error in for loop for row deletion
cywong-dev fbcee08
Change library to mylibrary for book storage
cywong-dev 1da4c07
Fix book constructor to use author value
cywong-dev 6350c1a
Correct delete button variable name
cywong-dev d030a55
Fix event listener typo for delete button
cywong-dev 780b6ac
Change row insertion to append at the end of table
cywong-dev ef6d410
Fix read status check in script.js
cywong-dev b74b562
Table deletion broken
cywong-dev f1bcc2a
Refactor book creation and storage in populateStorage
cywong-dev f447b3d
Enhance input validation in submit function
cywong-dev 35a2793
Fix empty table shown initially
cywong-dev d620ffb
Simplify rendering of library rows and buttons
cywong-dev a8f4d2c
Fix delete button creation and event listener
cywong-dev 374ddfd
Remove render call during library initialization
cywong-dev 456c37a
Add comments to explain detail changes
cywong-dev 023159b
Update HTML structure and input types for html validation
cywong-dev 1e71bf3
Fix book1 page number type in populateStorage
cywong-dev 1cf530c
Remove developer name comment
cywong-dev bb941e0
Fix rendering issue by clearing table content
cywong-dev 7f0c9c1
Remove unused variable in render function
cywong-dev 65c0c35
Use trimmed values instead of untrimmed values for updating
cywong-dev c0684e9
Implement status message display and fix variable name
cywong-dev c5d9795
Add table header to book library
cywong-dev 335ed1f
Add input validation for pages number field
cywong-dev 372ee5b
Refactor variable names for clarity in script.js
cywong-dev 57bbea8
Change myLibrary to a constant
cywong-dev File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Some comments aren't visible on the classic Files Changed page.
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,4 +1,4 @@ | ||
| let myLibrary = []; | ||
| const myLibrary = []; | ||
|
|
||
| window.addEventListener("load", function (e) { | ||
| populateStorage(); | ||
|
|
@@ -7,40 +7,67 @@ window.addEventListener("load", function (e) { | |
|
|
||
| function populateStorage() { | ||
| if (myLibrary.length == 0) { | ||
| let book1 = new Book("Robison Crusoe", "Daniel Defoe", "252", true); | ||
| let book2 = new Book( | ||
| "The Old Man and the Sea", | ||
| "Ernest Hemingway", | ||
| "127", | ||
| true | ||
| ); | ||
| myLibrary.push(book1); | ||
| myLibrary.push(book2); | ||
| render(); | ||
| let book1 = new Book("Robison Crusoe", "Daniel Defoe", 252, true); | ||
| let book2 = new Book("The Old Man and the Sea","Ernest Hemingway",127,true); | ||
|
cywong-dev marked this conversation as resolved.
|
||
|
|
||
|
|
||
| // Date : 9/8/2026 | ||
| // Purpose : Fix the problem "2. Error in console when you try to add a book" | ||
| // Change : replace Librarty with the correct variable myLibrary | ||
| myLibrary.push(book1, book2); | ||
| } | ||
| } | ||
|
|
||
| const title = document.getElementById("title"); | ||
| const author = document.getElementById("author"); | ||
| const pages = document.getElementById("pages"); | ||
| const check = document.getElementById("check"); | ||
| const titleElement = document.getElementById("title"); | ||
| const authorElement = document.getElementById("author"); | ||
| const pagesElement = document.getElementById("pages"); | ||
| const checkElement = document.getElementById("check"); | ||
|
|
||
| // Helper function to display messages to the user in the UI | ||
| function displayStatusMessage(message, isError = false) { | ||
| const statusContainer = document.getElementById("status-message") || createStatusContainer(); | ||
| statusContainer.textContent = message; | ||
| statusContainer.style.color = isError ? "red" : "black"; | ||
| } | ||
|
|
||
| function createStatusContainer() { | ||
| const container = document.createElement("div"); | ||
| container.id = "status-message"; | ||
| container.style.padding = "1rem"; | ||
| container.style.fontSize = "1.2rem"; | ||
| container.style.textAlign = "center"; | ||
|
|
||
| // Prepend to body or main container | ||
| document.body.prepend(container); | ||
| return container; | ||
| } | ||
|
|
||
|
|
||
| //check the right input from forms and if its ok -> add the new book (object in array) | ||
| //via Book function and start render function | ||
| function submit() { | ||
|
|
||
| // Date : 9/8/2026 | ||
| // Purpose : Fix the problem "3. It uses the title name as the author name" | ||
| // Change : replace the second title with author.value to fix the bug | ||
| let titleValue = titleElement.value.trim(); | ||
| let authorValue = authorElement.value.trim(); | ||
| let pagesValue = pagesElement.value.trim(); | ||
|
|
||
| if ( | ||
| title.value == null || | ||
| title.value == "" || | ||
| pages.value == null || | ||
| pages.value == "" | ||
| titleValue === "" || | ||
| authorValue === "" || | ||
| pagesValue === "" | ||
| ) { | ||
| alert("Please fill all fields!"); | ||
| return false; | ||
| } else { | ||
| let book = new Book(title.value, title.value, pages.value, check.checked); | ||
| library.push(book); | ||
| render(); | ||
| displayStatusMessage("Please fill all fields!", true); | ||
| return; | ||
| } | ||
|
|
||
| let book = new Book(titleValue, authorValue, pagesValue, checkElement.checked); | ||
|
|
||
| myLibrary.push(book); | ||
| render(); | ||
|
|
||
| } | ||
|
|
||
| function Book(title, author, pages, check) { | ||
|
|
@@ -52,52 +79,65 @@ function Book(title, author, pages, check) { | |
|
|
||
| function render() { | ||
| let table = document.getElementById("display"); | ||
| let rowsNumber = table.rows.length; | ||
|
|
||
| //delete old table | ||
| for (let n = rowsNumber - 1; n > 0; n-- { | ||
| table.deleteRow(n); | ||
| } | ||
| //insert updated row and cells | ||
| let length = myLibrary.length; | ||
| for (let i = 0; i < length; i++) { | ||
| let row = table.insertRow(1); | ||
| table.innerHTML = ""; | ||
|
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. You could also clear only |
||
|
|
||
| let thead = document.createElement("thead"); | ||
| thead.className = "thead-dark"; | ||
| thead.innerHTML = ` | ||
| <tr> | ||
| <th>Title</th> | ||
| <th>Author</th> | ||
| <th>Number of Pages</th> | ||
| <th>Read</th> | ||
| <th></th> | ||
| </tr> | ||
| `; | ||
| table.appendChild(thead); | ||
|
|
||
| // Date : 9/8/2026 | ||
| // Purpose : Fix the problem "1. Website loads but doesn't show any books" | ||
| // Change : Missing ) → script stops executing → nothing renders and add back the closing ) to fix it | ||
| for (let i = 0; i < myLibrary.length; i++) { | ||
| let row = table.insertRow(-1); | ||
|
|
||
| let titleCell = row.insertCell(0); | ||
| let authorCell = row.insertCell(1); | ||
| let pagesCell = row.insertCell(2); | ||
| let wasReadCell = row.insertCell(3); | ||
| let deleteCell = row.insertCell(4); | ||
|
|
||
| titleCell.innerHTML = myLibrary[i].title; | ||
| authorCell.innerHTML = myLibrary[i].author; | ||
| pagesCell.innerHTML = myLibrary[i].pages; | ||
|
|
||
| //add and wait for action for read/unread button | ||
| let changeBut = document.createElement("button"); | ||
| changeBut.id = i; | ||
| changeBut.className = "btn btn-success"; | ||
| wasReadCell.appendChild(changeBut); | ||
| let readStatus = ""; | ||
| if (myLibrary[i].check == false) { | ||
| readStatus = "Yes"; | ||
| } else { | ||
| readStatus = "No"; | ||
| } | ||
| changeBut.innerText = readStatus; | ||
|
|
||
| changeBut.addEventListener("click", function () { | ||
|
|
||
| // Date : 9/8/2026 | ||
| // Purpose : Fix the problem "5. When I add a book that I say I've read - it saves the wrong answer" | ||
| // Change : Correct the logic - if check == true, that means the book was read, so it should show Yes. | ||
| changeBut.innerText = myLibrary[i].check ? "Yes" : "No"; | ||
| changeBut.addEventListener("click", () => { | ||
| myLibrary[i].check = !myLibrary[i].check; | ||
| render(); | ||
| }); | ||
| wasReadCell.appendChild(changeBut); | ||
|
|
||
| //add delete button to every row and render again | ||
| let delButton = document.createElement("button"); | ||
| delBut.id = i + 5; | ||
| deleteCell.appendChild(delBut); | ||
| delBut.className = "btn btn-warning"; | ||
| delBut.innerHTML = "Delete"; | ||
| delBut.addEventListener("clicks", function () { | ||
| alert(`You've deleted title: ${myLibrary[i].title}`); | ||
| // Date : 9/8/2026 | ||
| // Purpose : Fix the problem "4. Delete button is broken" | ||
| // Change : Fix the incorrect nanmes "delBut" button and "clicks" event to the correct one "delButton" and "click" respectively | ||
| delButton.className = "btn btn-warning"; | ||
| delButton.innerText = "Delete"; | ||
| delButton.addEventListener("click", () => { | ||
| displayStatusMessage(`You've deleted title: ${myLibrary[i].title}`,false); | ||
| myLibrary.splice(i, 1); | ||
| render(); | ||
| }); | ||
| deleteCell.appendChild(delButton); | ||
| } | ||
| } | ||
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The JS code works to certain extent but it introduces some unusual behavior to the input field. For example, when I pressed '1', '2', '3', 'e', the input field was cleared.
Adding
min="1",step="1"andrequiredwould be enough if the input element is inside a<form>. When the input elements are in a<form>, the constraints would have been checked by the browser when the user submits the form (clicks the "Submit" button).