* 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

@@ -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 << '<a class="icon">' + image_tag("blank.png") + "</a> "
+  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 = '<a class="icon">' + image_tag("blank.png") + "</a> "
+    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?
+      "<span class=\"grey\">#{format_date( item.completed_at )}</span>"
+    elsif item.deferred?
+      show_date( item.show_from )
+    else
+      due_date( item.due ) 
+    end    
+  end
+  
+  def tag_list(item)
+    item.tags.collect{|t| "<span class=\"tag\">" + link_to(t.name, :action => "tag", :id => t.name) + "</span>"}.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:

tracks/app/views/contexts/_context_listing.rhtml

@@ -17,10 +17,10 @@
       <a class="delete_context_button" href="<%= formatted_context_path(context, :js) %>" title="delete the context '<%= context.name %>'"><%= image_tag( "blank.png", :title => "Delete context", :class=>"delete_item") %></a>
       <% 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 -%>
       <a class="edit_context_button" href="#"><%= image_tag( "blank.png", :title => "Edit context", :class=>"edit_item") %></a>
       <% apply_behavior 'a.edit_context_button:click', {:prevent_default => true } do |page, element|

tracks/app/views/notes/_notes_summary.rhtml

@@ -1,6 +1,6 @@
 <% note = notes_summary -%>
 <div class="note_wrapper" id="<%= dom_id(note) %>">
-<%= link_to( image_tag("blank.png", :border => 0), note_path(note), :title => "Show note", :class => "show_notes icon") %>&nbsp;
+<%= link_to( image_tag("blank.png", :border => 0), note_path(note), :title => "Show note", :class => "link_to_notes icon") %>&nbsp;
 <%= truncated_note(note) %>
 </div>
 <% note = nil -%>

tracks/app/views/todos/_item.rhtml

@@ -1,65 +1,22 @@
 <% Tag %>
 <div id="<%= dom_id(item) %>" class="item-container">
   <div id="<%= dom_id(item, 'line') %>">
-    <%= link_to_remote_todo item %>
-
-    <% unless source_view_is :deferred -%>
-      <input type="checkbox" class="item-checkbox" name="item_id" url="<%= toggle_check_todo_path(item) %>"
-             value="<%= item.id %>"<% if item.completed? %> 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 -%>
-
-    <div class="description<%= staleness_class( item ) %>"><% # start of div which has a class 'description', and possibly 'stale_11', 'stale_12', 'stale_13' etc %>
-      <% if item.completed? -%>
-        <span class="grey"><%= format_date( item.completed_at ) %></span>
-      <% 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 %>
+    <div class="description<%= staleness_class( item ) %>">
+      <%= date_span( item ) %>
       <%= sanitize(item.description) %>
-      
-      <%= if item.tag_list.blank?
-            ""
-          else
-            tag_string = ""
-            item.tags.each do |t|
-              tag_string << "<span class=\"tag\">" + link_to(t.name, :action => "tag", :id => t.name) + "</span>"
-            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? %>
     </div>
-  </div><!-- [end:<%= dom_id(item, 'line') %>] -->
+  </div>
   <div id="<%= dom_id(item, 'edit') %>" class="edit-form" style="display:none">
     <% form_remote_tag_edit_todo( item ) do -%>
     <% #note: edit form will load here remotely -%>
-      <div class="placeholder"> </div>
+    <div class="placeholder"> </div>
     <% end -%>
-  </div><!-- [end:<%= dom_id(item, 'edit') %>] -->
-</div><!-- [end:<%= dom_id(item) %>] -->
+  </div>
+</div>

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
 -%>
-<div class="notes" id="<%= dom_id(item, 'notes') %>" style="display:none">
-	<%= markdown( item.notes ) %>
-</div>
+      <div class="notes" id="<%= dom_id(item, 'notes') %>" style="display:none">
+        <%= markdown( item.notes ) %>
+      </div>

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

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 */
 

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"

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