London | 26-ITP-May | Vito Moratti | Sprint 2 | Book-library by vmoratti · Pull Request #570 · CodeYourFuture/Module-Data-Flows

vmoratti · 2026-08-20T16:40:36Z

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

I fixed all the "bugs" and also run it through the Lighthouse tool for accessibility with 94% success.

…licks to click

cjyuan

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.

cjyuan · 2026-08-21T09:22:19Z

          id="title"
          name="title"
          required
        />


The browser checks the input elements against the specified constraints only when a user submits a form.
Without <form>, the browser won't enforce the "required" constraint.

I have addressed that

cjyuan · 2026-08-21T09:23:02Z

Why not make this input element to accept only positive whole number?

cjyuan · 2026-08-21T09:23:45Z

This 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.

I have also run the HTML code through the validator and there are no red flags now.
I also added delay for the deletion message to show after the deletion of book.

cjyuan · 2026-08-21T12:56:00Z

A common practice is to exclude local configuration files, which typically differ between developers, from a PR branch unless the configuration is required for the changes to work.

To ignore a tracked file such as .vscode/settings.json locally, use:

git update-index --skip-worktree path/to/file

And if you ever need undo the operation in order to track it again, use:

git update-index --no-skip-worktree path/to/file

thank you, that file snuck in into commit, and i wasn't sure if it's needed to be pushed. So, does it mean that by doing:
git update-index --no-skip-worktree path/to/file
i have undone it now?

That only tells git not to track the file but the changes made before will still remain on the branch.

For this PR, it's ok to leave it there. You could practice that in future PRs.

cjyuan · 2026-08-21T13:03:17Z

+    titleInput.value.trim(),
+    authorInput.value.trim(),


What f titleInput.value is " "? Could a book title or author's name be an empty string?

I got it. trimming() before the condition checking

cjyuan · 2026-08-21T13:04:55Z

      setTimeout(function () {
        alert(`You've deleted title: ${deletedTitle}`);
-      }, 0.5);
+      }, 3);


Why 3 (or 0.5) ? What difference does it make?

Sorry, of course it's milliseconds. I've changed it to 300 now.

Not much , as it's in milliseconds. So, i have changed now.

cjyuan

Changes look good.

cjyuan · 2026-08-21T15:08:32Z

-    authorInput.value.trim(),
+    title,
+    author,
    Number(pagesInput.value),


Note: It's a safer practice to also check the input in JS. (Someone could have accidently changed the constraints in the HTML later).

cjyuan · 2026-08-21T15:11:31Z

-      setTimeout(function () {
-        alert(`You've deleted title: ${deletedTitle}`);
-      }, 3);
+      alert(`You've deleted title: ${deletedTitle}`);


Why remove the setTimeout()? You could set the delay to 0 (instead of some magic number). Because of the way JS queues a task with setTimeout(), it could prevent alert() from blocking the page from being updated.

vmoratti added 8 commits August 20, 2026 15:29

fix syntax error on line 57

ee2cff9

on line 41 not 'library', 'myLibrary', and 'title.value' is repeated

6bed9db

deleteButton was inconsistent variable, change to deleteBut, change c…

44d9dea

…licks to click

add <title> , and lang='en'

a963df2

fix <meta>, add extra <meta>

3df5e43

add defer to sccript links

50a35c6

add main tag

08dd57c

add author validation conditions,

2073970

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

vmoratti mentioned this pull request Aug 20, 2026

May-ITP-26 | Data Flows | Vito Moratti vmoratti/My-Coursework-Planner#88

Open

change delete button details

8ad6617

cjyuan reviewed Aug 21, 2026

View reviewed changes

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

vmoratti added 8 commits August 21, 2026 12:49

Refactor HTML structure and improve form layout

e6bd736

Change book management and some forms

1bb9eae

refactor beek-library

718d34c

add styling

16396b5

make changes after run through html validator and after general feedback

ec7bc67

just done merge as something gone wrong

2f8d0cb

second attempt

1b1390e

refactor book handling, and deletion delay

7560fea

vmoratti 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

vmoratti added 2 commits August 21, 2026 15:09

change myLibrary variarle as constant to prevent reassignement

029a35b

addressed some issues

415eb6b

vmoratti 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 Reviewed Volunteer to add when completing a review with trainee action still to take. label Aug 21, 2026

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

Uh oh!

Uh oh!

Conversation

vmoratti commented Aug 20, 2026

Changelist

Uh oh!

cjyuan left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cjyuan left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants