Skip to content

Conversation

rarityXtreme
Copy link

@rarityXtreme rarityXtreme commented Aug 13, 2025

Learners, PR Template

Self checklist

  • I have committed my files one by one, on purpose, and for a reason
  • I have titled my PR with Region | Cohort | FirstName LastName | Sprint | Assignment Title
  • I have tested my changes
  • My changes follow the style guide
  • My changes meet the requirements of this task

Changelist

Briefly explain your PR.

This PR updates the My Book Library project to fix bugs and improve code readability.

  • Corrected form validation to require title, author, and a positive page number
  • Fixed incorrect use of title as author when adding a book
  • Fixed broken delete button and read/unread toggle
  • Updated DOM rendering to be safer and clearer
  • Cleaned up HTML structure and input types
  • Removed placeholder table row and used a proper button for submit
  • Left CSS as-is since it’s functional and meets current needs

The project now loads initial books, allows adding/removing books, and correctly updates read status.

Questions

Ask any questions you have for your reviewer.

Copy link

Your PR's title isn't in the expected format.

Please check its title is in the correct format, and update it.

Reason: Sprint part (Sprint -2) doesn't match expected format (example: 'Sprint 2', without quotes)

@rarityXtreme rarityXtreme added the Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. label Aug 13, 2025
Copy link

Your PR's title isn't in the expected format.

Please check its title is in the correct format, and update it.

Reason: Sprint part (Sprint -2) doesn't match expected format (example: 'Sprint 2', without quotes)

1 similar comment
Copy link

Your PR's title isn't in the expected format.

Please check its title is in the correct format, and update it.

Reason: Sprint part (Sprint -2) doesn't match expected format (example: 'Sprint 2', without quotes)

@rarityXtreme rarityXtreme changed the title London | 25-ITP-May | Hendrine Zeraua| Sprint -2 | Data -Flows / Book-Library London | 25-ITP-May | Hendrine Zeraul | Sprint-2 | Data-Flows / Book-Library Aug 13, 2025
Copy link

Your PR's title isn't in the expected format.

Please check its title is in the correct format, and update it.

Reason: Sprint part (Sprint-2) doesn't match expected format (example: 'Sprint 2', without quotes)

@rarityXtreme rarityXtreme changed the title London | 25-ITP-May | Hendrine Zeraul | Sprint-2 | Data-Flows / Book-Library London | 25-ITP-May | Hendrine Zeraul | Data-Flows Sprint-2 | Book-Library Aug 13, 2025
Copy link

Your PR's title isn't in the expected format.

Please check its title is in the correct format, and update it.

Reason: Sprint part (Data-Flows Sprint-2) doesn't match expected format (example: 'Sprint 2', without quotes)

@rarityXtreme rarityXtreme changed the title London | 25-ITP-May | Hendrine Zeraul | Data-Flows Sprint-2 | Book-Library London | 25-ITP-May | Hendrine Zeraua | Data-Flows -Sprint-2 | Book-Library Aug 13, 2025
Copy link

Your PR's title isn't in the expected format.

Please check its title is in the correct format, and update it.

Reason: Sprint part (Data-Flows -Sprint-2) doesn't match expected format (example: 'Sprint 2', without quotes)

@rarityXtreme rarityXtreme changed the title London | 25-ITP-May | Hendrine Zeraua | Data-Flows -Sprint-2 | Book-Library London | ITP- May 2025 | Hendrine Zeraua | Sprint-2 | Book-Library Aug 13, 2025
Copy link

Your PR's title isn't in the expected format.

Please check its title is in the correct format, and update it.

Reason: Sprint part (Sprint-2) doesn't match expected format (example: 'Sprint 2', without quotes)

@rarityXtreme rarityXtreme changed the title London | ITP- May 2025 | Hendrine Zeraua | Sprint-2 | Book-Library London | ITP- May 2025 | Hendrine Zeraua | Sprint 2 | Book-Library Aug 13, 2025
@rarityXtreme rarityXtreme changed the title London | ITP- May 2025 | Hendrine Zeraua | Sprint 2 | Book-Library London | May 2025 | Hendrine Zeraua | Sprint 2 | Book Library Aug 13, 2025
@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 Aug 14, 2025
@rarityXtreme
Copy link
Author

@cjyuan
Hi CJ,

I’ve addressed all points as follows:

  • Indentation – Fixed indentation in populateStorage() and ensured consistent formatting across the file.
  • Input normalization – Trimmed and stored inputs (titleInput, authorInput, pagesInput) in variables and reused them throughout submit() to avoid repeated DOM access and raw input use.
  • Validation – Pages are validated as a positive integer before adding a book.
  • Efficient table clearing – Replaced the row-by-row deletion loop with table.querySelector("tbody").innerHTML = "" to clear rows in one operation.
  • Button IDs – Removed unnecessary id attributes from action buttons to avoid non-unique IDs.
  • Delete flow – Changed the delete button handler to delete the book first, then display the alert message to confirm the completed action.

Thanks again for taking the time to review — your suggestions helped improve both performance and code clarity.

@rarityXtreme rarityXtreme added the Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. label Aug 14, 2025
@cjyuan
Copy link
Contributor

cjyuan commented Aug 14, 2025

Changes look good. Well done.

@cjyuan cjyuan added Complete Volunteer to add when work is complete and all review comments have been addressed. and removed Reviewed Volunteer to add when completing a review with trainee action still to take. Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. labels Aug 14, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Complete Volunteer to add when work is complete and all review comments have been addressed.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants