Skip to content

London | 26-ITP-May | Dagim Daniel | Sprint 2 | book library - #538

Open
Dagim-Daniel wants to merge 4 commits into
CodeYourFuture:mainfrom
Dagim-Daniel:Sprint-2/bookLibrary
Open

London | 26-ITP-May | Dagim Daniel | Sprint 2 | book library#538
Dagim-Daniel wants to merge 4 commits into
CodeYourFuture:mainfrom
Dagim-Daniel:Sprint-2/bookLibrary

Conversation

@Dagim-Daniel

@Dagim-Daniel Dagim-Daniel commented Aug 12, 2026

Copy link
Copy Markdown

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

fixing major bugs of the book library project

@Dagim-Daniel Dagim-Daniel added 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 12, 2026
cjyuan
cjyuan reviewed Aug 12, 2026
View reviewed changes

@cjyuan cjyuan left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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.

Comment thread debugging/book-library/index.html Outdated
Comment on lines 49 to 55

@cjyuan cjyuan Aug 12, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

  • Even though this element has a required attribute, the user could still "submit" the input the even if this input field is empty. Could you find out why and then address the issue?

  • If possible, also make this input field to accept only positive whole numbers.

Comment thread debugging/book-library/index.html Outdated
Comment on lines 57 to 62

@cjyuan cjyuan Aug 12, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The checkbox is not showing.

The issue is related to Bootstrap 4.4.1. Could you use AI to find a way to fix the issue? Mentioning "Bootstrap 4.4.1" might help.

Comment thread debugging/book-library/index.html Outdated
value="Submit"
class="btn btn-primary"
onclick="submit();"
onclick="submit()"

@cjyuan cjyuan Aug 12, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Could you research the trade-off between

  1. Attaching event listener in HTML
  2. Attaching event listener using addEventListener() in JS

@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 12, 2026
@Dagim-Daniel Dagim-Daniel added the Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. label Aug 16, 2026
cjyuan
cjyuan reviewed Aug 16, 2026
View reviewed changes
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 Outdated
Comment thread debugging/book-library/script.js Outdated
Comment thread debugging/book-library/script.js Outdated
Comment thread debugging/book-library/script.js Outdated
@cjyuan cjyuan removed the Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. label Aug 16, 2026
@Dagim-Daniel Dagim-Daniel added the Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. label Aug 17, 2026
cjyuan
cjyuan reviewed Aug 17, 2026
View reviewed changes

@cjyuan cjyuan left a comment

Copy link
Copy Markdown
Contributor

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.

@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 17, 2026
@Dagim-Daniel Dagim-Daniel mentioned this pull request Aug 18, 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.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@Dagim-Daniel @cjyuan