Skip to content

Homework#5 [Done] #3

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 8 commits into
base: master
Choose a base branch
from
Open

Homework#5 [Done] #3

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
9a9b86e
Made basic app layout
TigranKirakosov Jul 29, 2019
6aa360f
Added main functionality
TigranKirakosov Aug 1, 2019
de5bc51
Ready for review
TigranKirakosov Aug 3, 2019
cbf5505
Fixed bugs and added functionality
TigranKirakosov Aug 3, 2019
907ee15
Fixed some more bugs
TigranKirakosov Aug 4, 2019
7e372ce
Fixed functionality and rethinked architecture
TigranKirakosov Aug 5, 2019
2598a40
Fixed issue with DOM targeting
TigranKirakosov Aug 5, 2019
f8b220d
made changes according to review
TigranKirakosov Aug 19, 2019
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
1 change: 1 addition & 0 deletions TigranKirakosov/.gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
index_copy.js
86 changes: 86 additions & 0 deletions TigranKirakosov/index.html
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,86 @@
<!DOCTYPE html>
<html lang="en">
<head>
<meta charset="UTF-8" />
<meta name="viewport" content="width=device-width, initial-scale=1.0" />
<meta http-equiv="X-UA-Compatible" content="ie=edge" />
<link
href="https://fonts.googleapis.com/css?family=Open+Sans:300,400,600"
rel="stylesheet"
/>
<link rel="stylesheet" href="style.css" />
<title>To-Do App</title>
</head>
<body>
<main>
<section class="application">
<div class="application__title"><span>To-Do List</span></div>
<form class="form-group">
<div class="input-container">
<label class="input-label" for="input-text">
<span>Type a task</span>
<input
type="text"
class="input-text"
id="input-text"
placeholder="What to do?"
/>
<span>Set deadline</span>
<select name="Deadline" id="deadline">
<option value="no deadline">no deadline</option>
<option value="tomorrow">tomorrow</option>
<option value="next week">next week</option>
</select>
<button class="submit-btn">Add</button>
</label>
</div>
<div class="clear-container">
<button class="clear-btn">Clear tasks</button>
</div>
<div class="search-container">
<label for="filter" class="filter-label">
<span>Filter to-do's</span>
<select name="Filter" id="filter">
<option value="no filter">no filter</option>
<option value="no deadline">Deadline: no deadline</option>
<option value="tomorrow">Deadline: tomorrow</option>
<option value="next week">Deadline: next week</option>
<option value="undone">Is done: undone</option>
<option value="done">Is done: done</option>
</select>
<button class="filter-btn">
<i class="fas fa-filter"></i>
</button>
</label>
</div>
</form>
<ul class="collection">
<!-- <li class="collection__item" id="item-0">
<span class="text">Lorem ipsum dolor</span>
<span class="deadline">Deadline: tomorrow</span>
<div class="icons">
<i class="far fa-circle"></i>
<i class="fas fa-backspace"></i>
</div>
</li>
<li class="collection__item" id="item-0">
<span>Lorem ipsum dolor</span>
<div class="icons">
<i class="far fa-check-circle"></i>
<i class="fas fa-backspace"></i>
</div>
</li>
<li class="collection__item" id="item-0">
<span>Lorem ipsum dolor</span>
<div class="icons">
<i class="far fa-circle"></i>
<i class="fas fa-backspace"></i>
</div>
</li> -->
</ul>
</section>
</main>
<script src="index.js"></script>
<script src="https://kit.fontawesome.com/a863ebaea0.js"></script>
</body>
</html>
253 changes: 253 additions & 0 deletions TigranKirakosov/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,253 @@
var UIController = (function() {
var DOMStrings;
DOMStrings = {
taskInput: '.input-text',
deadlineInput: '#deadline',
submitBtn: '.submit-btn',
filterInput: '#filter',
filterBtn: '.filter-btn',
uncheckedBtn: 'far fa-circle',
checkedBtn: 'far fa-check-circle',
deleteBtn: 'fas fa-backspace',
taskList: '.collection',
clearBtn: '.clear-btn'
};

return {
getInput: function() {
return {
text: document.querySelector(DOMStrings.taskInput).value,
deadline: document.querySelector(DOMStrings.deadlineInput).value,
filterValue: document.querySelector(DOMStrings.filterInput).value
};
},
getDOMStrings: function() {
return DOMStrings;
},
deleteListItem: function(selectorID) {
var element;
element = document.querySelector(`#${selectorID}`);
element.parentNode.removeChild(element);
},
toggleTask: function(icon) {
if (icon.className === DOMStrings.checkedBtn) {
icon.className = DOMStrings.uncheckedBtn;
} else if (icon.className === DOMStrings.uncheckedBtn) {
icon.className = DOMStrings.checkedBtn;
}
},
filterTasks: function(tasks) {
Copy link

Choose a reason for hiding this comment

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

why UI controller should know how to filter data? As I can see in your to do list UIController - it's classic view component. All what should do this controller - correctly display data to user. Filtering of the data should do controller (line 145) component and just put array of filtered tasks to UI controller

var filterValue, filteredTasks, taskList;
var html = '';
filterValue = document.querySelector(DOMStrings.filterInput).value;
taskList = document.querySelector(DOMStrings.taskList);

filteredTasks = tasks.filter(function(task) {
return task.deadline == filterValue || task.isDone == filterValue;
});
filteredTasks.forEach(function(task) {
Copy link

Choose a reason for hiding this comment

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

move this logic to functions renderTasks and renderTask

html += `
<li class="collection__item" id="item-${task.id}">
<span class="text">${task.text}</span>
<div class="icons">
<i class="${task.iconClass}"></i>
<i class="fas fa-backspace"></i>
</div>
<span class="deadline">Deadline: ${task.deadline}</span>
</li>
`;
});
taskList.innerHTML = html;
},
clearTasks: function() {
document.querySelector(DOMStrings.taskList).innerHTML = '';
},
clearFields: function() {
document.querySelector(DOMStrings.taskInput).value = '';
document.querySelector(DOMStrings.deadlineInput).value = 'no deadline';
document.querySelector(DOMStrings.taskInput).focus();
}
};
})();

