London | 26-ITP-May | Mandip Sanger | Sprint 2 | Book Library by mandipsanger · Pull Request #569 · CodeYourFuture/Module-Data-Flows

mandipsanger · 2026-08-20T10:55:09Z

Self cheklist

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
Sprint 2 Data flows book library

cjyuan

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.

mandipsanger · 2026-08-21T11:33:39Z

My Program is working now but failing checks Validate PR Metadata workflow run.

mandipsanger · 2026-08-21T11:39:48Z

its showing Validate PR Metadata
2s
Run args=("/tmp/pr-metadata-validator")

thread 'main' (2352) panicked at src/bin/pr-metadata-validator.rs:74:6:
Failed to validate PR: UserFacing("Failed to parse issue #571 - no submit label.\n\nIf this issue was made my a curriculum team member it should be given a sprint label.\nIf this issue was created by a trainee for step submission, it should probably be closed (and they should create the issue in their fork).")
note: run with RUST_BACKTRACE=1 environment variable to display a backtrace
Error: Process completed with exit code 101.
do you have any idea

cjyuan · 2026-08-21T12:45:07Z

@@ -25,22 +21,44 @@ const author = document.getElementById("author");
 const pages = document.getElementById("pages");
 const check = document.getElementById("check");


Could you name these variables to emphasise that they are input elements?

cjyuan · 2026-08-21T12:47:21Z

+  for (let n = rowsNumber - 1; n > 0; n--) {
    table.deleteRow(n);
  }


Could you look up a more efficient approach (than deleting table rows one by one) to clear the <tbody> part of a table?

cjyuan · 2026-08-21T12:48:54Z

      alert(`You've deleted title: ${myLibrary[i].title}`);
+
      myLibrary.splice(i, 1);
      render();


The alert message is shown before the book is actually deleted; the deletion only occurs after the alert dialog is dismissed. This introduces a risk that the operation may not complete (e.g., if the user closes the browser before dismissing the alert).

In general, it’s better to display a confirmation message only after the associated operation has successfully completed.

alert() is a blocking function call. As a result, invoking it prevents the browser from updating the UI until the dialog is dismissed.

If time permits, research for approaches that allows the UI to update before displaying the alert dialog. (This is an optional change).

cjyuan · 2026-08-21T16:00:04Z

If you have made any changes, you need to push the commits to GitHub.

mandipsanger · 2026-08-21T16:30:45Z

i tried my best. I think i am tired now

cjyuan

Changes look good.

cjyuan · 2026-08-21T17:16:27Z

+  const pageNumber = Number(pagesValue);
+
+  // Check that pages is greater than zero
+  if (pageNumber <= 0) {


Note: Could also consider using Number.isInteger() on line 54 instead of using regular expression on line 45.

mandipsanger added 3 commits August 20, 2026 10:30

book lilbrary fix bug

97c6d21

book library debugging

16fa264

fix changes

3013701

This comment has been minimized.

Sign in to view

This comment has been minimized.

Sign in to view

adding file

597ab64

mandipsanger added the Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. label Aug 20, 2026

This comment has been minimized.

Sign in to view

github-actions Bot removed the Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. label Aug 20, 2026

mandipsanger added the Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. label Aug 20, 2026

This comment has been minimized.

Sign in to view

github-actions Bot removed the Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. label Aug 20, 2026

mandipsanger added 3 commits August 20, 2026 18:11

adding relevant files

291e346

adding files

8bf9a93

adding title to html

269f218

cjyuan reviewed Aug 20, 2026

View reviewed changes

cjyuan added the Reviewed Volunteer to add when completing a review with trainee action still to take. label Aug 20, 2026

fixing validation issue

e686370

mandipsanger added the Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. label Aug 21, 2026

cjyuan reviewed Aug 21, 2026

View reviewed changes

cjyuan removed the Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. label Aug 21, 2026

mandipsanger 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 Aug 21, 2026

mandipsanger removed the Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. label Aug 21, 2026

response to feedback

10d0aba

mandipsanger added the Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. label Aug 21, 2026

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 21, 2026

changes

030e4f9

mandipsanger 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 Aug 21, 2026

cjyuan reviewed Aug 21, 2026

View reviewed changes

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 Aug 21, 2026

mandipsanger mentioned this pull request Aug 21, 2026

May - 2026 ITP | Data flows | Mandip sanger mandipsanger/My-Coursework-Planner#131

Open

		@@ -25,22 +21,44 @@ const author = document.getElementById("author");
		const pages = document.getElementById("pages");
		const check = document.getElementById("check");

Uh oh!

Uh oh!

Conversation

mandipsanger commented Aug 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

cjyuan left a comment

Choose a reason for hiding this comment

Uh oh!

mandipsanger commented Aug 21, 2026

Uh oh!

mandipsanger commented Aug 21, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

cjyuan Aug 21, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

cjyuan Aug 21, 2026

Choose a reason for hiding this comment

Uh oh!

cjyuan Aug 21, 2026

Choose a reason for hiding this comment

Uh oh!

cjyuan commented Aug 21, 2026

Uh oh!

mandipsanger commented Aug 21, 2026

Uh oh!

cjyuan left a comment

Choose a reason for hiding this comment

Uh oh!

cjyuan Aug 21, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

mandipsanger commented Aug 20, 2026 •

edited

Loading