From 3070d280eae315bdf7d1fe3fd309c23dc0a6b3d9 Mon Sep 17 00:00:00 2001 From: lukemelia Date: Tue, 6 Feb 2007 05:42:40 +0000 Subject: [PATCH] * Apply unobtrusive javascript principles to _item partial, and reduce the amount of inline javascript throughout the app by a lot. * Cleaned up the _item partial, moving logic into todo_helper methods. * Patched the unobtrusive_javascript plugin to avoid overflow of the session storage. I've submitted this patch by email to Luke Redpath, and hopefully, the plugin will incorporate this or similar functionality in the future. git-svn-id: http://www.rousette.org.uk/svn/tracks-repos/trunk@433 a4c988fc-2ded-0310-b66e-134b36920a42 --- tracks/app/helpers/todos_helper.rb | 87 ++++++++++++++----- .../app/views/contexts/_context_listing.rhtml | 4 +- tracks/app/views/notes/_notes_summary.rhtml | 2 +- tracks/app/views/todos/_item.rhtml | 69 +++------------ tracks/app/views/todos/_toggle_notes.rhtml | 13 +-- tracks/db/schema.rb | 28 +++--- tracks/public/stylesheets/standard.css | 5 +- tracks/test/selenium/notes/link_to_note.rsel | 2 +- .../lib/ujs/behaviour_script.rb | 6 +- 9 files changed, 112 insertions(+), 104 deletions(-) diff --git a/tracks/app/helpers/todos_helper.rb b/tracks/app/helpers/todos_helper.rb index ec56e756..1f473ea0 100644 --- a/tracks/app/helpers/todos_helper.rb +++ b/tracks/app/helpers/todos_helper.rb @@ -9,33 +9,78 @@ module TodosHelper def form_remote_tag_edit_todo( item, &block ) form_tag( todo_path(item), {:method => :put, :id => dom_id(item, 'form'), :class => "edit_todo_form inline-form" }, &block ) - apply_behavior 'form.edit_todo_form:submit', - remote_function(:url => javascript_variable('this.action'), :method => :put, :form => true), - :prevent_default => true + apply_behavior 'form.edit_todo_form', make_remote_form(:method => :put), :prevent_default => true end - def link_to_remote_todo(item) - itemurl = todo_path(:id => item.id, :_source_view => @source_view) - - str = link_to_remote( image_tag_for_delete, - { :url => todo_path(:id => item.id, :_source_view => @source_view), - :method => :delete, - :confirm => "Are you sure that you want to delete the action, \'#{item.description}\'?" }, - { :class => "icon" } - ) + "\n" - if !item.completed? - str << link_to_remote( image_tag_for_edit(item), - { :url => edit_todo_path(:id => item.id, :_source_view => @source_view), - :method => 'get', - :loading => visual_effect(:pulsate, dom_id(item, 'edit_icon')) }, - { :class => "icon" } - ) - else - str << '' + image_tag("blank.png") + " " + def remote_delete_icon(item) + str = link_to( image_tag("blank.png", :title =>"Delete action", :class=>"delete_item"), + todo_path(item), + :class => "icon delete_icon", :title => "delete the action '#{item.description}'") + apply_behavior '.item-container a.delete_icon:click', :prevent_default => true do |page| + page << "if (confirm('Are you sure that you want to ' + this.title + '?')) {" + page << " new Ajax.Request(this.href, { asynchronous : true, evalScripts : true, method : 'delete', parameters : { '_source_view' : '#{@source_view}' }})" + page << "}" end str end + def remote_edit_icon(item) + if !item.completed? + str = link_to( image_tag_for_edit(item), + edit_todo_path(item), + :class => "icon edit_icon") + apply_behavior '.item-container a.edit_icon:click', :prevent_default => true do |page| + page << "new Ajax.Request(this.href, { asynchronous : true, evalScripts : true, method : 'get', parameters : { '_source_view' : '#{@source_view}' }, onLoading: function(request){ Effect.Pulsate(this)}});" + end + else + str = '' + image_tag("blank.png") + " " + end + str + end + + def remote_toggle_checkbox(item) + str = check_box_tag('item_id', item.id, item.completed?, :class => 'item-checkbox', :url => toggle_check_todo_path(item)) + apply_behavior '.item-container input.item-checkbox:click', + remote_function(:url => javascript_variable('this.attributes.url.value'), + :with => "{ method : 'post', _source_view : '#{@source_view}' }") + str + end + + def date_span(item) + if item.completed? + "#{format_date( item.completed_at )}" + elsif item.deferred? + show_date( item.show_from ) + else + due_date( item.due ) + end + end + + def tag_list(item) + item.tags.collect{|t| "" + link_to(t.name, :action => "tag", :id => t.name) + ""}.join('') + end + + def deferred_due_date(item) + if item.deferred? && item.due + "(action due on #{format_date(item.due)})" + end + end + + def project_and_context_links(item, parent_container_type) + if item.completed? + "(#{item.context.name}#{", " + item.project.name unless item.project.nil?})" + else + str = '' + if (['project', 'tickler', 'tag'].include?(parent_container_type)) + str << item_link_to_context( item ) + end + if (['context', 'tickler', 'tag'].include?(parent_container_type)) && item.project_id + str << item_link_to_project( item ) + end + str + end + end + # Uses the 'staleness_starts' value from settings.yml (in days) to colour # the background of the action appropriately according to the age # of the creation date: diff --git a/tracks/app/views/contexts/_context_listing.rhtml b/tracks/app/views/contexts/_context_listing.rhtml index 5e1bab92..b9f669f6 100644 --- a/tracks/app/views/contexts/_context_listing.rhtml +++ b/tracks/app/views/contexts/_context_listing.rhtml @@ -17,10 +17,10 @@ <%= image_tag( "blank.png", :title => "Delete context", :class=>"delete_item") %> <% apply_behavior "a.delete_context_button:click", :prevent_default => true do |page| page << "if (confirm('Are you sure that you want to ' + this.title + '?')) {" - page << " new Ajax.Updater($(this).up('.list')," + page << " new Ajax.Updater(this.up('.list')," page << " this.href, {asynchronous:true," page << " evalScripts:true, method:'delete'," - page << " onLoading:function(request){ Effect.Fade($(this).up('.list'));}}); };" + page << " onLoading:function(request){ Effect.Fade(this.up('.list'));}}); };" end -%> <%= image_tag( "blank.png", :title => "Edit context", :class=>"edit_item") %> <% apply_behavior 'a.edit_context_button:click', {:prevent_default => true } do |page, element| diff --git a/tracks/app/views/notes/_notes_summary.rhtml b/tracks/app/views/notes/_notes_summary.rhtml index 25373930..9eb70db0 100644 --- a/tracks/app/views/notes/_notes_summary.rhtml +++ b/tracks/app/views/notes/_notes_summary.rhtml @@ -1,6 +1,6 @@ <% note = notes_summary -%>
-<%= link_to( image_tag("blank.png", :border => 0), note_path(note), :title => "Show note", :class => "show_notes icon") %>  +<%= link_to( image_tag("blank.png", :border => 0), note_path(note), :title => "Show note", :class => "link_to_notes icon") %>  <%= truncated_note(note) %>
<% note = nil -%> diff --git a/tracks/app/views/todos/_item.rhtml b/tracks/app/views/todos/_item.rhtml index c567662f..c65aa528 100644 --- a/tracks/app/views/todos/_item.rhtml +++ b/tracks/app/views/todos/_item.rhtml @@ -1,65 +1,22 @@ <% Tag %>
- <%= link_to_remote_todo item %> - - <% unless source_view_is :deferred -%> - checked="checked" <% end %> /> - <% apply_behavior '.item-container input.item-checkbox:click', - remote_function(:url => javascript_variable('this.attributes.url.value'), :with => "{ method : 'post', _source_view : '#{@source_view}' }") %> - <% end -%> - -
<% # start of div which has a class 'description', and possibly 'stale_11', 'stale_12', 'stale_13' etc %> - <% if item.completed? -%> - <%= format_date( item.completed_at ) %> - <% elsif item.deferred? -%> - <%= show_date( item.show_from ) -%> - <% else -%> - <%= due_date( item.due ) -%> - <% end -%> - + <%= remote_delete_icon( item ) %> + <%= remote_edit_icon( item ) %> + <%= remote_toggle_checkbox( item ) unless source_view_is :deferred %> +
+ <%= date_span( item ) %> <%= sanitize(item.description) %> - - <%= if item.tag_list.blank? - "" - else - tag_string = "" - item.tags.each do |t| - tag_string << "" + link_to(t.name, :action => "tag", :id => t.name) + "" - end - tag_string - end - %> - - <% if item.deferred? && item.due -%> - (action due on <%= format_date(item.due) %>) - <% end -%> - - <% if item.completed? -%> - (<%= item.context.name %><%= ", " + item.project.name unless item.project.nil? %>) - <% else -%> - <% if (parent_container_type == "project" || parent_container_type == "tickler") -%> - <%= item_link_to_context( item ) %> - <% end -%> - <% if (parent_container_type == "context" || parent_container_type == "tickler") && item.project_id -%> - <%= item_link_to_project( item ) %> - <% end -%> - <% if (parent_container_type == "tag") -%> - <%= item_link_to_context( item ) %> - <%= item_link_to_project( item ) if item.project_id %> - <% end -%> - <% end -%> - - <% if item.notes? -%> - <%= render :partial => "todos/toggle_notes", :locals => { :item => item } %> - <% end -%> + <%= tag_list( item ) %> + <%= deferred_due_date( item ) %> + <%= project_and_context_links( item, parent_container_type ) %> + <%= render(:partial => "todos/toggle_notes", :locals => { :item => item }) if item.notes? %>
-
+
-
+ + diff --git a/tracks/app/views/todos/_toggle_notes.rhtml b/tracks/app/views/todos/_toggle_notes.rhtml index 8a8b1586..6ccab15d 100644 --- a/tracks/app/views/todos/_toggle_notes.rhtml +++ b/tracks/app/views/todos/_toggle_notes.rhtml @@ -1,7 +1,8 @@ -<%= link_to_function(image_tag( 'blank.png', :width=>'16', :height=>'16', :border=>'0' ), nil, {:class => 'show_notes', :title => 'Show notes'}) do |page| - page[dom_id(item, 'notes')].toggle - end +<%= link_to(image_tag( 'blank.png', :width=>'16', :height=>'16', :border=>'0' ), nil, {:class => 'show_notes', :title => 'Show notes'}) %> +<% apply_behavior 'a.show_notes:click', :prevent_default => true do |page, element| + element.next('.notes').toggle + end -%> - \ No newline at end of file + \ No newline at end of file diff --git a/tracks/db/schema.rb b/tracks/db/schema.rb index a937bdbb..5afc62ac 100644 --- a/tracks/db/schema.rb +++ b/tracks/db/schema.rb @@ -5,14 +5,15 @@ ActiveRecord::Schema.define(:version => 27) do create_table "contexts", :force => true do |t| - t.column "name", :string, :default => "", :null => false - t.column "position", :integer, :default => 0, :null => false - t.column "hide", :boolean, :default => false - t.column "user_id", :integer, :default => 1 + t.column "name", :string, :default => "", :null => false + t.column "hide", :integer, :limit => 4, :default => 0, :null => false + t.column "position", :integer, :default => 0, :null => false + t.column "user_id", :integer, :default => 0, :null => false t.column "created_at", :datetime t.column "updated_at", :datetime end + add_index "contexts", ["user_id"], :name => "index_contexts_on_user_id" add_index "contexts", ["user_id", "name"], :name => "index_contexts_on_user_id_and_name" create_table "notes", :force => true do |t| @@ -63,13 +64,14 @@ ActiveRecord::Schema.define(:version => 27) do create_table "projects", :force => true do |t| t.column "name", :string, :default => "", :null => false t.column "position", :integer, :default => 0, :null => false - t.column "user_id", :integer, :default => 1 + t.column "user_id", :integer, :default => 0, :null => false t.column "description", :text t.column "state", :string, :limit => 20, :default => "active", :null => false t.column "created_at", :datetime t.column "updated_at", :datetime end + add_index "projects", ["user_id"], :name => "index_projects_on_user_id" add_index "projects", ["user_id", "name"], :name => "index_projects_on_user_id_and_name" create_table "sessions", :force => true do |t| @@ -98,16 +100,16 @@ ActiveRecord::Schema.define(:version => 27) do add_index "tags", ["name"], :name => "index_tags_on_name" create_table "todos", :force => true do |t| - t.column "context_id", :integer, :default => 0, :null => false - t.column "project_id", :integer - t.column "description", :string, :default => "", :null => false + t.column "context_id", :integer, :default => 0, :null => false + t.column "description", :string, :limit => 100, :default => "", :null => false t.column "notes", :text t.column "created_at", :datetime t.column "due", :date t.column "completed_at", :datetime - t.column "user_id", :integer, :default => 1 + t.column "project_id", :integer + t.column "user_id", :integer, :default => 0, :null => false t.column "show_from", :date - t.column "state", :string, :limit => 20, :default => "immediate", :null => false + t.column "state", :string, :limit => 20, :default => "immediate", :null => false end add_index "todos", ["user_id", "state"], :name => "index_todos_on_user_id_and_state" @@ -117,10 +119,10 @@ ActiveRecord::Schema.define(:version => 27) do add_index "todos", ["user_id", "context_id"], :name => "index_todos_on_user_id_and_context_id" create_table "users", :force => true do |t| - t.column "login", :string, :limit => 80, :default => "", :null => false - t.column "password", :string, :limit => 40, :default => "", :null => false + t.column "login", :string, :limit => 80 + t.column "password", :string, :limit => 40 t.column "word", :string - t.column "is_admin", :boolean, :default => false, :null => false + t.column "is_admin", :integer, :limit => 4, :default => 0, :null => false t.column "first_name", :string t.column "last_name", :string t.column "auth_type", :string, :default => "database", :null => false diff --git a/tracks/public/stylesheets/standard.css b/tracks/public/stylesheets/standard.css index 7cc001e0..88ab82b7 100644 --- a/tracks/public/stylesheets/standard.css +++ b/tracks/public/stylesheets/standard.css @@ -45,9 +45,8 @@ a.down:hover {background: transparent url(../images/down_on.png) no-repeat;} a.to_bottom {background: transparent url(../images/bottom_off.png) no-repeat;} a.to_bottom:hover {background: transparent url(../images/bottom_on.png) no-repeat;} -a.show_notes {background-image: url(../images/notes_off.png); background-repeat: no-repeat; padding: 1px; background-color: transparent;} -a.show_notes:hover {background-image: url(../images/notes_on.png); background-repeat: no-repeat; padding: 1px; background-color: transparent;} - +a.show_notes, a.link_to_notes {background-image: url(../images/notes_off.png); background-repeat: no-repeat; padding: 1px; background-color: transparent;} +a.show_notes:hover, a.link_to_notes:hover {background-image: url(../images/notes_on.png); background-repeat: no-repeat; padding: 1px; background-color: transparent;} /* Structural divs */ diff --git a/tracks/test/selenium/notes/link_to_note.rsel b/tracks/test/selenium/notes/link_to_note.rsel index 01e90f2a..ef966df3 100644 --- a/tracks/test/selenium/notes/link_to_note.rsel +++ b/tracks/test/selenium/notes/link_to_note.rsel @@ -1,5 +1,5 @@ setup :fixtures => :all include_partial 'login/login', :username => 'admin', :password => 'abracadabra' open "/projects/Build_a_working_time_machine" -click_and_wait "css=#note_1 .show_notes" +click_and_wait "css=#note_1 .link_to_notes" assert_element_present "note_1" diff --git a/tracks/vendor/plugins/unobtrusive_javascript/lib/ujs/behaviour_script.rb b/tracks/vendor/plugins/unobtrusive_javascript/lib/ujs/behaviour_script.rb index e50f174d..81ec75d5 100644 --- a/tracks/vendor/plugins/unobtrusive_javascript/lib/ujs/behaviour_script.rb +++ b/tracks/vendor/plugins/unobtrusive_javascript/lib/ujs/behaviour_script.rb @@ -8,7 +8,7 @@ class UJS::BehaviourScript def add_rule(selector, javascript, cancel_default=false) javascript = javascript << cancel_default_js if cancel_default - @rules << [selector, javascript] + @rules << [selector, javascript] unless rule_exists(selector, javascript) end def cache? @@ -51,4 +51,8 @@ class UJS::BehaviourScript def cancel_default_js " return false;" end + + def rule_exists(selector, javascript) + @rules.detect{|r| r[0] == selector && r[1] == javascript} != nil + end end \ No newline at end of file