Skip to content

London | 26-ITP-Jan | Carlos Abreu |Sprint 3 | Todo List#1147

Open
carlosyabreu wants to merge 14 commits intoCodeYourFuture:mainfrom
carlosyabreu:coursework/sprint-3/todo-list
Open

London | 26-ITP-Jan | Carlos Abreu |Sprint 3 | Todo List#1147
carlosyabreu wants to merge 14 commits intoCodeYourFuture:mainfrom
carlosyabreu:coursework/sprint-3/todo-list

Conversation

@carlosyabreu
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

Todo list app PR for Sprint 3 of Data Groups module

@carlosyabreu carlosyabreu added the Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. label Mar 29, 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
// Append a new task to todos[]
export function addTask(todos, task, completed = false) {
todos.push({ task, completed });
export function addTask(todos, task, completed = false, deadline = null) {
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

How can a user choose a deadline for the task?

Copy link
Copy Markdown
Author

@carlosyabreu carlosyabreu Apr 6, 2026

Choose a reason for hiding this comment

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

If addTask function had a deadline parameter but the function body doesn't actually use it (it still only creates { task, completed }), there are a few ways a user could choose a deadline:

Modify the function to include deadline in the task object
First, it needs to update the function implementation to actually store the deadline:

// Updated todos.mjs
export function addTask(todos, task, completed = false, deadline = null) {
todos.push({ task, completed, deadline });
}

Then a user could specify a deadline when calling the function:

// Pass deadline as the 4th parameter
const deadline = new Date("2026-12-31");
Todos.addTask(todos, "Finish project", false, deadline);

// Or with different deadlines
Todos.addTask(todos, "Morning meeting", false, new Date("2026-04-07T09:00:00"));
Todos.addTask(todos, "Submit report", false, new Date("2026-04-10"));

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

How would an end user call a function when using the web page?

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.

That's an interesting question.
I would expose this functionality to end users through a web page creating a UI that allows users to interact with these functions.

Here's how I could build a web interface using HTML and CSS:

<title>Todo List App</title> <style> .todo-item { margin: 10px 0; padding: 10px; border: 1px solid #ccc; } .completed { text-decoration: line-through; background-color: #e0ffe0; } button { margin-left: 10px; } </style>

Todo List

<!-- Add Task Form -->
<div>
    <input type="text" id="taskInput" placeholder="Enter task description">
    <input type="datetime-local" id="deadlineInput">
    <button id="addBtn">Add Task</button>
</div>

<!-- Todo List Display -->
<div id="todoList"></div>

<script type="module" src="web-app.js"></script>

JavaScript Frontend (web-app.js):

// Import the todos module
import * as Todos from "./todos.mjs";

// Initialize empty todo list
let todos = [];

// DOM elements
const taskInput = document.getElementById('taskInput');
const deadlineInput = document.getElementById('deadlineInput');
const addBtn = document.getElementById('addBtn');
const todoListDiv = document.getElementById('todoList');

// Function to render the todo list to the page
function renderTodos() {
if (!todoListDiv) return;

todoListDiv.innerHTML = '';

todos.forEach((todo, index) => {
    const todoDiv = document.createElement('div');
    todoDiv.className = 'todo-item';
    if (todo.completed) {
        todoDiv.classList.add('completed');
    }
    
    // Task text with deadline info
    let taskHtml = `<strong>${todo.task}</strong>`;
    if (todo.deadline) {
        const deadlineDate = new Date(todo.deadline);
        taskHtml += ` <small>(Due: ${deadlineDate.toLocaleString()})</small>`;
    }
    
    // Toggle completed button
    const toggleBtn = document.createElement('button');
    toggleBtn.textContent = todo.completed ? '✓ Completed' : '○ Incomplete';
    toggleBtn.onclick = () => {
        Todos.toggleCompletedOnTask(todos, index);
        renderTodos(); // Re-render to show changes
    };
    
    // Delete button
    const deleteBtn = document.createElement('button');
    deleteBtn.textContent = 'Delete';
    deleteBtn.onclick = () => {
        Todos.deleteTask(todos, index);
        renderTodos(); // Re-render to show changes
    };
    
    todoDiv.innerHTML = taskHtml;
    todoDiv.appendChild(toggleBtn);
    todoDiv.appendChild(deleteBtn);
    todoListDiv.appendChild(todoDiv);
});

// Show message if list is empty
if (todos.length === 0) {
    todoListDiv.innerHTML = '<p>No tasks yet. Add one above!</p>';
}

}

// Add task function (connects UI to the todos module)
function addTaskFromUI() {
const taskText = taskInput.value.trim();

if (!taskText) {
    alert('Please enter a task description');
    return;
}

// Get deadline from input
let deadline = null;
if (deadlineInput.value) {
    deadline = new Date(deadlineInput.value);
}

// Call the addTask function with deadline
// Note: You need to update todos.mjs to actually use the deadline parameter
Todos.addTask(todos, taskText, false, deadline);

// Clear inputs
taskInput.value = '';
deadlineInput.value = '';

// Re-render the updated list
renderTodos();

}

// Event listeners
addBtn.addEventListener('click', addTaskFromUI);
taskInput.addEventListener('keypress', (e) => {
if (e.key === 'Enter') {
addTaskFromUI();
}
});

// Initial render
renderTodos();

The end users would interact adding a task with deadline:
Type task description in the text box.
Select a date/time from the datetime-local input (this is how users choose a deadline)
Click "Add Task" button or press Enter
Toggling task completion:
Clicking the Completed or Incomplete button next to any task, then the task's appearance will change.
Deleting a task:
Click the Delete button next to any task

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 didn't you commit this changes? At the moment the code includes parts that are not really functional. They should either be removed or additional code should be added to make it work

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.

You're absolutely right!
Looking more closely to the code it had a critical issue the todos.mjs function wasn't actually storing the deadline parameter. I refactored the code based on your feedback.
Thank you

Comment on lines +34 to +38
for (let i = todos.length - 1; i >= 0; i--) {
if (todos[i].completed) {
todos.splice(i, 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 correctly. You can also research build in array functions that allow you to filter elements

Copy link
Copy Markdown
Author

@carlosyabreu carlosyabreu Apr 6, 2026

Choose a reason for hiding this comment

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

Thank you for pointing that out.
Definitely I'll research that option of building an array functions that allows element filtering to expand my knowledge.

@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
@carlosyabreu carlosyabreu added the Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. label Apr 6, 2026
@Luro91 Luro91 removed the Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. label Apr 7, 2026
@carlosyabreu carlosyabreu added the Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. label Apr 8, 2026
@Luro91 Luro91 removed the Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. label Apr 9, 2026
@carlosyabreu carlosyabreu force-pushed the coursework/sprint-3/todo-list branch from c32bdbd to e529e16 Compare April 12, 2026 18:30
@carlosyabreu carlosyabreu added the Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. label Apr 12, 2026
@carlosyabreu
Copy link
Copy Markdown
Author

@Luro91
I refactored the code and pushed it based on your feedback.
It's been a good learning process.
Thanks

Comment thread Sprint-3/todo-list/index.html Outdated
<meta name="viewport" content="width=device-width, initial-scale=1.0">
<title>Todo List App</title>
<style>
body {
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 move the styles to the html file?

});
});

describe("deleteCompleted()", () => {
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

All 3 tests fail for me with the error "Todos.deleteCompleted is not a function"

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The tests are still failing.

Comment thread Sprint-3/todo-list/index.html Outdated
<div id="todoList"></div>
</div>

<script type="module" src="web-app.js"></script>
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 change the name of the script file?

Comment thread Sprint-3/todo-list/index.html Outdated
<div class="add-task-form">
<input type="text" id="taskInput" placeholder="Enter task description (e.g., Finish project report)">
<input type="datetime-local" id="deadlineInput">
<button id="addBtn">➕ Add Task</button>
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The add task button does not work for me. Nothing happens when I click it

@Luro91 Luro91 removed the Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. label Apr 13, 2026
@carlosyabreu
Copy link
Copy Markdown
Author

carlosyabreu commented Apr 22, 2026

Sorry @Luro91 for the delay.
I've been stressed working on the following and final module "Module Data Flows" as it contains the final Project TV Show and I'm in the last stage in level 500 of Project TV Show.
It's a group project with different levels and it's been my first time working in collaborative way and it's been a fight all these days with Git and GitHub and to make my colleague to understand its workflow.

Going back to this project.
I tried to understand your queries but I misread what you meant and changed completely the workflow as I've add web-app.js and moved the style.css to index.html.

Your initial question was:
How can a user choose a deadline for the task?

Based on initial change I've done the deadline parameter in todos.mjs exists but is never used by the UI, so users have no way to set it. The previous existing code at that time only allows adding a task description and toggling completion/deletion.
Now with the new changes I think it works as asked by you.
This journey has been challenging but rewarding at same time as I'm learning alot and you're aptly questions helped me to clarify my understanding.
Thank you.

@carlosyabreu carlosyabreu added the Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. label Apr 22, 2026
@Luro91
Copy link
Copy Markdown

Luro91 commented Apr 23, 2026

The button for deleting completed task is missing and the tests are failing. It's nice that you also spend time on the extension task with the adding a date and time field. However the core functionality is the important part which should be working as expected.

I recommend that you always check the core functionality and make sure it is working as expected first.
The extension is a bonus. Your solution looked already quite good in the first review and I would also have been fine if you would have said that you do not have the time to improve the extension task.

Please also ask clarifying questions if you are unsure about my comments and let me know how I can communicate more clearly.

I did not understand why you added the needed changes as a comment instead of committing them, since you anyway already did the work of writing the code.

@Luro91 Luro91 removed the Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. label Apr 23, 2026
@carlosyabreu
Copy link
Copy Markdown
Author

carlosyabreu commented Apr 23, 2026

@Luro91
Thanks for pointing that out to me.
It adds to my understanding.
You mentioned the delete button is missing. In fact, the delete button is at the right end of every task created.

As shown by these screenshots:

Inserting a task:
Inserting clean the dishwasher

Button for deleting tasks at the right end:
Inserting clean the dishwasher done

@carlosyabreu carlosyabreu added the Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. label Apr 23, 2026
@carlosyabreu
Copy link
Copy Markdown
Author

@Luro91
All tests now pass without errors. The deleteCompleted function has been added to todos.mjs to match the test expectations. I'm using:
node --test todos.test.mjs

Again, thanks @Luro91, it's been a marathon running with your direction.

@Luro91
Copy link
Copy Markdown

Luro91 commented Apr 24, 2026

@Luro91 Thanks for pointing that out to me. It adds to my understanding. You mentioned the delete button is missing. In fact, the delete button is at the right end of every task created.

As shown by these screenshots:

Inserting a task: Inserting clean the dishwasher

Button for deleting tasks at the right end: Inserting clean the dishwasher done

Sorry for being not clear. I am referring to the button that will delete all completed tasks.
Add a button that deletes all completed tasks at once.


// Command to execute this script:
// npm test todos.test.mjs
// node --test todos.test.mjs
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 this change?

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.

Afternoon @Luro91
As I was unsure which framework you used to test (todos.test.mjs) I used a few test frameworks to cover most cases.
Based on this scenario, first I run (npm test) and check (package.json) looking for ("jest", "mocha", "vitest", "test": "...").

Later, I tried using Jest but because Jest does not support .mjs out of the box cleanly, I run (npx jest todos.test.mjs).
I even tried Mocha (npx mocha todos.test.mjs) and Vitest (npx vitest run todos.test.mjs).
But I decided to drop these approaches and my best guest the framework you would be using was Node as Node ≥ v18 has built-in test runner. But left (npm test todos.test.mjs) as a comment.
node --test todos.test.mjs


describe("addTask()", () => {
test("Add a task to an empty ToDo list", () => {
it("Add a task to an empty ToDo list", () => {
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 are you did you change it to test?

Copy link
Copy Markdown
Author

@carlosyabreu carlosyabreu Apr 24, 2026

Choose a reason for hiding this comment

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

The answer goes back to previous explanation.
My last codebase I'm not using test just it.
Below, I attach the screenshot of my local repo for your reference (I'm using Vim. Vim is my favourite IDE).

todos tesst mjs

@Luro91 Luro91 removed the Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. label Apr 24, 2026
@carlosyabreu
Copy link
Copy Markdown
Author

carlosyabreu commented Apr 24, 2026

You wrote below:
"Sorry for being not clear. I am referring to the button that will delete all completed tasks.
Add a button that deletes all completed tasks at once."

I didn't implement this functionality because on the requirement there is no mention of that.
In the classroom 2 Saturdays before I asked my colleagues to run their app but I didn't see this functionality implemented so based on requirement and other implementations I decided to do the same.

@carlosyabreu carlosyabreu added the Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. label Apr 24, 2026
@Luro91
Copy link
Copy Markdown

Luro91 commented Apr 25, 2026

You wrote below: "Sorry for being not clear. I am referring to the button that will delete all completed tasks. Add a button that deletes all completed tasks at once."

I didn't implement this functionality because on the requirement there is no mention of that. In the classroom 2 Saturdays before I asked my colleagues to run their app but I didn't see this functionality implemented so based on requirement and other implementations I decided to do the same.

Which requirements are you following? These https://github.com/CodeYourFuture/Module-Data-Groups/tree/main/Sprint-3/todo-list?

@Luro91 Luro91 removed the Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. label Apr 25, 2026
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.

2 participants