ZA Cape Town | ITP-May-2025 | Dawud Vermeulen | Sprint 2 | Book-Library by Dave-Vermeulen · Pull Request #308 · CodeYourFuture/Module-Data-Flows

Dave-Vermeulen · 2025-08-16T04:12:25Z

Learners, PR Template

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

Fixed the errors in the code. some were small some were bigger like the submit() function had problems with author being set to title so changed to author value. Then went on to the render() function and fixed the delButton use. A 3rd bug was on the delete buttons event listener there was a typo. Then finally the if statement logic in the render function was reversed so fixed that.

Questions

Please review my PR 🙏🏽

github-actions · 2025-08-16T04:12:43Z

Your PR couldn't be matched to an assignment in this module.

Please check its title is in the correct format, and that you only have one PR per assignment.

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

github-actions · 2025-08-16T04:14:51Z

Your PR couldn't be matched to an assignment in this module.

Please check its title is in the correct format, and that you only have one PR per assignment.

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

cjyuan · 2025-08-16T09:11:53Z

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.

I think the PR title is not quite in the expected format.

github-actions · 2025-08-16T09:12:23Z

Your PR couldn't be matched to an assignment in this module.

Please check its title is in the correct format, and that you only have one PR per assignment.

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

github-actions · 2025-08-16T09:12:25Z

Your PR couldn't be matched to an assignment in this module.

Please check its title is in the correct format, and that you only have one PR per assignment.

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

Dave-Vermeulen · 2025-08-18T11:59:45Z

@cjyuan no mention in the feedback.md doc about any PR naming. I updated to try make it easier. Thanks for sharing the feedback.md. I have used it to update the code making it more DRY and less redundant. Kindly let me know if you have questions or any further suggestions.

LonMcGregor

Hi, I've left some comments that should help improve your solution further. Well done on getting this far, you're close to being finished

LonMcGregor · 2025-08-20T08:18:49Z

debugging/book-library/index.html

There are some things missing from your HTML file that should be added. Hint: how people are going to interact with your webpage in a web browser?

There is also some invalid HTML. Can you spot these? Hint: have you used an HTML validator before? https://validator.w3.org/

@LonMcGregor this was actually a lot. Really useful site. So have update the file, please review and let me know if more is required.

LonMcGregor · 2025-08-20T08:21:39Z

debugging/book-library/script.js

-    title.value == "" ||
-    pages.value == null ||
-    pages.value == ""
+    titleInput.value.trim == "" || // cant be null so removed all that


There may be ways to improve your form validation here. Things to think about:

Is your validation checking all the inputs?

Is there a way to get the browser to do some of the checking for you, without needing to manually check for empty inputs?

@LonMcGregor ah very DRY indeed. cool HTML5 tricks. have updated both files to handle the validation and submission a bit differently.

LonMcGregor · 2025-08-20T08:23:25Z

debugging/book-library/script.js

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


Think about how deleting a file works on your desktop. Is being notified after the deletion normally how this works? Is there a better interaction you could use than an alert here?

@LonMcGregor okay so this is bit higher grade. i think what you mean is that the users flow is interrupted when the alert pops up (clarify if im wrong kanala). based on this assumption and the fact that the delete works i have built a new function to display the notification and done away with the disruptive alert. The new one disappears after 3 secs.

The alert is quite interrupting, yes, making it less "in your face" is one solution. The other is, if you really want something interrupting, to use something like confirm instead.

It sounds like you've made good progress here. I can't see the files yet on github though - have you comitted them all?

LonMcGregor · 2025-08-20T15:13:30Z

Good work updating the HTML file. Can you also have a look at the form validation comment I made - that would be the minimum needed to fix to complete this sprint task.

At minimum, could you ensure that invalid page values are checked properly?

LonMcGregor · 2025-08-20T15:56:45Z

OK. Good to see the final version. I am happy with what you have finished with here. Well done, you are complete with this sprint

Dave-Vermeulen added 3 commits August 16, 2025 05:17

fixed submit but still not working. debug further

3c360ff

the render function was the issue.

afae57d

corrected logic in if statement for render function

07d1bd8

Dave-Vermeulen added the Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. label Aug 16, 2025

Dave-Vermeulen mentioned this pull request Aug 16, 2025

ZA-Cape Town | Data flows | Dawud Vermeulen Dave-Vermeulen/My-Coursework-Planner#107

Open

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

tidy up the script.js following the feedback.md shared on PR

5699d2c

Dave-Vermeulen added the Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. label Aug 20, 2025

LonMcGregor added the Review in progress This review is currently being reviewed. This label will be replaced by "Reviewed" soon. label Aug 20, 2025

LonMcGregor reviewed Aug 20, 2025

View reviewed changes

LonMcGregor removed Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. Review in progress This review is currently being reviewed. This label will be replaced by "Reviewed" soon. labels Aug 20, 2025

use validator to tidy up index.html

6980aeb

updating code to accommodate PR.

3dd9e9a

LonMcGregor 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. labels Aug 20, 2025

illicitonion closed this Sep 1, 2025

Uh oh!

Conversation

Dave-Vermeulen commented Aug 16, 2025

Learners, PR Template

Changelist

Questions

Uh oh!

github-actions bot commented Aug 16, 2025

Uh oh!

github-actions bot commented Aug 16, 2025

Uh oh!

cjyuan commented Aug 16, 2025

Uh oh!

github-actions bot commented Aug 16, 2025

Uh oh!

github-actions bot commented Aug 16, 2025

Uh oh!

Dave-Vermeulen commented Aug 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

LonMcGregor left a comment

Choose a reason for hiding this comment

Uh oh!

LonMcGregor Aug 20, 2025

Choose a reason for hiding this comment

Uh oh!

Dave-Vermeulen Aug 20, 2025

Choose a reason for hiding this comment

Uh oh!

LonMcGregor Aug 20, 2025

Choose a reason for hiding this comment

Uh oh!

Dave-Vermeulen Aug 20, 2025 • 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!

LonMcGregor Aug 20, 2025

Choose a reason for hiding this comment

Uh oh!

Dave-Vermeulen Aug 20, 2025 • 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!

LonMcGregor Aug 20, 2025

Choose a reason for hiding this comment

Uh oh!

LonMcGregor commented Aug 20, 2025

Uh oh!

LonMcGregor commented Aug 20, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Dave-Vermeulen commented Aug 18, 2025 •

edited

Loading

Dave-Vermeulen Aug 20, 2025 •

edited

Loading

Dave-Vermeulen Aug 20, 2025 •

edited

Loading