London | 25-ITP-SEP | Imran Mohamed | Sprint 2 | Book Library #329

i786m · 2025-11-26T09:38:42Z

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

Refactor HTML structure, enhance form validation, and improve JavaScript functionality for book library & complete code reading tasks

…ipt functionality for book library application & complete code reading tasks

github-actions · 2025-11-26T09:38:56Z

Your PR description contained template fields which weren't filled in.

Check you've ticked everything in the self checklist, and that any sections which prompt you to fill in an answer are either filled in or removed.

If this PR is not coursework, please add the NotCoursework label (and message on Slack in #cyf-curriculum or it will probably not be noticed).

If this PR needs reviewed, please add the 'Needs Review' label to this PR after you have resolved the issues listed above.

github-actions · 2025-11-26T09:39:04Z

Your PR description contained template fields which weren't filled in.

Check you've ticked everything in the self checklist, and that any sections which prompt you to fill in an answer are either filled in or removed.

If this PR is not coursework, please add the NotCoursework label (and message on Slack in #cyf-curriculum or it will probably not be noticed).

If this PR needs reviewed, please add the 'Needs Review' label to this PR after you have resolved the issues listed above.

cjyuan

Can you check if any of this general feedback can help you further improve your code?
https://github.com/cjyuan/Module-Data-Flows/blob/book-library-feedback/debugging/book-library/feedback.md

Doing so can help me speed up the review process. Thanks.

i786m · 2025-11-27T22:48:43Z

Thank you for the feedback. I'll make sure to work on the issues/suggestions you've pointed out.

Copilot

Pull request overview

This PR refactors the book library application by fixing multiple bugs, improving form validation, enhancing HTML semantics, and completing code reading exercises. The changes address issues with book display, form submission, author attribution, and delete functionality.

Key changes:

Fixed JavaScript bugs including incorrect variable references, missing parentheses, and incorrect event listener names
Improved HTML structure with proper input types, accessibility attributes, and semantic elements
Enhanced form handling by moving to button-based submission with proper event listeners

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 8 comments.

Show a summary per file

File	Description
debugging/book-library/script.js	Fixed multiple bugs including syntax errors, variable references, and improved code quality with proper comparisons and string interpolation
debugging/book-library/index.html	Enhanced semantic HTML with lang attribute, proper input types, accessibility labels, and form structure
debugging/book-library/style.css	Updated selectors to match HTML changes and improved styling consistency
debugging/code-reading/readme.md	Added answers to scope and reference-related JavaScript questions
debugging/book-library/readme.md	Documented completion of bug fixes

debugging/book-library/style.css

debugging/book-library/script.js

debugging/code-reading/readme.md

debugging/book-library/script.js

cjyuan

Excellent work.

cjyuan · 2025-12-09T03:56:56Z

debugging/code-reading/readme.md

+First console.log outputs 9 as x is passed and the f1 call doesn't alter its value outside the fn.
+Second console.log prints the object {x:10} as f2 increments y.x by 1


Suggestion: Look up the relevant technical terms, pass by value and pass by reference.

cjyuan · 2025-12-09T04:12:46Z

debugging/book-library/script.js

+  authorInput.value = "";
+  pagesInput.value = "";
+  checkInput.checked = false;
+  alert(`You've added ${book.title} to your library.`);


Note:

alert() is not a friendly way to display messages in an app, especially non-critical messages.
It is ok to use alert() in this "exercise" app, just don't make it a habit.

Also, alert() can block the execution of the application. For example, even if you moved alert0 after render(), the rendering will still be blocked by alert(). So in both "addition" and "deletion" (lines 97-99), the updated list of books will only be rendered after the user closed the dialog box.

I was looking into Bootstrap's alert classes and realised I could have in-page messages as opposed to alerts. I will implement this as a workaround.

Refactor HTML structure, enhance form validation, and improve JavaScr…

fee43ee

…ipt functionality for book library application & complete code reading tasks

i786m added the Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. label Nov 26, 2025

github-actions bot removed the Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. label Nov 26, 2025

i786m added the Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. label Nov 26, 2025

cjyuan reviewed Nov 27, 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 Nov 27, 2025

Merge branch 'CodeYourFuture:main' into coursework/booklibrary

2f35ce1

Copilot AI review requested due to automatic review settings December 6, 2025 11:18

Copilot started reviewing on behalf of i786m December 6, 2025 11:19 View session

Copilot AI reviewed Dec 6, 2025

View reviewed changes

i786m added 16 commits December 8, 2025 10:44

fix meta tag errors in html

2648f54

update script tag to use module type

bcf8c09

fix ID for add book button in event listener

f95fbc6

use querySelector to target form tag

9304166

convert pages value to a number when adding a book

df6e09d

change variable names for input elements to reflect what they store

778d81d

fix typo in book1

aea2a43

normalize, sanitize and validate data on input

58798b5

refactor to delete rows in one operation

2a58ad6

fix delete message timing issue

8ac46e3

use textContent instead of innerText for rendering book details

5691196

improve button naming and clarity

f6a4060

fix page numbers in populateStorage function to use numbers

d9acf45

refactor addBook and render functions

c6bf51d

use textContent instead of innerText for buttons

e6143cb

removed trailing slash on void elements

ea49aa2

i786m added 6 commits December 8, 2025 19:38

remove empty table row in book display

f2a5fcf

refactor render to protect against null tbody

620b93c

format button label for better readability and update button styles

0b76c33

update button colors for accessibility

d349fa1

tidy up

cbcd993

add button color for improved visibility

7f03371

i786m 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 Dec 8, 2025

i786m requested a review from cjyuan December 8, 2025 20:27

cjyuan reviewed Dec 9, 2025

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 Dec 9, 2025

i786m mentioned this pull request Dec 9, 2025

ITP-SEP-25| Data Flows | Imran Mohamed i786m/My-Coursework-Planner#92

Open

illicitonion closed this Jan 7, 2026

		First console.log outputs 9 as x is passed and the f1 call doesn't alter its value outside the fn.
		Second console.log prints the object {x:10} as f2 increments y.x by 1

Uh oh!

London | 25-ITP-SEP | Imran Mohamed | Sprint 2 | Book Library #329

London | 25-ITP-SEP | Imran Mohamed | Sprint 2 | Book Library #329

Uh oh!

Conversation

i786m commented Nov 26, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Changelist

Uh oh!

github-actions bot commented Nov 26, 2025

Uh oh!

github-actions bot commented Nov 26, 2025

Uh oh!

cjyuan left a comment

Choose a reason for hiding this comment

Uh oh!

i786m commented Nov 27, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

cjyuan left a comment

Choose a reason for hiding this comment

Uh oh!

cjyuan Dec 9, 2025

Choose a reason for hiding this comment

Uh oh!

cjyuan Dec 9, 2025

Choose a reason for hiding this comment

Uh oh!

i786m Dec 9, 2025

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

i786m commented Nov 26, 2025 •

edited

Loading