Skip to content

Sheffield | 26-ITP-jan | Richard Frimpong | Sprint 3 | Todo List#1059

Open
Richiealx wants to merge 5 commits intoCodeYourFuture:mainfrom
Richiealx:coursework/sprint-3-todo-list
Open

Sheffield | 26-ITP-jan | Richard Frimpong | Sprint 3 | Todo List#1059
Richiealx wants to merge 5 commits intoCodeYourFuture:mainfrom
Richiealx:coursework/sprint-3-todo-list

Conversation

@Richiealx
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 PR implements the Sprint 3 Todo List app.

The application populates a list of hardcoded todos on page load using the populateTodoList() function. Each todo item includes a checkbox icon, a trash icon, and the todo text.

Clicking the checkbox toggles the completed state of the task. This updates the icon from to and applies or removes a strikethrough style on the todo text. Clicking the trash icon deletes the individual todo item.

I added support for creating new todos using an input field and included a deadline input so a date can be set when a task is created. When a deadline is provided, it is displayed next to the task description.

A "Delete Completed" button has also been implemented to remove all completed tasks at once.

The todo module functions (addTask, deleteTask, toggleCompletedOnTask) were preserved and extended without breaking the existing tests. I also added a helper function to delete completed tasks.

The app was tested in the browser to confirm:

  • todos load on page load
  • checkbox toggles correctly
  • completed tasks show strikethrough
  • individual todos can be deleted
  • new todos can be added
  • completed todos can be mass deleted
  • deadlines display correctly

I also ran the provided tests for the todo module, and all tests passed successfully.

Questions

No questions at this time.

@Richiealx Richiealx added the Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. label Mar 21, 2026
@Luro91 Luro91 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 Apr 6, 2026
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Nice styling. I like how the todo list looks

}

/* Mobile layout */
@media (max-width: 700px) {
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Nice that you added support for smaller layouts

Comment on lines +37 to +41
for (let index = todos.length - 1; index >= 0; index -= 1) {
if (todos[index].completed) {
todos.splice(index, 1);
}
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This works but do you know an array method that is build in that can do this for you?

/**
* Clears and rebuilds the todo list from the hardcoded/current todos array.
*/
function populateTodoList() {
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Why did you rename this? The name is not fitting and the instructions say "⚠️ You should not need to modify the render() function."

if (todo.deadline) {
deadlineEl.textContent = `Deadline: ${todo.deadline}`;
} else {
deadlineEl.textContent = "";
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

What is the default of the textContent?

Comment on lines +22 to +25
<input
type="date"
id="deadline-input"
aria-label="Set a deadline"
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Nice implementation of the stretch 1 task

Copy link
Copy Markdown
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 feedback.

I have now updated the todo list based on your comments:

  1. I changed the completed task deletion logic to use a built-in array method instead of the manual loop.
  2. I renamed the function back to render() so it matches the original starter structure and the task guidance.
  3. I removed the unnecessary else branch for the deadline text because the element is empty by default unless text is set.

I tested the app again and confirmed the functionality still works correctly.

@Luro91 Luro91 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 Apr 6, 2026
@Richiealx Richiealx added the Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. label Apr 11, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. 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.

2 participants