Skip to content

Ex. 1 - Rotem Ben David #14

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
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Ex. 1 - Rotem Ben David #14

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
9f812be
feat: adds TO-DO list
rotem-bendavid May 19, 2022
5e70d67
Merge branch 'monday-u-com:main' into main
rotem-bendavid May 20, 2022
d431f4c
Merge branch 'monday-u-com:main' into main
rotem-bendavid Jun 10, 2022
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
20 changes: 15 additions & 5 deletions src/ex1/index.html
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,14 +1,24 @@
<!DOCTYPE html>
<html lang="en">
<head>
<title>Exercise 1</title>
<title>TO-DO List</title>
<link rel="stylesheet" href="style.css" />
</head>
<body>

<h1>Todo list</h1>


<div class="container">
<h1>TO-DO list</h1>
<div class="newTask">
<input type="text" placeholder="Add new TO-DO">
<button type="submit" class="add">+</button>
</div>
<div class="tasks">
</div>
<div class="bottom">
<span class="howMany"></span>
<button type="button" class="sort" style="display:none">sort</button>
Copy link

Choose a reason for hiding this comment

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

Using a class instead of an inline style attribute is always better. Only in specific cases, using an inline style is preferred.

<button type="reset" class="clear" style="display:none">clear all</button>
</div>
</div>
</body>
<script src="script.js"></script>
</html>
89 changes: 89 additions & 0 deletions src/ex1/script.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,89 @@
let input = document.querySelector(".newTask input");
Copy link

Choose a reason for hiding this comment

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

It could also be a constant.
Moreover, you could have used a more straightforward name. input is too generic and doesn't say much about the element it points at.

const add = document.querySelector(".newTask button");
Copy link

Choose a reason for hiding this comment

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

Again, you could have used a more specific name

const tasksView = document.querySelector(".tasks");
const tasks = [];
const howManyTasks = document.querySelector(".bottom .howMany");
const showClearAll = document.querySelector(".bottom .clear");
Copy link

Choose a reason for hiding this comment

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

clearAllButtom would be a better name

const showSort = document.querySelector(".bottom .sort");
Copy link

Choose a reason for hiding this comment

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

sortButton would be a better name


//defult massege displayed when there are no tasks
const defultMassage = 'Please add your TO-DO List!';
howManyTasks.innerHTML = defultMassage;

//add new tasks
add.addEventListener('click', () => { //recognize click on 'add' button
addNewTask(input.value);
});

input.addEventListener('keypress', (event) => { //recognize pressing enter key
if (event.keyCode === 13) {
event.preventDefault();
addNewTask(input.value);
}
});

function addNewTask(value) {
if (value === '') {
alert("You can't add an empty task");
}
else {
showTasks(value);
updateBottom();
}
}

//show the tasks
function showTasks(newTask) { //adds new task to tasks array and adds it to html
let newLiTag = '';
if (newTask != undefined) {
Copy link

Choose a reason for hiding this comment

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

Lines 38-40 should have been part of addNewTask
This is the code that adds the task to the tasks array.

showTasks should be in charge of showing the tasks. No more logic should be in it.

tasks.push(newTask);
}
tasks.forEach((element, index) => {
newLiTag += '<li onClick="taskAlert('+index+')">'+element+'<span onClick="deleteTask('+index+');event.stopPropagation();">&#x1F5D1;</span></li>';
Copy link

Choose a reason for hiding this comment

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

You called the variable newLiTag and eventually, it holds multiple li tags, each for every task in the tasks array.

Besides it, it is not clear what &#x1F5D1; means. Make sure you add explanations when you code is not 100% straightforward.

});
tasksView.innerHTML = newLiTag;
input.value = ""; //clears the input after adding task
}

//updates the bottom
function updateBottom() { //modify the amount of tasks displayed
Copy link

Choose a reason for hiding this comment

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

It's unclear what updateBottom does. Use better names that explain what the code is doing.

if (tasks.length >= 1) {
let newSpan = '';
newSpan = '<span>You have '+tasks.length+' pending tasks</span>'
howManyTasks.innerHTML = newSpan;
if (tasks.length === 1) { //display clear all button only when there are tasks
showClearAll.style = 'display: block';
Copy link

Choose a reason for hiding this comment

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

Using classes is preferred

Copy link
Author

Choose a reason for hiding this comment

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

@or-nuri-monday can you please explain what do you mean? (I fixed all other commants)

showSort.style = 'display: block';
}
}
else { //delete clear all button when there aren't tasks
showClearAll.style = 'display: none';
showSort.style = 'display: none';
howManyTasks.innerHTML = defultMassage;
Copy link

Choose a reason for hiding this comment

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

defultMassage is not clear.

}
}

//delete task
function deleteTask(index){
tasks.splice(index, 1);
showTasks();
updateBottom();
}

//clear all
showClearAll.addEventListener('click', () => { //recognize click on 'clear all' button
tasks.length = 0;
showTasks();
updateBottom();
});

//task alert
function taskAlert(index) {
alert(tasks[index]);
}

//sort
showSort.addEventListener('click', () => { //recognize click on 'sort' button
tasks.sort();
Copy link

Choose a reason for hiding this comment

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

Sort without a function is usually not recommended as it is unclear how the array is sorted.

showTasks();
});
120 changes: 120 additions & 0 deletions src/ex1/style.css
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,120 @@

* {
box-sizing: border-box;
}

body {
height: 100vh;
width: 100%;
background: linear-gradient( to bottom, #f0fba6 0%, #fb5b2f 100%)
}

.container {
margin: 120px auto;
max-width: 50%;
background-color: aliceblue;
padding-inline: 25px;
padding-top: 10px;
padding-bottom: 25px;
border-radius: 5px;
}

.container h1 {
color: rgb(0, 0, 0);
font-size: 30px;
text-align: center;
}


.newTask input {
width: 100%;
min-width: 85%;
height: 100%;
border: 1px solid #ccc;
font-size: 17px;
border-radius: 3px;
padding-left: 15px;
outline: none;
}

.newTask button {
width: 50px;
height: 100%;
border: none;
outline: none;
background: rgb(255, 162, 0);
color: #fff;
font-size: 22px;
cursor: pointer;
border-radius: 3px;
margin-left: 5px;

}

.container .newTask {
display: flex;
height: 45px;
width: 100%;
margin: 20px 0;
}

.container .tasks {
max-height: 250px;
overflow-y: auto;
}

.tasks li {
list-style: none;
height: 45px;
line-height: 45px;
position: relative;
background: #f2f2f2;
border-radius: 3px;
margin-bottom: 8px;
padding: 0 15px;
overflow: hidden;
}

.tasks li:hover {
background: #d9d9d9;
cursor: pointer;
}

.tasks li span {
position: absolute;
right: -45px;
color: #fff;
background-color: red;
width: 45px;
text-align: center;
border-radius: 0 3px 3px 0;
cursor: pointer;
transition: all 0.3s ease;
}

.tasks li:hover span {
right: 0px;
}

.container .bottom {
display: flex;
width: 100%;
margin-top: 15px;
padding-left: 15px;
}

.bottom button {
border: none;
outline: none;
background: gray;
color: #fff;
font-size: 16px;
cursor: pointer;
border-radius: 3px;
padding: 6px 10px;
}

.container .bottom .sort {
margin-left: auto;
margin-right: 5px;
}