Skip to content

Manchester | 26-ITP-Jan | Khalidbih | Sprint 2| Book Library#456

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

Manchester | 26-ITP-Jan | Khalidbih | Sprint 2| Book Library#456
khalidbih wants to merge 2 commits intoCodeYourFuture:mainfrom
khalidbih:book-library

Conversation

@khalidbih
Copy link
Copy Markdown

@khalidbih khalidbih commented Apr 19, 2026 •
edited
Loading

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 :

         Fixed bugs in the Book Library app and improved input validation.
         Also updated HTML to pass validation and cleaned up the code.

@khalidbih khalidbih added 📅 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 Apr 19, 2026
cjyuan
cjyuan reviewed Apr 19, 2026
View reviewed changes
Comment thread debugging/book-library/index.html
Comment thread debugging/book-library/script.js Outdated
Comment thread debugging/book-library/script.js Outdated
Comment thread debugging/book-library/script.js Outdated
Comment thread debugging/book-library/script.js
Comment thread debugging/book-library/script.js Outdated
Comment thread debugging/book-library/script.js Outdated
@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 19, 2026
@khalidbih khalidbih 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 20, 2026
cjyuan
cjyuan reviewed Apr 20, 2026
View reviewed changes
Copy link
Copy Markdown
Contributor

@cjyuan cjyuan left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Changes look good and thanks for clearly responding to each comment. Well done.

Comment thread debugging/book-library/script.js
const trimmedTitle = titleInput.value.trim();
const trimmedAuthor = authorInput.value.trim();

const trimmedPages = Number(pagesInput.value);
Copy link
Copy Markdown
Contributor

@cjyuan cjyuan Apr 20, 2026

Choose a reason for hiding this comment

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

The name trimmedPages tends to suggest its value is a string.

Copy link
Copy Markdown
Author

@khalidbih khalidbih Apr 20, 2026

Choose a reason for hiding this comment

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

The name trimmedPages tends to suggest its value is a string.

Thanks for the feedback, I’ll keep that in mind.

@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 20, 2026
@khalidbih
Copy link
Copy Markdown
Author

khalidbih commented Apr 20, 2026

Changes look good, and thanks for clearly responding to each comment. Well done.

Thank you. I appreciate the feedback, it helps a lot, and you’re welcome.

@khalidbih khalidbih mentioned this pull request Apr 20, 2026
Open
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. Module-Data-Flows The name of the module. 📅 Sprint 2 Assigned during Sprint 2 of this module

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@khalidbih @cjyuan