Skip to content

London | 26-ITP-January | Eugenie Ahangama | Sprint 3 | Todo List App #1006

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Eugenie-A wants to merge 5 commits into
base: main
Choose a base branch
from
+106 −53
Open

London | 26-ITP-January | Eugenie Ahangama | Sprint 3 | Todo List App #1006

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
e06dff0
Add delete completed tasks button
Eugenie-A Mar 13, 2026
c173e8b
Add deleteCompleted function to todos
Eugenie-A Mar 13, 2026
8c60f9d
Add deleteCompleted tests
Eugenie-A Mar 13, 2026
cc75d04
Add deleteCompletedTodos button handler
Eugenie-A Mar 13, 2026
cea3408
Fix: Refactor deleteCompleted to use filter without forEach
Eugenie-A Mar 22, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
74 changes: 42 additions & 32 deletions Sprint-3/todo-list/index.html
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,40 +1,50 @@
<!DOCTYPE html>
<!doctype html>
Copy link

@kyle-tightest kyle-tightest Mar 22, 2026

Choose a reason for hiding this comment

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

this is not case sensitive so it doesn't matter but it is more standard to capitalise this.
did your formatter do this automatically?

Copy link
Author

@Eugenie-A Eugenie-A Mar 22, 2026

Choose a reason for hiding this comment

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

Yes, that was my Prettier formatter in VS Code doing it automatically on save. I know it's more standard to keep it uppercase but Prettier lowercases it by default. Prettier formatter is the standard formatter we were asked to use in our work.

Copy link

@kyle-tightest kyle-tightest Mar 23, 2026

Choose a reason for hiding this comment

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

Okay no problem

<html lang="en">
<head>
<meta charset="UTF-8" />
<meta name="viewport" content="width=device-width, initial-scale=1.0"/>
<title>ToDo List</title>
<link rel="stylesheet" href="style.css" />
<link href="https://cdnjs.cloudflare.com/ajax/libs/font-awesome/6.4.0/css/all.min.css" rel="stylesheet">
<head>
<meta charset="UTF-8" />
<meta name="viewport" content="width=device-width, initial-scale=1.0" />
<title>ToDo List</title>
<link rel="stylesheet" href="style.css" />
<link
href="https://cdnjs.cloudflare.com/ajax/libs/font-awesome/6.4.0/css/all.min.css"
rel="stylesheet"
/>

<script type="module" src="script.mjs"></script>
</head>
<body>
<div class="todo-container">
<h1>My ToDo List</h1>
<script type="module" src="script.mjs"></script>
</head>
<body>
<div class="todo-container">
<h1>My ToDo List</h1>

<div class="todo-input">
<input type="text" id="new-task-input" placeholder="Enter a new task..." />
<button id="add-task-btn">Add</button>
</div>

<ul id="todo-list" class="todo-list">
</ul>
<div class="todo-input">
<input
type="text"
id="new-task-input"
placeholder="Enter a new task..."
/>
<button id="add-task-btn">Add</button>
</div>

<!--
<ul id="todo-list" class="todo-list"></ul>
<button id="delete-completed-btn">Delete completed tasks</button>
<!--
This is a template for the To-do list item.
It can simplify the creation of list item node in JS script.
-->
<template id="todo-item-template">
<li class="todo-item"> <!-- include class "completed" if the task completed state is true -->
<span class="description">Task description</span>
<div class="actions">
<button class="complete-btn"><span class="fa-solid fa-check" aria-hidden="true"></span></button>
<button class="delete-btn"><span class="fa-solid fa-trash" aria-hidden="true"></span></button>
</div>
</li>
</template>

