-
-
Notifications
You must be signed in to change notification settings - Fork 236
Sheffield| 26-ITP-Jan| Mona- Eltantawy | Sprint 2 | Book library #472
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
Changes from all commits
9bfbf87
3ba261d
fec510d
5b6c13a
6677db9
9109674
bc9b694
adaf7fc
0ba3876
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 |
|---|---|---|
| @@ -1,96 +1,80 @@ | ||
| <!DOCTYPE html> | ||
| <html> | ||
| <head> | ||
| <title> </title> | ||
| <meta | ||
| charset="utf-8" | ||
| name="viewport" | ||
| content="width=device-width, initial-scale=1.0" | ||
| /> | ||
| <script src="https://ajax.googleapis.com/ajax/libs/jquery/3.4.1/jquery.min.js"></script> | ||
| <script src="https://cdnjs.cloudflare.com/ajax/libs/popper.js/1.16.0/umd/popper.min.js"></script> | ||
| <script src="https://maxcdn.bootstrapcdn.com/bootstrap/4.4.1/js/bootstrap.min.js"></script> | ||
| <link | ||
| rel="stylesheet" | ||
| href="https://maxcdn.bootstrapcdn.com/bootstrap/4.4.1/css/bootstrap.min.css" | ||
| /> | ||
| <link rel="stylesheet" type="text/css" href="style.css" /> | ||
| </head> | ||
|
|
||
| <body> | ||
| <div class="jumbotron text-center"> | ||
| <h1>Library</h1> | ||
| <p>Add books to your virtual library</p> | ||
| </div> | ||
| <html lang="en"> | ||
|
|
||
| <head> | ||
| <title>Book Library</title> | ||
| <meta charset="utf-8"> | ||
| <meta name="viewport" content="width=device-width, initial-scale=1.0"> | ||
|
|
||
| <!-- Bootstrap --> | ||
| <link rel="stylesheet" href="https://maxcdn.bootstrapcdn.com/bootstrap/4.4.1/css/bootstrap.min.css"> | ||
|
|
||
| <!-- Optional JS (needed for collapse button) --> | ||
| <script src="https://ajax.googleapis.com/ajax/libs/jquery/3.4.1/jquery.min.js"></script> | ||
| <script src="https://cdnjs.cloudflare.com/ajax/libs/popper.js/1.16.0/umd/popper.min.js"></script> | ||
| <script src="https://maxcdn.bootstrapcdn.com/bootstrap/4.4.1/js/bootstrap.min.js"></script> | ||
|
|
||
| <link rel="stylesheet" href="style.css"> | ||
| </head> | ||
|
|
||
| <body> | ||
| <div class="jumbotron text-center"> | ||
| <h1>Library</h1> | ||
| <p>Add books to your virtual library</p> | ||
| </div> | ||
|
|
||
| <!-- Toggle form --> | ||
| <div class="text-center mb-3"> | ||
| <button data-toggle="collapse" data-target="#demo" class="btn btn-info"> | ||
| Add new book | ||
| </button> | ||
| </div> | ||
|
|
||
| <!-- ✅ FORM added --> | ||
| <div id="demo" class="collapse"> | ||
| <form id="bookForm" class="form-group"> | ||
|
|
||
| <label for="title">Title:</label> | ||
| <input type="text" class="form-control" id="title" required minlength="2" maxlength="100"> | ||
|
|
||
| <label for="author">Author:</label> | ||
| <input type="text" class="form-control" id="author" required minlength="2" maxlength="100"> | ||
|
|
||
| <div id="demo" class="collapse"> | ||
| <div class="form-group"> | ||
| <label for="title">Title:</label> | ||
| <input | ||
| type="title" | ||
| class="form-control" | ||
| id="title" | ||
| name="title" | ||
| required | ||
| /> | ||
| <label for="author">Author: </label> | ||
| <input | ||
| type="author" | ||
| class="form-control" | ||
| id="author" | ||
| name="author" | ||
| required | ||
| /> | ||
| <label for="pages">Pages:</label> | ||
| <input | ||
| type="number" | ||
| class="form-control" | ||
| id="pages" | ||
| name="pages" | ||
| required | ||
| /> | ||
| <label class="form-check-label"> | ||
| <input | ||
| type="checkbox" | ||
| class="form-check-input" | ||
| id="check" | ||
| value="" | ||
| />Read | ||
| </label> | ||
| <input | ||
| type="submit" | ||
| value="Submit" | ||
| class="btn btn-primary" | ||
| onclick="submit();" | ||
| /> | ||
| <label for="pages">Pages:</label> | ||
| <input type="number" class="form-control" id="pages" required min="1" max="9999"> | ||
|
|
||
| <div class="form-check mt-2 mb-3"> | ||
| <input type="checkbox" class="form-check-input" id="check"> | ||
| <label class="form-check-label" for="check">Read</label> | ||
| </div> | ||
| </div> | ||
|
|
||
| <table class="table" id="display"> | ||
| <thead class="thead-dark"> | ||
| <tr> | ||
| <th>Title</th> | ||
| <th>Author</th> | ||
| <th>Number of Pages</th> | ||
| <th>Read</th> | ||
| <th></th> | ||
| </tr> | ||
| </thead> | ||
| <tbody> | ||
| <tr> | ||
| <td></td> | ||
| <td></td> | ||
| <td></td> | ||
| <td></td> | ||
| <td></td> | ||
| </tr> | ||
| </tbody> | ||
| </table> | ||
|
|
||
| <script src="script.js"></script> | ||
| </body> | ||
| </html> | ||
|
|
||
| <!-- ✅ FIXED BUTTON --> | ||
| <button type="submit" class="btn btn-primary"> | ||
| Add Book | ||
| </button> | ||
|
|
||
| </form> | ||
| </div> | ||
|
|
||
| <!-- Message --> | ||
| <div id="message" class="text-center mt-2 text-success"></div> | ||
|
|
||
| <!-- Table --> | ||
| <table class="table mt-3" id="display"> | ||
| <thead class="thead-dark"> | ||
| <tr> | ||
| <th>Title</th> | ||
| <th>Author</th> | ||
| <th>Number of Pages</th> | ||
| <th>Read</th> | ||
| <th>Delete</th> | ||
| </tr> | ||
| </thead> | ||
| <tbody></tbody> | ||
| </table> | ||
|
|
||
| <!-- ✅ ES module (no inline JS anywhere) --> | ||
| <script src="script.js" type="module"></script> | ||
| </body> | ||
|
|
||
| </html> | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,103 +1,128 @@ | ||
| let myLibrary = []; | ||
| // ✅ Data | ||
| const myLibrary = []; | ||
|
|
||
| window.addEventListener("load", function (e) { | ||
| populateStorage(); | ||
| // ✅ DOM elements (clear naming) | ||
| const formEl = document.getElementById("bookForm"); | ||
| const titleInputEl = document.getElementById("title"); | ||
| const authorInputEl = document.getElementById("author"); | ||
| const pagesInputEl = document.getElementById("pages"); | ||
| const checkInputEl = document.getElementById("check"); | ||
| const tableBodyEl = document.querySelector("#display tbody"); | ||
| const messageEl = document.getElementById("message"); | ||
|
|
||
| // ✅ Init | ||
| window.addEventListener("load", () => { | ||
| populateStorage(); // only called once | ||
| render(); | ||
| }); | ||
|
|
||
| // ✅ Populate initial data | ||
| 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 | ||
| if (myLibrary.length === 0) { | ||
| myLibrary.push( | ||
| new Book("Robinson Crusoe", "Daniel Defoe", 252, true), | ||
| new Book("The Old Man and the Sea", "Ernest Hemingway", 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"); | ||
|
|
||
| //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(); | ||
| // ✅ Handle form submit (NO inline onclick) | ||
| formEl.addEventListener("submit", (e) => { | ||
| e.preventDefault(); | ||
|
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. What is the purpose of this statement (line 31)?
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. this line helped sorting the issue of missing the data submitted to the form with every page reload that was one of the issues I struggled with
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. That's not what |
||
|
|
||
| // 🔹 Preprocessing | ||
| const title = titleInputEl.value.trim(); | ||
| const author = authorInputEl.value.trim(); | ||
| const pages = Number(pagesInputEl.value); | ||
| const isRead = checkInputEl.checked; | ||
|
|
||
| // 🔹 Validation | ||
| if (!title || !author) { | ||
| showMessage("Title and Author cannot be empty."); | ||
| return; | ||
| } | ||
| } | ||
|
|
||
| function Book(title, author, pages, check) { | ||
| if (Number.isNaN(pages) || pages < 1 || pages > 9999) { | ||
| showMessage("Pages must be between 1 and 9999."); | ||
| return; | ||
| } | ||
|
|
||
| // 🔹 Add book | ||
| const book = new Book(title, author, pages, isRead); | ||
| myLibrary.push(book); | ||
|
|
||
| // 🔹 Reset form | ||
| formEl.reset(); | ||
|
|
||
| render(); | ||
| }); | ||
|
|
||
| // ✅ Constructor | ||
| function Book(title, author, pages, isRead) { | ||
| this.title = title; | ||
| this.author = author; | ||
| this.pages = pages; | ||
| this.check = check; | ||
| this.pages = pages; // number (correct type) | ||
| this.isRead = isRead; | ||
| } | ||
|
|
||
| // ✅ Render table | ||
| 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); | ||
| 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 () { | ||
| myLibrary[i].check = !myLibrary[i].check; | ||
| // 🔹 Efficient clear | ||
| tableBodyEl.innerHTML = ""; | ||
|
|
||
| myLibrary.forEach((book, index) => { | ||
| const row = document.createElement("tr"); | ||
|
|
||
| const titleCell = document.createElement("td"); | ||
| const authorCell = document.createElement("td"); | ||
| const pagesCell = document.createElement("td"); | ||
| const readCell = document.createElement("td"); | ||
| const deleteCell = document.createElement("td"); | ||
|
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. Why use this approach instead of the original approach to create table cell? 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);
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. actually I used the original one and fixed it in my first edit to the code and when I passed it to an AI tool for review it fixed it to this one and I considered it when no issues came out in validation. I understand that there are differences between the two approaches but I considered the validation impact.
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. In you opinion, which approach is better, and why?
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. for me the document .create element the original one and this because it's the one I'm confident to use right now actually but I'm afraid to change it to affect the validation again |
||
|
|
||
| // 🔹 Safe text assignment | ||
| titleCell.textContent = book.title; | ||
| authorCell.textContent = book.author; | ||
| pagesCell.textContent = book.pages; | ||
|
|
||
| // 🔹 Toggle read button (simplified) | ||
| const toggleBtn = document.createElement("button"); | ||
| toggleBtn.className = "btn btn-success"; | ||
| toggleBtn.textContent = book.isRead ? "Yes" : "No"; | ||
|
|
||
| toggleBtn.addEventListener("click", () => { | ||
| book.isRead = !book.isRead; | ||
| render(); | ||
| }); | ||
|
|
||
| //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}`); | ||
| myLibrary.splice(i, 1); | ||
| readCell.appendChild(toggleBtn); | ||
|
|
||
| // 🔹 Delete button | ||
| const deleteBtn = document.createElement("button"); | ||
| deleteBtn.className = "btn btn-warning"; | ||
| deleteBtn.textContent = "Delete"; | ||
|
|
||
| deleteBtn.addEventListener("click", () => { | ||
| // delete first | ||
| const deletedTitle = book.title; | ||
| myLibrary.splice(index, 1); | ||
|
|
||
| // then show message (non-blocking) | ||
| showMessage(`Deleted: "${deletedTitle}"`); | ||
| render(); | ||
| }); | ||
| } | ||
|
|
||
| deleteCell.appendChild(deleteBtn); | ||
|
|
||
| row.append(titleCell, authorCell, pagesCell, readCell, deleteCell); | ||
| tableBodyEl.appendChild(row); | ||
| }); | ||
| } | ||
|
|
||
| // ✅ Non-blocking message (instead of alert) | ||
| function showMessage(text) { | ||
| messageEl.textContent = text; | ||
|
|
||
| setTimeout(() => { | ||
| messageEl.textContent = ""; | ||
| }, 2000); | ||
| } | ||
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.
What does class
mt-2andmb-3do?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.
they are mainly styling and doesn't affect the javascript but it makes the style for the checkbox looks clear. as the mt-2 add space above the dive and prevents it from sticking too close to the page input.
Also mt-3 add space blow that section to separate it from the submit button.
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.
Can you look it up ?