generated from CodeYourFuture/Module-Template
-
-
Notifications
You must be signed in to change notification settings - Fork 162
West Midlands | 25 Sep ITP | Iswat Bello | Sprint 2 | Book library #349
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
Closed
Closed
Changes from 19 commits
Commits
Show all changes
23 commits
Select commit
Hold shift + click to select a range
e2bf444
Fix syntax error in for-loop (line 57) that blocked book list from re…
Iswanna c39e375
Fix: rename delButton to delBut and correct event to 'click' to enabl…
Iswanna c794174
Rename library to myLibrary in submit() so new books are added to the…
Iswanna 436136b
fix: use author.value for author field when creating a new Book (line…
Iswanna dccb8a5
fix(render): correct read status display so checked=true shows “Yes” …
Iswanna 07c6620
Remove redundant render() call in populateStorage (line 19) to avoid …
Iswanna 085a373
Remove placeholder table row <tr> from <tbody> to allow render() to m…
Iswanna 4bf7b31
Fix invalid HTML input types by changing <input type="title"/> and <i…
Iswanna d9870cf
Switch script.js to type="module", remove inline onclick="submit()", …
Iswanna 3acd315
Add id="submitBtn" to the submit button to enable attaching an event …
Iswanna dae6c85
Refactor code to remove redundant null checks on input values, ensure…
Iswanna a6619ba
Refactor variable names for clarity: DOM inputs now use descriptive n…
Iswanna d413566
fix(js): add full input validation for author and page count
Iswanna c784153
refactor(js): optimize table clearing using tbody.innerHTML
Iswanna 4757d2f
Clear rows via tbody.innerHTML to preserve header and speed up re-render
Iswanna 0a4c4ae
ux(delete): confirm before deletion and show non-blocking success alert
Iswanna 757fcd3
Correct book title spelling from “Robison Crusoe” to “Robinson Crusoe…
Iswanna 2b98534
Fix(form): prevent default submit to avoid page reload on Submit butt…
Iswanna 63652a8
Chore(html): add lang, split meta tags, set meaningful title, and mak…
Iswanna 7c28cc2
Add #alerts container and move the dependency scripts to end of the body
Iswanna 9526485
Fix indentation inconsistency in index.html
Iswanna 9c3c39f
refactor(notices): use #alerts container with CSS-only auto-hide; rem…
Iswanna 2417ca4
fix(validation): require pages to be a positive integer
Iswanna 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
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 |
|---|---|---|
|
|
@@ -3,44 +3,77 @@ let myLibrary = []; | |
| window.addEventListener("load", function (e) { | ||
| populateStorage(); | ||
| render(); | ||
|
|
||
| // Attach submit listener (prevent default form submission) | ||
| const submitBtn = document.getElementById("submitBtn"); | ||
| if (submitBtn) { | ||
| submitBtn.addEventListener("click", function (evt) { | ||
| evt.preventDefault(); | ||
| submit(); | ||
| }); | ||
| } | ||
| }); | ||
|
|
||
| function populateStorage() { | ||
| if (myLibrary.length == 0) { | ||
| let book1 = new Book("Robison Crusoe", "Daniel Defoe", "252", true); | ||
| let book1 = new Book("Robinson Crusoe", "Daniel Defoe", 252, true); | ||
| let book2 = new Book( | ||
| "The Old Man and the Sea", | ||
| "Ernest Hemingway", | ||
| "127", | ||
| 127, | ||
| true | ||
| ); | ||
| myLibrary.push(book1); | ||
| myLibrary.push(book2); | ||
| render(); | ||
| } | ||
| } | ||
|
|
||
| const title = document.getElementById("title"); | ||
| const author = document.getElementById("author"); | ||
| const pages = document.getElementById("pages"); | ||
| const check = document.getElementById("check"); | ||
| const titleInput = document.getElementById("title"); | ||
| const authorInput = document.getElementById("author"); | ||
| const pagesInput = document.getElementById("pages"); | ||
| const readCheckbox = document.getElementById("check"); | ||
|
|
||
| // Non-blocking success message (appears briefly and fades out) | ||
| function showSuccess(message) { | ||
| const alertEl = document.createElement("div"); | ||
| alertEl.className = "alert alert-success"; | ||
| alertEl.textContent = message; | ||
| alertEl.style.position = "fixed"; | ||
| alertEl.style.top = "1rem"; | ||
| alertEl.style.right = "1rem"; | ||
| alertEl.style.zIndex = "1050"; | ||
| document.body.appendChild(alertEl); | ||
|
|
||
| setTimeout(() => { | ||
| alertEl.style.transition = "opacity 0.3s"; | ||
| alertEl.style.opacity = "0"; | ||
| setTimeout(() => alertEl.remove(), 300); | ||
| }, 2500); | ||
| } | ||
|
|
||
| //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() { | ||
| if ( | ||
| title.value == null || | ||
| title.value == "" || | ||
| pages.value == null || | ||
| pages.value == "" | ||
| ) { | ||
| alert("Please fill all fields!"); | ||
| return false; | ||
| } else { | ||
| let book = new Book(title.value, title.value, pages.value, check.checked); | ||
| library.push(book); | ||
| render(); | ||
| const title = titleInput.value.trim(); | ||
| const author = authorInput.value.trim(); | ||
| const pages = Number(pagesInput.value); | ||
|
|
||
| // Validate title and author | ||
| if (title === "" || author === "") { | ||
| alert("Title and author cannot be empty."); | ||
| return; | ||
| } | ||
|
|
||
| // Validate pages | ||
| if (Number.isNaN(pages) || pages <= 0) { | ||
|
||
| alert("Pages must be a positive number."); | ||
| return; | ||
| } | ||
|
|
||
| // Create and add book | ||
| let book = new Book(title, author, pages, readCheckbox.checked); | ||
| myLibrary.push(book); | ||
| render(); | ||
| } | ||
|
|
||
| function Book(title, author, pages, check) { | ||
|
|
@@ -51,53 +84,48 @@ 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; | ||
| const table = document.getElementById("display"); | ||
| const tbody = table.tBodies[0] || table.createTBody(); | ||
|
|
||
| // Clear all data rows while preserving the header | ||
| tbody.innerHTML = ""; | ||
|
|
||
| const length = myLibrary.length; | ||
| for (let i = 0; i < 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; | ||
| const book = myLibrary[i]; | ||
| const row = tbody.insertRow(); | ||
|
|
||
| const titleCell = row.insertCell(0); | ||
| const authorCell = row.insertCell(1); | ||
| const pagesCell = row.insertCell(2); | ||
| const wasReadCell = row.insertCell(3); | ||
| const deleteCell = row.insertCell(4); | ||
|
|
||
| titleCell.textContent = book.title; | ||
| authorCell.textContent = book.author; | ||
| pagesCell.textContent = book.pages; | ||
|
|
||
| // Read/unread toggle button | ||
| const changeBut = document.createElement("button"); | ||
| changeBut.className = "btn btn-success"; | ||
| changeBut.textContent = book.check ? "Yes" : "No"; | ||
| changeBut.addEventListener("click", function () { | ||
| 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); | ||
| // Delete button (confirm before deleting, then show non-blocking success) | ||
| const delBut = document.createElement("button"); | ||
| delBut.className = "btn btn-warning"; | ||
| delBut.innerHTML = "Delete"; | ||
| delBut.addEventListener("clicks", function () { | ||
| alert(`You've deleted title: ${myLibrary[i].title}`); | ||
| myLibrary.splice(i, 1); | ||
| render(); | ||
| delBut.textContent = "Delete"; | ||
| delBut.addEventListener("click", function () { | ||
| if (confirm(`Are you sure you want to delete "${book.title}"?`)) { | ||
| myLibrary.splice(i, 1); | ||
| render(); | ||
| showSuccess(`Deleted "${book.title}"`); | ||
| } | ||
| }); | ||
| deleteCell.appendChild(delBut); | ||
| } | ||
| } | ||
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.
(Optional)
Could consider designating an element in
index.htmlto display the message. This way, we can have more control where to position the element.Separation of concern -- We should avoid specifying CSS styles in JS or in HTML file. (Assigning CSS class is ok)
It is possible to create the effect of "making the foreground visible for a fixed amount of time" using only CSS. And then in JS we can just add/remove CSS class to trigger the animation without using
setTimeout().Uh oh!
There was an error while loading. Please reload this page.
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.
Thank you for the suggestion. I’ve made the updates to the code based on your feedback.