Skip to content

London | 26-ITP-January | Eugenie Ahangama | Sprint 2 | Book Library#434

Open
Eugenie-A wants to merge 4 commits intoCodeYourFuture:mainfrom
Eugenie-A:book-library
Open

London | 26-ITP-January | Eugenie Ahangama | Sprint 2 | Book Library#434
Eugenie-A wants to merge 4 commits intoCodeYourFuture:mainfrom
Eugenie-A:book-library

Conversation

@Eugenie-A
Copy link
Copy Markdown

@Eugenie-A Eugenie-A commented Apr 14, 2026

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

index.html:

  • Fixed empty page title

script.js:

  • Fixed syntax error in for loop that prevented any books from displaying on load
  • Fixed library reference to myLibrary so new books are added correctly
  • Fixed author field using title value instead of author value
  • Fixed delete button variable name mismatch and wrong event name "clicks" to "click"
  • Fixed inverted read status logic so "Yes" and "No" display correctly
  • Added author field validation so books can't be added without an author

Questions

N/A

@Eugenie-A Eugenie-A added 🏕 Priority Mandatory This work is expected 🐂 Size Medium 1-4 hours 📅 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. Submit:PR Module-Data-Flows The name of the module. Core This is a core task and should be completed by all trainees labels Apr 14, 2026
@Eugenie-A Eugenie-A mentioned this pull request Apr 14, 2026
Open
cjyuan
cjyuan reviewed Apr 16, 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.

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 16, 2026
@Eugenie-A
Copy link
Copy Markdown
Author

Eugenie-A commented Apr 16, 2026

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.

Hi, I've gone through the feedback and addressed all the points raised.

@Eugenie-A Eugenie-A 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 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.

Changes look good. Well done.

Comment thread debugging/book-library/script.js
@@ -1,103 +1,107 @@
let myLibrary = [];
Copy link
Copy Markdown
Contributor

@cjyuan cjyuan Apr 17, 2026

Choose a reason for hiding this comment

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

Could declare this using const.

Comment thread debugging/book-library/script.js
Comment on lines +15 to 19
window.addEventListener("load", function () {
// Removed extra render() call
// — populateStorage() already calls it internally
populateStorage();
render();
});
Copy link
Copy Markdown
Contributor

@cjyuan cjyuan Apr 17, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

Could also place the code that sets up event listener on page load (code on line 13) here.

@cjyuan 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 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

Complete Volunteer to add when work is complete and all review comments have been addressed. Core This is a core task and should be completed by all trainees Module-Data-Flows The name of the module. 🏕 Priority Mandatory This work is expected 🐂 Size Medium 1-4 hours 📅 Sprint 2 Assigned during Sprint 2 of this module Submit:PR

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@Eugenie-A @cjyuan