London | ITP-May-25 | Emiliano Uruena | Data-Flows Sprint-2 | Book-Library 📚 #253

Emilianouz · 2025-07-29T09:27:28Z

Self checklist

I have committed my files one by one, on purpose, and for a reason
I have titled my PR with Region | Cohort | FirstName LastName | Sprint | Assignment Title
I have tested my changes
My changes follow the style guide
My changes meet the requirements of this task

Changelist

Bug Fixes Implemented in JavaScript and HTML Functionality:

View a list of books I've read:

The display function now correctly renders the full list of books stored in memory or local storage. Any updates to the list—such as adding or removing books—are reflected immediately in the user interface.

Add new books to the list:

Users can now successfully add books they've read to the list. Each book entry includes:

Title
Author
Number of pages
A checkbox to indicate whether the book has been read

All form inputs are now properly validated. If any required field is missing or left empty, the book will not be added, and the system will display an alert message prompting the user to complete all fields. This prevents empty or incomplete entries in the book list.

Remove books from the list:

Each book in the list now includes a "Remove" button. When clicked, the corresponding book is deleted from the list both visually and from the underlying data. The removal process has been debugged to ensure that it consistently targets the correct entry without affecting others.

cjyuan · 2025-08-03T22:01:35Z

debugging/book-library/index.html

According to https://validator.w3.org/, there are errors in your index.html. Can you fix these errors?

You're right: Element title must not be empty. Duplicate ID title. Duplicate ID author.Duplicate ID check. Sorted!

According to https://validator.w3.org/, there are still errors in your index.html.

Apologies, I’ve reviewed and resolved it.

cjyuan · 2025-08-03T22:03:07Z

debugging/book-library/script.js

  if (
    title.value == null ||
    title.value == "" ||
+    author.value == null ||
+    author.value == "" ||
    pages.value == null ||
    pages.value == ""


In terms of input validation,

Can .value be null? (Do we need to check someInputElement.value == null?)

Do you want to allow book title and author name to contain only space characters?

What if a user enters an invalid page number in the "pages" input field?

Mmm... I didn't think that. Now, I added trim() to check the spaces on title and author. Also, a validator for number of pages.

debugging/book-library/script.js

cjyuan · 2025-08-03T22:28:51Z

debugging/book-library/script.js

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


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.

Thank you for the advice. Sorted.

- Using user input with validation. - Arguments are pre-processed/sanitize before calling the constructor; removing leading/trailing whitespace - rows are inserted inside the <tbody>, not the <thead>

Emilianouz

@cjyuan Thank you for the feedback on my JavaScript PR; it was very constructive.
Issues solved according to the advices:

Using user input with validation.
Arguments are pre-processed/sanitize before calling the constructor; removing leading/trailing whitespace
rows are inserted inside the , not the

Emilianouz · 2025-08-07T15:43:48Z

debugging/book-library/index.html

You're right: Element title must not be empty. Duplicate ID title. Duplicate ID author.Duplicate ID check. Sorted!

Emilianouz · 2025-08-07T16:30:14Z

debugging/book-library/script.js

  if (
    title.value == null ||
    title.value == "" ||
+    author.value == null ||
+    author.value == "" ||
    pages.value == null ||
    pages.value == ""


Mmm... I didn't think that. Now, I added trim() to check the spaces on title and author. Also, a validator for number of pages.

debugging/book-library/script.js

Emilianouz · 2025-08-07T18:52:38Z

debugging/book-library/script.js

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


Thank you for the advice. Sorted.

debugging/book-library/script.js

cjyuan

Changes in script.js look good.

index.html still have errors (according to W3C validator).

cjyuan · 2025-08-07T23:13:14Z

debugging/book-library/index.html

According to https://validator.w3.org/, there are still errors in your index.html.

- Attribute type updated in input for author and title - Charset and name in separated tags. - Empty rows on tbody removed. - html tag with language attribute. On script: Variable to store name of book to show the name after it is deleted.

Emilianouz

It were reviewed and resolved.

Emilianouz · 2025-08-08T10:17:57Z

debugging/book-library/index.html

Apologies, I’ve reviewed and resolved it.

Bugs were fixed in scripts and html.

4847d35

Emilianouz added the Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. label Jul 29, 2025

cjyuan reviewed Aug 3, 2025

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 3, 2025

Issues solved according to the feedback.

bf2388c

- Using user input with validation. - Arguments are pre-processed/sanitize before calling the constructor; removing leading/trailing whitespace - rows are inserted inside the <tbody>, not the <thead>

Emilianouz commented Aug 7, 2025

View reviewed changes

Emilianouz added the Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. label Aug 7, 2025

cjyuan reviewed Aug 7, 2025

View reviewed changes

debugging/book-library/index.html Outdated

Copy link

Contributor

cjyuan Aug 7, 2025

Choose a reason for hiding this comment

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

According to https://validator.w3.org/, there are still errors in your index.html.

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

Errors in html fixed:

06a8a08

- Attribute type updated in input for author and title - Charset and name in separated tags. - Empty rows on tbody removed. - html tag with language attribute. On script: Variable to store name of book to show the name after it is deleted.

Emilianouz commented Aug 8, 2025

View reviewed changes

debugging/book-library/index.html Outdated

Copy link

Author

Emilianouz Aug 8, 2025

Choose a reason for hiding this comment

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

Apologies, I’ve reviewed and resolved it.

Emilianouz 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 8, 2025

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

Uh oh!

London | ITP-May-25 | Emiliano Uruena | Data-Flows Sprint-2 | Book-Library 📚 #253

Are you sure you want to change the base?

London | ITP-May-25 | Emiliano Uruena | Data-Flows Sprint-2 | Book-Library 📚 #253

Conversation

Emilianouz commented Jul 29, 2025

Changelist

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!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Emilianouz left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

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!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

cjyuan left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Emilianouz left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Emilianouz left a comment •

edited

Loading