var storageController = (function(UICtrl) {
var Task, tasks, DOM;
DOM = UICtrl.getDOMStrings();

Task = function(id, text, deadline) {
this.id = id;
this.text = text;
this.deadline = deadline;
this.isDone = 'undone';
this.iconClass = DOM.uncheckedBtn;
};
Task.prototype.toggleState = function() {
if (this.isDone === 'undone') {
Copy link

Choose a reason for hiding this comment

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

move strings done and undone to some object with constants

this.isDone = 'done';
this.iconClass = DOM.checkedBtn;
} else if (this.isDone === 'done') {
this.isDone = 'undone';
this.iconClass = DOM.uncheckedBtn;
}
};
if (
localStorage.getItem('tasks') === null ||
localStorage.getItem('tasks') === ''
) {
tasks = [];
} else {
tasks = JSON.parse(localStorage.getItem('tasks'));
}
return {
getFromLS: function() {
return tasks;
},
addTask: function(text, deadline) {
var newTask, ID;
if (tasks.length > 0) ID = tasks[tasks.length - 1].id + 1;
else ID = 0;
newTask = new Task(ID, text, deadline);
return newTask;
},
addToLS: function(task) {
Copy link

Choose a reason for hiding this comment

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

why both functions - addTask and addToLS are public api of storageController? - I think you need to extract all logic that is responsible for interactions with localStorage to separated controller/service and use it inside your storageController

Copy link
Author

Choose a reason for hiding this comment

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

Will do that, valuable suggestion, should've separated the logic

tasks.push(task);
Copy link

Choose a reason for hiding this comment

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

why addToLS function makes more than just add task to LS? - in fact, you sync your internal state firstly and then add new state to LS. I think you need to move this logic to addTask function. current implementation of addTask function better to rename to smth like createTask

Copy link
Author

Choose a reason for hiding this comment

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

I see, will fix that

localStorage.setItem('tasks', JSON.stringify(tasks));
},
deleteFromLS: function(id) {
var ids, index;
Copy link

Choose a reason for hiding this comment

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

again - I don't think deleteFromLS is good naming for this function - you delete task from internal state, and just update localStorage value

ids = tasks.map(function(current) {
return current.id;
});
index = ids.indexOf(id);
if (index !== -1) {
tasks.splice(index, 1);
}
localStorage.setItem('tasks', JSON.stringify(tasks));
},
toggleTask: function(id) {
var ids, index;
ids = tasks.map(function(current) {
return current.id;
});
index = ids.indexOf(id);
if (index !== -1) {
Task.prototype.toggleState.call(tasks[index]);
}
localStorage.setItem('tasks', JSON.stringify(tasks));
},
clearLS: function() {
tasks = [];
localStorage.setItem('tasks', []);
}
};
})(UIController);

