Skip to content

Sheffield | 26-ITP-Jan | Mahmoud Shaabo | Sprint 2 | Book Library#450

Open
mahmoudshaabo1984 wants to merge 2 commits intoCodeYourFuture:mainfrom
mahmoudshaabo1984:book-library-mahmoud
Open

Sheffield | 26-ITP-Jan | Mahmoud Shaabo | Sprint 2 | Book Library#450
mahmoudshaabo1984 wants to merge 2 commits intoCodeYourFuture:mainfrom
mahmoudshaabo1984:book-library-mahmoud

Conversation

@mahmoudshaabo1984
Copy link
Copy Markdown

@mahmoudshaabo1984 mahmoudshaabo1984 commented Apr 16, 2026

What bugs did I fix?

  • Bug 1 - Website loads but doesn't show any books
    The for loop in the render function was missing a closing parenthesis ) before the {. This caused a syntax error that stopped all JavaScript from running.

  • Bug 2 - Error in console when you try to add a book
    The submit function was calling library.push(book) but the array is named myLibrary. Fixed by using the correct variable name. Also added validation for the author field.

  • Bug 3 - It uses the title name as the author name
    In the submit function, title.value was used twice. The second argument should be author.value.

  • Bug 4 - Delete button is broken
    The delete button variable was created as delButton but then referenced as delBut three times. Also the event name was "clicks" instead of "click". Fixed both issues.

  • Bug 5 - Read status saves the wrong answer
    The if condition was inverted. check == false was showing "Yes". Fixed so check == true correctly shows "Yes".

How to test

  1. Open index.html in the browser
  2. Two books appear on load — both show "Yes" in the Read column
  3. Click "Add new book" and fill in the form — the new book appears in the table
  4. Click the "Yes"/"No" button on any book — it toggles correctly
  5. Click "Delete" on any book — the book is removed from the table
  6. Try submitting the form with empty fields — an alert appears

Hi @cifarquhar — I have fixed all 5 bugs listed in the readme. Please let me know if you have any feedback. Thank you!

@github-actions

This comment has been minimized.

Sign in to view
@mahmoudshaabo1984
Copy link
Copy Markdown
Author

mahmoudshaabo1984 commented Apr 16, 2026

Hi @cifarquhar — PR #450 is ready for review.

I have fixed all 5 bugs listed in the readme.md file. The app now loads correctly, books can be added and deleted, and the read status toggles as expected.

Please let me know if anything needs to be changed. Thank you!

@mahmoudshaabo1984 mahmoudshaabo1984 added the Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. label Apr 16, 2026
@github-actions

This comment has been minimized.

Sign in to view
@github-actions github-actions bot removed the Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. label Apr 16, 2026
@mahmoudshaabo1984 mahmoudshaabo1984 added the Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. label Apr 16, 2026
@github-actions

This comment has been minimized.

Sign in to view
@github-actions github-actions bot removed the Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. label Apr 16, 2026
@mahmoudshaabo1984 mahmoudshaabo1984 changed the title Sheffield Jan 2026 | Book Library | Mahmoud Shaabo Sheffield | 26-ITP-Jan | Mahmoud Shaabo | Sprint 2 | Book Library Apr 16, 2026
@mahmoudshaabo1984 mahmoudshaabo1984 added the Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. label Apr 16, 2026
cjyuan
cjyuan reviewed Apr 17, 2026
View reviewed changes
Copy link
Copy Markdown
Contributor

@cjyuan cjyuan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You have fixed all the bugs, but there are still plenty of room you can improve the code.

Can you 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 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 Apr 17, 2026
@mahmoudshaabo1984
Copy link
Copy Markdown
Author

mahmoudshaabo1984 commented Apr 17, 2026

Hi CJ,

Thank you for your feedback. I have reviewed your comments and applied the following improvements:

script.js

  • Changed let to const for all DOM references that never change
  • Renamed confusing variables: check → hasBeenRead, title → titleInput, author → authorInput
  • Renamed submit() to addBook() to avoid conflict with the browser's built-in submit behaviour
  • Replaced innerHTML with textContent for all user-provided data to prevent XSS vulnerabilities
  • Renamed populateStorage() to addDefaultBooks() — the old name implied localStorage usage
  • Extracted createReadButton() and createDeleteButton() as separate helper functions to make render() easier to read
  • Added form field clearing after a book is successfully added

index.html

  • Added lang="en" to the <html> element for accessibility
  • Added a proper page title: "My Book Library"
  • Fixed invalid input types: type="title" and type="author" changed to type="text"

Please let me know if there is anything else to improve.

Thank you,
Mahmoud Shaabo

@mahmoudshaabo1984 mahmoudshaabo1984 added the Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. label Apr 17, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@cjyuan cjyuan cjyuan left review comments

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. Reviewed Volunteer to add when completing a review with trainee action still to take.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@mahmoudshaabo1984 @cjyuan