Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
11 changes: 8 additions & 3 deletions Sprint-3/todo-list/index.html
Original file line number Diff line number Diff line change
Expand Up @@ -3,16 +3,20 @@
<head>
<meta charset="UTF-8" />
<meta name="viewport" content="width=device-width, initial-scale=1.0" />
<title>Title here</title>
<title>Todo List📃</title>
<link rel="stylesheet" href="style.css" />
<script
src="https://kit.fontawesome.com/ffdbc20524.js"
crossorigin="anonymous"
></script>
</head>
<body>
<form>
<div>
<input type="text" placeholder="New todo..." />
<input type="text" id="todo-input" placeholder="New todo..." />
</div>
<div>
<button type="submit">Add Todo</button>
<button type="submit" id="add-todo-btn">Add Todo</button>
<button
type="button"
id="remove-all-completed"
Expand All @@ -21,6 +25,7 @@
Remove all completed
</button>
</div>
<ul id="todo-list"></ul>
</form>
<script src="script.js"></script>
</body>
Expand Down
125 changes: 122 additions & 3 deletions Sprint-3/todo-list/script.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,88 @@
function populateTodoList(todos) {
let list = document.getElementById("todo-list");
// Write your code to create todo list elements with completed and delete buttons here, all todos should display inside the "todo-list" element.
let body = document.querySelector("body");

list.innerHTML = "";

let todoButtonsIndex = 0;

let todoButtons = `<span class="badge bg-primary rounded-pill">
Copy link
Contributor

Choose a reason for hiding this comment

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

If todoButtons does not need to be reassigned a different value later, it is better to declare it using const.

<i class="fa-solid fa-check" aria-hidden="true"></i>
<i class="fa fa-trash" aria-hidden="true"></i>
</span>`;

todos.forEach((todoItem, index) => {

let listItem = document.createElement("li");
listItem.id = index;

let breakLine = document.createElement("br");
let span = document.createElement("span");


span.innerHTML = todoButtons;
listItem.style.background = "lightblue";
listItem.style.padding = "1em";
listItem.style.display = "inline-block";
listItem.style.margin = "0.5em";
listItem.textContent = todoItem.task;
list.style.margin = "0 auto";
list.style.backgroundColor = "#FCC4AB";
body.style.textAlign = "center"
Comment on lines +24 to +31
Copy link
Contributor

Choose a reason for hiding this comment

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

Why not set CSS styles in styles.css? That way, whenever we need to change the view we don't have to change the code.


list.appendChild(breakLine);
Copy link
Contributor

Choose a reason for hiding this comment

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

Adding <br> to a list would result in creating something like

<ul>
<li>....</li>
<br>
<li>...</li>
<br>
</ul>

which is considered an error in HTML.

Can you think of a better way to create space between list items using only CSS?


if (todoItem.completed === true) {
listItem.style.textDecoration = "line-through";
}
listItem.appendChild(span);
list.appendChild(listItem);
//target the span instead of targetin the document, to make it more precise
const checkMark = span.querySelector(".fa-check");
const deleteButton = span.querySelector(".fa-trash");

checkMark.id = todoButtonsIndex;
deleteButton.id = todoButtonsIndex;
todoButtonsIndex++;
Comment on lines +44 to +46
Copy link
Contributor

Choose a reason for hiding this comment

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

  • These id values are not unique.
  • Why do you want to assign id to these two buttons? Where are these id values being used?
  • If you don't need these id values, you probably won't need todoButtonsIndex.


//when the checkmark or trash buttons are clicked
//make span be inside the listItem element and target the nearest listItem


checkMark.addEventListener("click", function () {

const listItem = this.closest("li");
const todoID = listItem.id;

if (todoID && todos[parseInt(todoID)].completed === false) {
todos[parseInt(todoID)].completed = true;
listItem.style.textDecoration = "line-through";

}
else {
todos[parseInt(todoID)].completed = false;
listItem.style.textDecoration = "none";
}
Comment on lines +57 to +65
Copy link
Contributor

Choose a reason for hiding this comment

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

Why not just call populateTodoList() at line 59 and 64 to update the whole UI?

Any chance todoID is undefined or an empty string?
The code at lines 57-65 could possibly be reduced to 2-3 lines of code.


})


deleteButton.addEventListener("click", function () {
const listItem = this.closest("li");
Copy link
Contributor

Choose a reason for hiding this comment

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

Do you know what this is at line 71?

const todoID = listItem.id;

if (todoID) {
todos.splice(parseInt(todoID), 1);
listItem.remove();
Copy link
Contributor

Choose a reason for hiding this comment

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

Is line 76 necessary? The next statement, populateTodoList(), will update the whole UI by reconstructing all the elements that make up UI.

populateTodoList(todos);

}


})


});
}

// These are the same todos that currently display in the HTML
Expand All @@ -12,14 +94,51 @@ let todos = [

populateTodoList(todos);


// This function will take the value of the input field and add it as a new todo to the bottom of the todo list. These new todos will need the completed and delete buttons adding like normal.
function addNewTodo(event) {
// The code below prevents the page from refreshing when we click the 'Add Todo' button.
event.preventDefault();
// Write your code here... and remember to reset the input field to be blank after creating a todo!

let inputField = document.getElementById("todo-input");
let inputFieldValue = inputField.value;
let completedStatus = false;

let todo = {};

if (inputFieldValue.length > 0) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Do you want to accept any input containing only space characters?

todo["task"] = inputFieldValue
todo["completed"] = completedStatus;
todos.push(todo);

console.log(`Added new Todo - Task:${todo.task}: Status:${todo.completed}`);
populateTodoList(todos);
}
else {
alert("Please input a new todo item!!");
}



}


//need to target form when the button is clicked

let form = document.querySelector("form");
form.addEventListener("submit", addNewTodo);

// Advanced challenge: Write a fucntion that checks the todos in the todo list and deletes the completed ones (we can check which ones are completed by seeing if they have the line-through styling applied or not).

let removeButton = document.getElementById("remove-all-completed");
function deleteAllCompletedTodos() {
// Write your code here...

todos = todos.filter((todoItem) => {
return todoItem.completed === false;
})

populateTodoList(todos);
}

removeButton.addEventListener("click", deleteAllCompletedTodos);