Skip to content

Conversation

@hibosharif202504
Copy link

@hibosharif202504 hibosharif202504 commented Jul 25, 2025

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

  • I have implemented the populateTodoList() function to loop through the given list of todos. For each todo, I created a list element and added the appropriate text and action icons. The function now dynamically adds each todo item to the page, matching the structure in index.html.
  • I added a button that allows users to delete all completed ToDos with a single click. When clicked, the button triggers a function that loops through the list of ToDos and removes any that are marked as completed. The undo button makes it easy for users to clean up their list and keep it organised.
  • I also implemented an Undo button that restores the most recently deleted completed ToDos. When a user clicks "Delete All Completed," the removed items are stored temporarily. Clicking "Undo" then brings them back to the list. This feature adds flexibility and prevents the accidental loss of essential Todo items.

Questions

Ask any questions you have for your reviewer.

@hibosharif202504 hibosharif202504 added the Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. label Jul 25, 2025
@MattGoodwin0 MattGoodwin0 added Review in progress This review is currently being reviewed. This label will be replaced by "Reviewed" soon. and removed Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. labels Aug 5, 2025
@MattGoodwin0 MattGoodwin0 self-requested a review August 6, 2025 14:15
@MattGoodwin0
Copy link

Really great submission! Very well organised and hitting the brief well - a few comments to take the code to the next level!


<div class=" container">
<h1> Todo List</h1>
<form class=""add-todo-form>

Choose a reason for hiding this comment

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

This is a small issue on the form element

Copy link
Author

Choose a reason for hiding this comment

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

Thank you for the review @MattGoodwin0 . I have made the change.

<input type="text" id="new-todo-input" placeholder="New todo..." />
<button type="submit" class="add-btn">Add Todo</button>
</form>
<button onclick="deleteAllCompletedTodos()" class="delete-all-btn">Delete All Completed</button>

Choose a reason for hiding this comment

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

These two button implement their interaction functionality differently - is there a reason for this?
One uses the class and the other onclick.

event.preventDefault();
// Write your code here... and remember to reset the input field to be blank after creating a todo!

let input =

Choose a reason for hiding this comment

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

With the HTML fix you can probably look to simplify this.

Copy link
Author

Choose a reason for hiding this comment

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

Hi @MattGoodwin0 ,

I have updated the code.

todos.forEach((todo, index) => {
// create the todo elements
const listItem = document.createElement("li"); // create list item
listItem.className = todo.completed ? "todo-item completed" : "todo-item";

Choose a reason for hiding this comment

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

Can you think of a way we could reduce this logic?

Choose a reason for hiding this comment

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

Could a compound selector be used here to reduce the javascript code.

Let me know if this isn't part of the sprint!

listItem.className = todo.completed ? "todo-item completed" : "todo-item";

let todoText = document.createElement("span");
todoText.className = todo.completed ? "todo-text completed" : "todo-text";

Choose a reason for hiding this comment

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

same as above - Can you think of a way we could reduce this logic?

Choose a reason for hiding this comment

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

Could a compound selector be used here to reduce the javascript code.

Let me know if this isn't part of the sprint!

@MattGoodwin0 MattGoodwin0 added Reviewed Volunteer to add when completing a review with trainee action still to take. and removed Review in progress This review is currently being reviewed. This label will be replaced by "Reviewed" soon. labels Aug 11, 2025
@hibosharif202504 hibosharif202504 added the Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. label Aug 12, 2025
@MattGoodwin0 MattGoodwin0 removed the Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. label Aug 14, 2025
@github-actions
Copy link

Your PR's title isn't in the expected format.

Please check its title is in the correct format, and update it.

Reason: Sprint part (Module-Data-Group) doesn't match expected format (example: 'Sprint 2', without quotes)

@hibosharif202504 hibosharif202504 changed the title London | ITP-May-2025 | Hibo Sharif | Module-Data-Group | Sprint3 -Todo_list London | ITP-May-2025 | Hibo Sharif | Module-Data-Group | Sprint 3 -Todo_list Aug 16, 2025
@github-actions
Copy link

Your PR's title isn't in the expected format.

Please check the expected title format, and update yours to match.

Reason: Sprint part (Module-Data-Group) doesn't match expected format (example: 'Sprint 2', without quotes)

@hibosharif202504 hibosharif202504 changed the title London | ITP-May-2025 | Hibo Sharif | Module-Data-Group | Sprint 3 -Todo_list London | ITP-May-2025 | Hibo Sharif | Sprint 3 -Todo_list Aug 16, 2025
@hibosharif202504 hibosharif202504 changed the title London | ITP-May-2025 | Hibo Sharif | Sprint 3 -Todo_list London | ITP-May-2025 | Hibo Sharif | Sprint 3 | Todo_list Aug 16, 2025
@github-actions
Copy link

Your PR's title isn't in the expected format.

Please check the expected title format, and update yours to match.

Reason: Wrong number of parts separated by |s

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Reviewed Volunteer to add when completing a review with trainee action still to take.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants