Skip to content

London | 26-ITP-Jan | Karla Grajales | Sprint 2 | Book Library - #416

Closed
Grajales-K wants to merge 30 commits into
CodeYourFuture:mainfrom
Grajales-K:Book-Library
Closed

London | 26-ITP-Jan | Karla Grajales | Sprint 2 | Book Library #416
Grajales-K wants to merge 30 commits into
CodeYourFuture:mainfrom
Grajales-K:Book-Library

Conversation

@Grajales-K

@Grajales-K Grajales-K commented Apr 9, 2026 •
edited
Loading

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

This project provided a valuable opportunity for debugging; I checked the project and used the devtools to identify and fix the errors.

@Grajales-K
missing for loop brackets on line 57, added
7d232c0
@Grajales-K
changed let for const to create the new button
8836023
@Grajales-K
bug with declaration of variable fixed with delButton
6688f1b
@Grajales-K
fixed bug incorrect name event, clicks instead "click"
d20bada 
and passing as parameter index directly in the event to avoid overlapping, delButton eventListener
@Grajales-K
fixed boolean comparision to false == "No"
86d7c97
@Grajales-K
fix: add raw code
1fa900d
@Grajales-K
fix: correct book title and variable declaration in library management
601d465
@Grajales-K
fix: add missing newline at end of style.css file
3485ff1
@Grajales-K
fix: correct for loop syntax and update delete button variable name
2fcb6a8
@Grajales-K
fix: update title of book library page for clarity
438f376
@Grajales-K
fix: correct library variable name and update delete button splice me…
b290901 
…thod
@Grajales-K
fix: return author value instead title twice
7694e71
@Grajales-K
fix: remove "s" from event listener click
dd289a1
@Grajales-K
fix: correct read status boolean logic in render function
71e5cc0
@Grajales-K
fix: validate author and pages input in submit function
8ad522c
@Grajales-K
fix: validate pages input in submit function
592e239
@github-actions

This comment has been minimized.

Sign in to view
@Grajales-K Grajales-K added Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. 🏕 Priority Mandatory This work is expected 📅 Sprint 2 Assigned during Sprint 2 of this module javascript Pull requests that update javascript code labels Apr 9, 2026
cjyuan
cjyuan reviewed Apr 10, 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.

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.

@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 Apr 10, 2026
Grajales-K and others added 3 commits April 10, 2026 18:04
@Grajales-K
Merge branch 'main' into Book-Library
e167834
@Grajales-K
fix: meta lang missing.
dcf690f 
- remove static <tr> tags, generated by Dom and.

- fix: id in the opening tag <tbody>
@Grajales-K
refactor: change the name var from table to librarybody, removed the …
872bff6 
…for loop delete to clear the table before to render
@Grajales-K Grajales-K mentioned this pull request Apr 14, 2026
Open
@Grajales-K
style: refine form structure and enhance input field attributes
6080c0c
@Grajales-K
style: update button styles and adjust margins for better layout
6b9270d
@Grajales-K
refactor: restructure book submission logic and improve initial libra…
2db7dcb 
…ry population
@Grajales-K
fix: prevent page refresh on book form submission
39a70df
@Grajales-K
style: enhance jumbotron and form styles for improved layout and usab…
1e58499 
…ility
@Grajales-K
style: standardize CSS formatting and enhance button styles for consi…
f4ad102 
…stency
@Grajales-K
style: restructure HTML layout and enhance form input organization fo…
5a7f32d 
…r better usability
@Grajales-K
style: add new button styles and refine existing CSS for improved lay…
5a2831b 
…out and usability
@Grajales-K
refactor: improve book form processing and enhance library rendering …
4d7f4a0 
…logic
@Grajales-K
Grajales-K requested a review from cjyuan April 14, 2026 15:59
@Grajales-K Grajales-K added the Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. label Apr 14, 2026
@Grajales-K

Grajales-K commented Apr 14, 2026

Copy link
Copy Markdown
Author

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.

I have made a lot of changes, and I hope they are better now. Thank you!

cjyuan
cjyuan reviewed Apr 14, 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.

Comment thread debugging/book-library/script.js
Comment on lines +40 to +46
const titleVal = titleInput.value.trim();
const authorVal = authorInput.value.trim();
const pagesVal = pagesInput.value;

if (!titleVal || !authorVal || pagesVal <= 0) {
alert("Please fill the field with valid info!");
return;

@cjyuan cjyuan Apr 14, 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.

Note: pagesInput.value is a string.

Your code can still work correctly, but best practice is to sanitise/normalise the input to proper data type first before using them.

Comment thread debugging/book-library/style.css
Comment on lines +53 to +55
background-color: #117a8b !important;
border-color: #10707f !important;
color: #fff !important;

@cjyuan cjyuan Apr 14, 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.

CSS is designed around specificity and order.
Using !important in CSS is generally considered a last resort, not a standard practice.

Is there any reason you choose to use !important?

@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 Apr 14, 2026
@illicitonion

illicitonion commented May 4, 2026

Copy link
Copy Markdown
Member

Closing PR because the January ITP run has finished. Feel free to re-open if you're still working on it.

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

Assignees

No one assigned

Labels

Complete Volunteer to add when work is complete and all review comments have been addressed. javascript Pull requests that update javascript code 🏕 Priority Mandatory This work is expected 📅 Sprint 2 Assigned during Sprint 2 of this module

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@Grajales-K @illicitonion @cjyuan