Skip to content
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

London |Phone_Naing| Module-Data-Groups | WEEK3-TodoList #257

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open

London |Phone_Naing| Module-Data-Groups | WEEK3-TodoList #257

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
cc11db5
display li buttons, and can add todos
Mgphone Dec 28, 2024
2ecd004
change add and delete button work
Mgphone Dec 28, 2024
e7ba72c
add deleteallbutton is workin
Mgphone Dec 28, 2024
db4b45d
bugs fix to remove all true statement
Mgphone Dec 29, 2024
4ea6c2b
do as review
Mgphone Jan 13, 2025
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
3 changes: 2 additions & 1 deletion Sprint-3/todo-list/index.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
<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" />
</head>
<body>
Expand All @@ -22,6 +22,7 @@
</button>
</div>
</form>
<ul id="todo-list"></ul>
<script src="script.js"></script>
</body>
</html>
83 changes: 80 additions & 3 deletions Sprint-3/todo-list/script.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,25 +1,102 @@
function populateTodoList(todos) {
let list = document.getElementById("todo-list");
list.innerHTML = "";
// Write your code to create todo list elements with completed and delete buttons here, all todos should display inside the "todo-list" element.
if (todos.length <= 0) {
let createLi = document.createElement("li");

createLi.innerHTML = "No Todos ";
list.appendChild(createLi);
} else {
todos.map((todo, index) => {
//display li
const createLi = document.createElement("li");
const taskSpan = document.createElement("span");
taskSpan.innerHTML = todo.task;
createLi.appendChild(taskSpan);
list.appendChild(createLi);
//end li
if (todo.completed == true) {
taskSpan.style.textDecoration = "line-through";
}
//add button in li
let completeButton = document.createElement("button");
let deleteButton = document.createElement("button");
completeButton.textContent = todo.completed == true ? "✅" : "❌";
deleteButton.innerText = "Delete";
createLi.appendChild(completeButton);
createLi.append(deleteButton);
//button eventListener
completeButton.addEventListener("click", () => {
// if (todos[index].completed) {
// todos[index].completed = false;
// } else {
// todos[index].completed = true;
// }
todos[index].completed = !todos[index].completed;
console.log(`You click complete button${index}`);
populateTodoList(todos);
});
deleteButton.addEventListener("click", () => {
console.log(`you click delete button ${index}`);
todos.splice(index, 1);
populateTodoList(todos);
});
//end button in li
});
}
}

// These are the same todos that currently display in the HTML
// You will want to remove the ones in the current HTML after you have created them using JavaScript
let todos = [
{ task: "Wash the dishes", completed: false },
{ task: "Wash the dishes", completed: true },
{ task: "Do the shopping", completed: false },
];

populateTodoList(todos);
deleteAllCompletedTodos();
//add to do button listener
const addTodoButton = document.querySelector("button[type='submit']");
const deleteAllButton = document.getElementById("remove-all-completed");

addTodoButton.addEventListener("click", (event) => {
event.preventDefault();
Mgphone marked this conversation as resolved.
Show resolved Hide resolved
console.log("you click add to do button");
addNewTodo(event);
});

// 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();
// event.preventDefault();
const addValue = document.querySelector("input[type='text']");
if (addValue.value == "" || addValue.value == " ") {
alert("Please add todoList");
} else {
// alert(addValue.value);
const newAddObj = { task: addValue.value, completed: false };
todos.push(newAddObj);
populateTodoList(todos);
addValue.value = "";
}
// Write your code here... and remember to reset the input field to be blank after creating a todo!
}

// 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).
// Advanced challenge: Write a function 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).
function deleteAllCompletedTodos() {
// Write your code here...
//remove all button listener
deleteAllButton.addEventListener("click", () => {
console.log("you click delete all button");
// const finalFilter = todos.filter((ele) => ele.completed === "true");
//just mutate it
for (let i = todos.length - 1; i >= 0; i--) {
if (todos[i].completed === true) {
todos.splice(i, 1);
}
}

populateTodoList(todos);
});
Copy link

Choose a reason for hiding this comment

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

Based on the name of this function deleteAllCompletedTodos(), I think the function should contain only lines 91-99. Lines 88, 89 (the part that set up the event listener) should be performed around line 58.

Copy link
Author

Choose a reason for hiding this comment

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

Thanks for pointing that out. I understand now, and I believe the main reason is to make my code easier to maintain, right?

Copy link

Choose a reason for hiding this comment

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

It's just a habit worth cultivating.
First, ensure correctness. Then, refactor the code (which also includes improve maintainability).

}
6 changes: 5 additions & 1 deletion Sprint-3/todo-list/style.css
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1 +1,5 @@

li {
padding-bottom: 12px;
display: flex;
gap: 10px;
}