var controller = (function(UICtrl, storageCtrl) {
var DOM = UICtrl.getDOMStrings();
var setUpEventListeners = function() {
document.addEventListener('DOMContentLoaded', ctrlDisplayTasks);
document
.querySelector(DOM.submitBtn)
.addEventListener('click', ctrlAddTask);
document.addEventListener('keypress', function(event) {
if (event.keyCode === 13 || event.which === 13) {
Copy link

Choose a reason for hiding this comment

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

move keycode 13 to constants

Copy link
Author

Choose a reason for hiding this comment

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

Okay, thanks!

ctrlAddTask();
event.preventDefault();
}
});
document
.querySelector(DOM.taskList)
.addEventListener('click', ctrlDeleteTask);
document
.querySelector(DOM.taskList)
.addEventListener('click', ctrlToggleTask);
document
.querySelector(DOM.filterBtn)
.addEventListener('click', ctrlFilterTasks);
document
.querySelector(DOM.clearBtn)
.addEventListener('click', ctrlClearTasks);
};
var ctrlDisplayTasks = function() {
var tasks, taskList;
var html = '';
tasks = storageCtrl.getFromLS();
taskList = document.querySelector(DOM.taskList);
tasks.forEach(function(task) {
Copy link

Choose a reason for hiding this comment

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

use just renderTasks function that I mentioned in comment above here instead of code duplication

html += `
<li class="collection__item" id="item-${task.id}">
<span class="text">${task.text}</span>
<div class="icons">
<i class="${task.iconClass}"></i>
<i class="fas fa-backspace"></i>
</div>
<span class="deadline">Deadline: ${task.deadline}</span>
</li>
`;
});
taskList.innerHTML = html;
};
var ctrlAddTask = function() {
var input, newTask;
event.preventDefault();
input = UICtrl.getInput();
if (input.text !== '') {
newTask = storageCtrl.addTask(input.text, input.deadline, input.state);
storageCtrl.addToLS(newTask);
ctrlDisplayTasks();
UICtrl.clearFields();
}
};
var ctrlDeleteTask = function(event) {
var itemID, splitID, ID;
if (event.target.className === DOM.deleteBtn) {
itemID = event.target.parentNode.parentNode.id;
}
if (itemID) {
splitID = itemID.split('-');
ID = +splitID[1];
storageCtrl.deleteFromLS(ID);
UICtrl.deleteListItem(itemID);
}
};
var ctrlToggleTask = function(event) {
var itemID, splitID, ID, icon;
if (event.target.className === DOM.uncheckedBtn || DOM.checkedBtn) {
Copy link

Choose a reason for hiding this comment

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

if you wanna check that className of target equals to DOM.uncheckedBtn or DOM.checkedBtn you should write another condition, because current condition checks if className equals to DOM.uncheckedBtn or DOM.checkedBtn isn't equal to boolean false ( isn't empty string, undefined, 0 and so on)

Copy link
Author

Choose a reason for hiding this comment

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

In current case, all I want to do is to check if event.target.className matches any of these certain classNames. Isn't that correct if there's no match the boolean result is false?

Copy link

Choose a reason for hiding this comment

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

yeah, it's correct, but I meant that your condition isn't correct - try in console if (1 === 2 || 3) console.log('oh!')

Copy link
Author

Choose a reason for hiding this comment

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

Now I see. To be correct, I should've done code like this, I suppose: if (1 === 2 || 1 === 3) console.log('oh')

itemID = event.target.parentNode.parentNode.id;
icon = event.target;
}
if (itemID) {
splitID = itemID.split('-');
Copy link

Choose a reason for hiding this comment

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

move this logic to separated function getItemInfo that returns object with 2 fields - type and id

type = splitID[0];
ID = +splitID[1];
storageCtrl.toggleTask(ID);
UICtrl.toggleTask(icon);
}
};
var ctrlFilterTasks = function() {
var tasks, filterValue, DOM;
DOM = UICtrl.getDOMStrings();
tasks = storageCtrl.getFromLS();
filterValue = document.querySelector(DOM.filterInput).value;
if (filterValue === 'no filter') {
Copy link

Choose a reason for hiding this comment

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

move no filter value to some object with constants

ctrlDisplayTasks();
} else {
UICtrl.filterTasks(tasks);
}
event.preventDefault();
};
var ctrlClearTasks = function() {
event.preventDefault();
storageCtrl.clearLS();
UICtrl.clearTasks();
document.querySelector(DOM.filterInput).value = 'no filter';
};

return {
init: function() {
setUpEventListeners();
}
};
})(UIController, storageController);

controller.init();
Loading