</div>
</body>
<template id="todo-item-template">
<li class="todo-item">
<!-- include class "completed" if the task completed state is true -->
<span class="description">Task description</span>
<div class="actions">
<button class="complete-btn">
<span class="fa-solid fa-check" aria-hidden="true"></span>
</button>
<button class="delete-btn">
<span class="fa-solid fa-trash" aria-hidden="true"></span>
</button>
</div>
</li>
</template>
</div>
</body>
</html>
28 changes: 18 additions & 10 deletions Sprint-3/todo-list/script.mjs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// Store everything imported from './todos.mjs' module as properties of an object named Todos
// Store everything imported from './todos.mjs' module as properties of an object named Todos
import * as Todos from "./todos.mjs";

// To store the todo tasks
Expand All @@ -7,16 +7,19 @@ const todos = [];
// Set up tasks to be performed once on page load
window.addEventListener("load", () => {
document.getElementById("add-task-btn").addEventListener("click", addNewTodo);
// Call deleteCompletedTodos whenever the delete completed button is clicked
document
.getElementById("delete-completed-btn")
.addEventListener("click", deleteCompletedTodos);

// Populate sample data
Todos.addTask(todos, "Wash the dishes", false);
Todos.addTask(todos, "Wash the dishes", false);
Todos.addTask(todos, "Do the shopping", true);

render();
});


// A callback that reads the task description from an input field and
// A callback that reads the task description from an input field and
// append a new task to the todo list.
function addNewTodo() {
const taskInput = document.getElementById("new-task-input");
Expand Down Expand Up @@ -45,12 +48,11 @@ function render() {
});
}


// Note:
// - First child of #todo-item-template is a <li> element.
// We will create each ToDo list item as a clone of this node.
// - This variable is declared here to be close to the only function that uses it.
const todoListItemTemplate =
const todoListItemTemplate =
document.getElementById("todo-item-template").content.firstElementChild;

// Create a <li> element for the given todo task
Expand All @@ -62,15 +64,21 @@ function createListItem(todo, index) {
li.classList.add("completed");
}

li.querySelector('.complete-btn').addEventListener("click", () => {
li.querySelector(".complete-btn").addEventListener("click", () => {
Todos.toggleCompletedOnTask(todos, index);
render();
});
li.querySelector('.delete-btn').addEventListener("click", () => {

li.querySelector(".delete-btn").addEventListener("click", () => {
Todos.deleteTask(todos, index);
render();
});

return li;
}
}

// A callback that deletes all completed tasks from the todolist and re-renders
function deleteCompletedTodos() {
Todos.deleteCompleted(todos);
render();
}
8 changes: 7 additions & 1 deletion Sprint-3/todo-list/todos.mjs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,4 +26,10 @@ export function toggleCompletedOnTask(todos, taskIndex) {
if (todos[taskIndex]) {
todos[taskIndex].completed = !todos[taskIndex].completed;
}
}
}

// Filter out all the completed tasks and update the original array in place
export function deleteCompleted(todos) {
const incomplete = todos.filter((todo) => !todo.completed);
todos.splice(0, todos.length, ...incomplete);
}
Comment on lines +32 to +35
Copy link

@kyle-tightest kyle-tightest Mar 22, 2026

Choose a reason for hiding this comment

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

While this code works, it is inefficient as you are iterating over the entire array when you do filter and then iterating over each of the filtered array items when you do forEach.
The .filter method returns an array filtered by the condition you give it.
Try and see if you can use the .filter method differently to achieve the same outcome, without needing forEach?

Copy link
Author

@Eugenie-A Eugenie-A Mar 22, 2026

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've updated the function to use filter once to get the incomplete todos, then splice to update the original array in place.

Copy link

@kyle-tightest kyle-tightest Mar 23, 2026

Choose a reason for hiding this comment

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

Thanks, this works as well.
But again, when you do splice in that second step you're iterating over each of the filtered array items.
This is not necessary.
Try to look at the function again and see if you can do it in 1 line, instead of 2?

49 changes: 39 additions & 10 deletions Sprint-3/todo-list/todos.test.mjs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ function createMockTodos() {
{ task: "Task 1 description", completed: true },
{ task: "Task 2 description", completed: false },
{ task: "Task 3 description", completed: true },
{ task: "Task 4 description", completed: false },
{ task: "Task 4 description", completed: false },
];
}

