London | 26-ITP-May | Edina Kurdi | Sprint 2 | Book Library by edinakurdi · Pull Request #552 · CodeYourFuture/Module-Data-Flows

edinakurdi · 2026-08-14T13:20:29Z

Learners, PR Template

Self checklist

I have titled my PR with Region | Cohort | FirstName LastName | Sprint | Assignment Title
My changes meet the requirements of the task
I have tested my changes
My changes follow the style guide

Changelist

Added the missing closing bracket in the render() function's loop
Corrected the array name to myLibrary and fixed the event listener to use click
Fixed the variable mapping in render()
Fixed variable name mismatch. (delButton)
Fixed the submit() function so it saves the actual author's name instead of copying the title twice. (Note - in the commit msg I may have accidentally said this was in render())
Fixed the read (y/n) display logic for new table entries

Other corrections I made:
Added missing author form validation, fixed HTML input types, corrected a book title typo, and removed redundant CSS.

cjyuan

Could you also check if any of this general feedback can help you further improve your code?
https://github.com/CodeYourFuture/Module-Data-Flows/blob/general-review-feedback/debugging/book-library/feedback.md

Doing so can help me speed up the review process. Thanks.

cjyuan · 2026-08-14T22:54:27Z

+      <label class="form-check-label">
+        <input type="checkbox" class="form-check-input" id="check" value="" />Read
+      </label>


If this checkbox is not showing, could you try fixing the issue with the help of AI. The issue is related to Bootstrap 4.4.1. So mentioning "Bootstrap 4.4.1" to AI might help.

cjyuan · 2026-08-14T22:55:38Z

+      <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();" />


Could you look up the trade-off between

attaching event listener in HTML

attaching event listener in JS using .addEventListener()?

inline breaks separation of concersn and doesnt allow multiple event listeneres, and forces the function to global scope which can cause issues, but it is quick to write, shorter
addEventListener supports multiple event listeners, no global scope, but requires more code.
updated

condense pushing the books to the library into a single statement

-clear book-table-body with innerHTML -append rows in order -replace innerHTML with textContent for title, author, pand ages

-rename buttons to toggleReadBtn and deleteBtn, -use textContent for button text, -add confirm() to check if user wants to delete a book

cjyuan

Changes look good.

Well done.

cjyuan · 2026-08-16T18:20:16Z

-    let book = new Book(title.value, title.value, pages.value, check.checked);
-    library.push(book);
-    render();
+bookForm.addEventListener("submit", function (e) {


Could consider placing all code that runs once on page load in a single function. For example, you could put it inside the page load callback or create a function named init() or setup() and call it once when the page loads.

This makes it easier to locate and manage all the code that runs once when the app starts.

cjyuan · 2026-08-16T18:29:02Z

+      const confirmDel = confirm(
+        `Are you sure you want to delete "${myLibrary[i].title}"?`
+      );
+      if (confirmDel) {
+        myLibrary.splice(i, 1);
+        render();
+      }
    });


Informing the user that the book has been deleted is not quite the same as asking whether they want to delete the book though.

edinakurdi added 11 commits August 14, 2026 10:06

complete missing closing ')' in render() for loop

1e412fd

correct typo in book title: Robinson Crusoe

25d43db

keep delButton consistently named

626dc3d

correct event listener event name to click

11cc372

add title in html

8da7a71

fix variable name in submit() from library to myLibrary

db875d7

fix title being used as an author (still in render())

9e96d3e

add missing author validation

c121439

fix new book read? (y/n) display in table

b3ed0a2

fix type input to be text for title and author in HTML

8dd9945

remove redundant code from css

276fa65

edinakurdi added 🏕 Priority Mandatory This work is expected 📅 Sprint 2 Assigned during Sprint 2 of this module Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. Module-Data-Flows The name of the module. labels Aug 14, 2026

cjyuan reviewed Aug 14, 2026

View reviewed changes

cjyuan added Reviewed Volunteer to add when completing a review with trainee action still to take. and removed Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. labels Aug 14, 2026

edinakurdi added 12 commits August 15, 2026 14:43

add language in html

af09d6d

separate out meta tag

4faebe8

remove extra rneder() in populateStorage()

fe78e54

add min attribute to book pages

fed4503

rename variables to more descriptive names

6ded0c0

add type attribute to add new book button

202b520

wrap inputs in form element

bc7961b

convert input to submit element

e3f87fd

add header for actions in table

7509788

delete emplty table rows and add id to tbody

f325171

add type=module to script tag

a61281f

fix: move button attributes inside the opening tag for submit button

96c8779

edinakurdi added 13 commits August 16, 2026 14:49

fix: add text on submit button: 'Submit'

07a64a1

fix: remove value from button element

06ea771

update populateStorgage() to take number page values &

e93ea32

condense pushing the books to the library into a single statement

rename form input variables in submit function

596c6f5

remove unnecessary null checks for string inputs

624dc39

grab book-form and save it to variable bookForm

a003460

replace submit fnction with form submit event listener

3cf13da

refactor render()

b84a871

-clear book-table-body with innerHTML -append rows in order -replace innerHTML with textContent for title, author, pand ages

-remove duplicate button ids,

769936a

-rename buttons to toggleReadBtn and deleteBtn, -use textContent for button text, -add confirm() to check if user wants to delete a book

change let to const where possible

61a0126

enforce valid integer for pages in html and js

e997b5e

correct typo and ensure book-table-body matches the js DOM reference

2cade97

fix typos

03993a0

edinakurdi added Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. and removed Reviewed Volunteer to add when completing a review with trainee action still to take. labels Aug 16, 2026

cjyuan reviewed Aug 16, 2026

View reviewed changes

cjyuan added Complete Volunteer to add when work is complete and all review comments have been addressed. and removed Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. labels Aug 16, 2026

edinakurdi mentioned this pull request Aug 18, 2026

ITP-May-2026 | Data flow | Edina_Kurdi edinakurdi/My-Coursework-Planner#151

Open

9 tasks

Navigation Menu

Uh oh!

Uh oh!

London | 26-ITP-May | Edina Kurdi | Sprint 2 | Book Library - #552

London | 26-ITP-May | Edina Kurdi | Sprint 2 | Book Library#552
edinakurdi wants to merge 36 commits into
CodeYourFuture:mainfrom
edinakurdi:sprint-2/book-library

edinakurdi commented Aug 14, 2026

Uh oh!

cjyuan left a comment

Uh oh!

Uh oh!

cjyuan Aug 14, 2026

Uh oh!

cjyuan Aug 14, 2026

Uh oh!

edinakurdi Aug 16, 2026

Uh oh!

cjyuan left a comment

Uh oh!

cjyuan Aug 16, 2026

Uh oh!

cjyuan Aug 16, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Uh oh!

Conversation

edinakurdi commented Aug 14, 2026

Learners, PR Template

Changelist

Uh oh!

cjyuan left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

cjyuan Aug 14, 2026

Choose a reason for hiding this comment

Uh oh!

cjyuan Aug 14, 2026

Choose a reason for hiding this comment

Uh oh!

edinakurdi Aug 16, 2026

Choose a reason for hiding this comment

Uh oh!

cjyuan left a comment

Choose a reason for hiding this comment

Uh oh!

cjyuan Aug 16, 2026

Choose a reason for hiding this comment

Uh oh!

cjyuan Aug 16, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants