patternjavascriptModerate
To-do list in jQuery
Viewed 0 times
listjquerystackoverflow
Problem
I'm making a simple to-do list with jQuery. This is my first step in JavaScript and jQuery. How can I improve my code? User enter tasks in input field, submit by pressing 'Enter'. On double-click, he can edit his record. He can mark check-box, and record will make completed. By clicking bottom check-box all records will be marked as completed. Button 'Clear competed' delete all records marked as completed.
`function addListItem()
{
var text = $('#new-text').val();
$("#todo-list").append(''
+ text + ' ');
$("#new-text").val('');
}
function clearCompleted()
{
$("#todo-list .toggle:checked").parent().remove();
}
function deleteItem()
{
$(this).parent().remove();
}
function completed() {
if
(
$(this).parent().css('textDecoration') == 'line-through' )
{
$(this).parent().css('textDecoration', 'none');
$(this).parent().css('opacity', '1');
}
else
{
$(this).parent().css('textDecoration', 'line-through');
$(this).parent().css('opacity', '0.50');
}
}
$(document).ready(function(){
$('#new-text').keyup(function(e)
{
if (e.keyCode === 13)
{
addListItem();
}
});
$(document).on('click', '.destroy', deleteItem);
$("#toggle-all").click(function ()
{
$('input:checkbox').not(this).prop('checked', this.checked);
if ( $('li').css('textDecoration') == 'line-through' )
{
$('li').css('textDecoration', 'none');
$('li').parent().css('opacity', '1');
}
else
{
$('li').css('textDecoration', 'line-through');
$('li').parent().css('opacity', '0.5');
}
});
$(document).on('click', '.toggle', completed);
$("#clearcompleted").click(clearCompleted);
$('#todo-list').on('dblclick', 'span', function ()
{
var thisData = this.innerHTML,
$e
`function addListItem()
{
var text = $('#new-text').val();
$("#todo-list").append(''
+ text + ' ');
$("#new-text").val('');
}
function clearCompleted()
{
$("#todo-list .toggle:checked").parent().remove();
}
function deleteItem()
{
$(this).parent().remove();
}
function completed() {
if
(
$(this).parent().css('textDecoration') == 'line-through' )
{
$(this).parent().css('textDecoration', 'none');
$(this).parent().css('opacity', '1');
}
else
{
$(this).parent().css('textDecoration', 'line-through');
$(this).parent().css('opacity', '0.50');
}
}
$(document).ready(function(){
$('#new-text').keyup(function(e)
{
if (e.keyCode === 13)
{
addListItem();
}
});
$(document).on('click', '.destroy', deleteItem);
$("#toggle-all").click(function ()
{
$('input:checkbox').not(this).prop('checked', this.checked);
if ( $('li').css('textDecoration') == 'line-through' )
{
$('li').css('textDecoration', 'none');
$('li').parent().css('opacity', '1');
}
else
{
$('li').css('textDecoration', 'line-through');
$('li').parent().css('opacity', '0.5');
}
});
$(document).on('click', '.toggle', completed);
$("#clearcompleted").click(clearCompleted);
$('#todo-list').on('dblclick', 'span', function ()
{
var thisData = this.innerHTML,
$e
Solution
-
Use CSS classes rather than inline styles. Instead of modifying the
This makes your code appear more fluent and language-like.
-
Remove multiple
Use CSS classes rather than inline styles. Instead of modifying the
style of each TODO item, use CSS classes. This offloads the styling into a style sheet where it belongs, and it still allows you to test an item to see if it is already completed:function completed() {
if ( !$(this).parent().hasClass('completed') ) {
$(this).parent().addClass('completed');
} else {
$(this).parent().removeClass('completed');
}
}This makes your code appear more fluent and language-like.
-
Remove multiple
$(this) calls. There is no need to call $(this) multiple times. The $ function returns a new jQuery object on every call. Save the return value of this call in a variable. Furthermore, the complete function constantly refers to $(this).parent(), so stash that as a variable:function completed() {
var $item = $(this).parent();
if ( !$item.hasClass('completed') ) {
$item.addClass('completed');
} else {
$item.removeClass('completed');
}
}Code Snippets
function completed() {
if ( !$(this).parent().hasClass('completed') ) {
$(this).parent().addClass('completed');
} else {
$(this).parent().removeClass('completed');
}
}function completed() {
var $item = $(this).parent();
if ( !$item.hasClass('completed') ) {
$item.addClass('completed');
} else {
$item.removeClass('completed');
}
}Context
StackExchange Code Review Q#93664, answer score: 14
Revisions (0)
No revisions yet.