Expand All @@ -29,7 +29,6 @@ describe("addTask()", () => {
});

test("Should append a new task to the end of a ToDo list", () => {

const todos = createMockTodos();
const lengthBeforeAddition = todos.length;
Todos.addTask(todos, theTask.task, theTask.completed);
Expand All @@ -42,7 +41,6 @@ describe("addTask()", () => {
});

describe("deleteTask()", () => {

test("Delete the first task", () => {
const todos = createMockTodos();
const todosBeforeDeletion = createMockTodos();
Expand All @@ -53,7 +51,7 @@ describe("deleteTask()", () => {

expect(todos[0]).toEqual(todosBeforeDeletion[1]);
expect(todos[1]).toEqual(todosBeforeDeletion[2]);
expect(todos[2]).toEqual(todosBeforeDeletion[3]);
expect(todos[2]).toEqual(todosBeforeDeletion[3]);
});

test("Delete the second task (a middle task)", () => {
Expand All @@ -66,7 +64,7 @@ describe("deleteTask()", () => {

expect(todos[0]).toEqual(todosBeforeDeletion[0]);
expect(todos[1]).toEqual(todosBeforeDeletion[2]);
expect(todos[2]).toEqual(todosBeforeDeletion[3]);
expect(todos[2]).toEqual(todosBeforeDeletion[3]);
});

test("Delete the last task", () => {
Expand All @@ -79,7 +77,7 @@ describe("deleteTask()", () => {

expect(todos[0]).toEqual(todosBeforeDeletion[0]);
expect(todos[1]).toEqual(todosBeforeDeletion[1]);
expect(todos[2]).toEqual(todosBeforeDeletion[2]);
expect(todos[2]).toEqual(todosBeforeDeletion[2]);
});

test("Delete a non-existing task", () => {
Expand All @@ -94,7 +92,6 @@ describe("deleteTask()", () => {
});

describe("toggleCompletedOnTask()", () => {

test("Expect the 'completed' property to toggle on an existing task", () => {
const todos = createMockTodos();
const taskIndex = 1;
Expand All @@ -111,13 +108,12 @@ describe("toggleCompletedOnTask()", () => {
const todos = createMockTodos();
const todosBeforeToggle = createMockTodos();
Todos.toggleCompletedOnTask(todos, 1);
expect(todos[0]).toEqual(todosBeforeToggle[0]);

expect(todos[0]).toEqual(todosBeforeToggle[0]);
expect(todos[2]).toEqual(todosBeforeToggle[2]);
expect(todos[3]).toEqual(todosBeforeToggle[3]);
});


test("Expect no change when toggling on a non-existing task", () => {
const todos = createMockTodos();
const todosBeforeToggle = createMockTodos();
Expand All @@ -130,3 +126,36 @@ describe("toggleCompletedOnTask()", () => {
});
});

describe("deleteCompleted()", () => {
test("Deletes all completed tasks from the list", () => {
const todos = createMockTodos();
Todos.deleteCompleted(todos);
expect(todos).toHaveLength(2);
expect(todos.every((todo) => !todo.completed)).toEqual(true);
});

test("Does not affect incomplete tasks", () => {
const todos = createMockTodos();
Todos.deleteCompleted(todos);
expect(todos[0]).toEqual({ task: "Task 2 description", completed: false });
expect(todos[1]).toEqual({ task: "Task 4 description", completed: false });
});

test("Does nothing when there are no completed tasks", () => {
const todos = [
{ task: "Task 1", completed: false },
{ task: "Task 2", completed: false },
];
Todos.deleteCompleted(todos);
expect(todos).toHaveLength(2);
});

test("Empties the list when all tasks are completed", () => {
const todos = [
{ task: "Task 1", completed: true },
{ task: "Task 2", completed: true },
];
Todos.deleteCompleted(todos);
expect(todos).toHaveLength(0);
});
});
Loading