Changed UI design/action link placement.

Modified recurrence listing in issue view. Updated look to better match Redmine.

app/controllers/recurring_tasks_controller.rb

@@ -7,6 +7,7 @@ class RecurringTasksController < ApplicationController
   before_filter :find_recurring_task, :except => [:index, :new, :create]
   before_filter :set_interval_units, :except => [:index, :show]
   before_filter :set_recurrable_issues, :except => [:index, :show]
+  before_filter :cancel_edit, :only => [:new, :create, :edit, :update]
 
   def index
     @recurring_tasks = RecurringTask.all_for_project(@project)
@@ -36,6 +37,14 @@ def edit
     # default behavior is fine
   end
 
+  def cancel_edit
+    if params[:commit] == l(:button_cancel)
+      redirect_to :back
+    end
+  rescue ActionController::RedirectBackError
+    redirect_to default
+  end
+
   # saves the task and redirects to issue view
   def update
     logger.info "Updating recurring task #{params[:id]}"
@@ -54,10 +63,10 @@ def destroy
 
     if @recurring_task.destroy
       flash[:notice] = l(:recurring_task_removed)
-      redirect_to :action => :index
+      redirect_to :back
     else
       flash[:notice] = l(:error_recurring_task_could_not_remove)
-      render :edit
+      render :back
     end
   end
 

app/helpers/recurring_tasks_helper.rb

@@ -6,7 +6,7 @@ def show_error msg
 
   def delete_button recurring_task
     if User.current.allowed_to?(:delete_issue_recurrence, recurring_task.project)
-      button_to(l(:button_delete), {:action => 'destroy', :id => recurring_task, :project_id => recurring_task.project.id}, :method => :delete, :class => 'icon icon-del', :confirm => l(:text_are_you_sure))
+      link_to(l(:button_delete), {:action => 'destroy', :id => recurring_task, :project_id => recurring_task.project.id}, :method => :delete, :class => 'icon icon-del', :confirm => l(:text_are_you_sure))
     end
   end
 

app/models/recurring_task.rb

@@ -216,7 +216,7 @@ def recurrence_to_s
     frequency = (interval_number == 1) ? "" : interval_number.ordinalize
     modifier = (interval_unit == INTERVAL_MONTH) ? " #{interval_localized_modifier}" : ""
     schedule = fixed_schedule ? l(:label_recurs_fixed) : l(:label_recurs_dependent)
-    "#{l(:label_recurrence_pattern)} #{frequency} #{interval_localized_name}#{modifier}, #{schedule} (#{l(:label_next_scheduled_run).downcase}: #{next_scheduled_recurrence})"
+    "#{l(:label_recurrence_pattern)} #{frequency} #{interval_localized_name}#{modifier}, #{schedule}"
   end
 
   def to_s

app/views/issues/_show_recurrence.html.erb

@@ -1,18 +1,33 @@
 <hr />
-<tr>
-	<td class="recurrence" colspan="2"></th>
-		<% if User.current.allowed_to?(:add_issue_recurrence, project) %>		
-		    <div class="contextual"><%= link_to(l(:label_add_recurring_task), new_recurring_task_path(:issue_id => issue.id, :project_id => project.id), :class => 'icon icon-add') %></div>
-		<% end %>
-		<p><strong><%= l(:field_recurrence)%></strong></p>
-		<% if issue.recurs? %>
-			<ul>
-				<% issue.recurring_tasks.each do |rt| %>
-					<li><%= if User.current.allowed_to?(:view_issue_recurrence, project) then link_to(rt.recurrence_to_s, edit_recurring_task_path(:id => rt.id, :project_id => project.id)) else rt.recurrence_to_s end %></li>
-				<% end %>
-			</ul>
-		<% else %>
-			<p><%= l(:label_no_recurrence)%></p>
-		<% end %>
-	</td>
-</tr>
+<div class="recurrence">
+  <% if User.current.allowed_to?(:add_issue_recurrence, project) %>
+      <div class="contextual"><%= link_to(l(:label_add_recurring_task), new_recurring_task_path(:issue_id => issue.id, :project_id => project.id)) %></div>
+  <% end %>
+  <p><strong><%= l(:field_recurrence)%></strong></p>
+  <% if issue.recurs? %>
+    <table class="list">
+    <tbody>
+      <% issue.recurring_tasks.each do |rt| %>
+        <tr>
+          <% if User.current.allowed_to?(:view_issue_recurrence, project) %>
+            <td class="subject">
+              <%= link_to(rt.recurrence_to_s, edit_recurring_task_path(:id => rt.id, :project_id => project.id)) %>
+            </td>
+            <td>
+              <%= "#{l(:label_next_scheduled_run)}: #{rt.next_scheduled_recurrence}" %>
+            </td>
+            <td class="buttons">
+              <%= link_to(l(:button_edit), edit_recurring_task_path(:id => rt.id, :project_id => project.id), :class => 'icon icon-edit') %>
+              <%= link_to(l(:button_delete), destroy_recurring_task_path(:id => rt.id, :project_id => project.id), :method => :delete, :class => 'icon icon-del') %></td>
+          <% else %>
+            <td colspan=3><%= rt.recurrence_to_s %></td>
+          <% end %>
+        </tr>
+      <% end %>
+    </tbody>
+    </table>
+  <% else %>
+    <p><%= l(:label_no_recurrence)%></p>
+  <% end %>
+</div>
+

app/views/recurring_tasks/_form.html.erb

@@ -1,16 +1,17 @@
-<div class="box tabular"> 
-  <%= labelled_form_for rt, :url => { :action => next_step, :id=>rt } do |f| %>
-    <p><%= error_messages_for 'recurring_task' %></p>
-    <p><%= f.hidden_field :id %></p>
+<%= labelled_form_for rt, :url => { :action => next_step, :id=>rt } do |f| %>
+  <div class="box tabular">
+    <%= error_messages_for 'recurring_task' %>
+    <%= f.hidden_field :id %>
     <p><%= label(:recurring_task, :current_issue_id, l(:field_issue)) %><%= collection_select('recurring_task', 'current_issue_id', @recurrable_issues, :id, :subj_date) %></p>
-    <p><%= f.number_field :interval_number %></p>
-    <p><%= label(:recurring_task, :interval_unit, l(:field_interval_unit)) %><%= select 'recurring_task', 'interval_unit', @interval_units, {}, :onChange =>"document.getElementById('interval_modifier_block').style.display=(this.value=='"+ RecurringTask::INTERVAL_MONTH + "' ? 'block' : 'none')" %></p>
-    <% modifier_display = (rt.interval_unit == RecurringTask::INTERVAL_MONTH) ? "display:block" : "display:none" %>
+    <p><%= f.number_field :interval_number, :size => 4, :maxlength => 4 %></p>
+    <p><%= f.select :interval_unit, @interval_units, {}, :onChange =>"document.getElementById('interval_modifier_block').style.display=(this.value=='"+ RecurringTask::INTERVAL_MONTH + "' ? 'block' : 'none')" %></p>
+    <% modifier_display = (rt.interval_unit == RecurringTask::INTERVAL_MONTH) ? "display:inline" : "display:none" %>
     <%= content_tag :div, :id => "interval_modifier_block", :style => modifier_display do -%>
-      <p><%= label(:recurring_task, :interval_modifier, l(:field_interval_modifier)) %><%= select 'recurring_task', 'interval_modifier', rt.get_modifiers_descriptions.collect{|k,v| [v, k]} %></p>
+      <p><%= f.select 'interval_modifier', rt.get_modifiers_descriptions.collect{|k,v| [v, k]} %></p>
     <% end -%>
-    <p><%= f.check_box :fixed_schedule %> </p>
-    <p><%= f.submit %></p>
-  <% end %>
-</div>
+    <p><%= f.check_box :fixed_schedule %></p>
+  </div>
+  <%= f.submit %>
+  <%= link_to l(:button_cancel), :back %>
+<% end %>
 

app/views/recurring_tasks/edit.html.erb

@@ -2,5 +2,3 @@
 
 <%= render :partial => "form", :locals => {:next_step => 'update', :rt => @recurring_task} %>
 
-<p><%= link_to l(:button_cancel), "#", :onclick => 'Element.hide("add-task")' %></p>
-<p><%= delete_button @recurring_task %></p>

app/views/recurring_tasks/index.html.erb

@@ -1,12 +1,11 @@
 <div class="contextual">
-    <%= link_to(l(:label_add_recurring_task), {:action => 'new'}, :class => 'icon icon-add') %>
+  <%= link_to(l(:label_add_recurring_task), {:action => 'new'}, :class => 'icon icon-add') %>
 </div>
 
 <h2><%= l(:label_recurring_tasks) %></h2>
 
 <% if @recurring_tasks.nil? or @recurring_tasks.empty? %>
     <p><%= l(:label_no_recurring_tasks)%></p>
-
 <% else %>
     <!-- at least one recurring task exists -->
     <table class="list">
@@ -29,7 +28,7 @@
                     <td><%= pluralize(rt.interval_number, rt.interval_localized_name) %></td>
                     <td><%= rt.fixed_schedule %></td>
                     <td><%= format_date(rt.next_scheduled_recurrence) %></td>
-                    <td><%= edit_button rt %></td>
+                    <td class="buttons"><%= delete_button rt %></td>
 	             <% rescue => e %>
 	             	<td><%= "ERROR: #{e}" %></td>
 	             <% end %>
@@ -38,3 +37,4 @@
         </tbody>
     </table>
 <% end %>
+

app/views/recurring_tasks/new.html.erb

@@ -1,3 +1,4 @@
 <h2><%= l(:label_add_recurring_task) %></h2>
+
 <%= render :partial => "form", :locals => { :next_step => 'create', :rt => @recurring_task } %>
-<p><%= link_to l(:button_cancel), "#", :onclick => 'Element.hide("add-task")' %></p>
+

config/locales/en.yml

@@ -3,7 +3,7 @@ en:
   label_recurring_tasks:    "Recurring Issues"
   label_current_issue:      "Current Issue for Recurrence"
   label_recurrence_pattern: "Recurs every"
-  label_add_recurring_task: "Add Recurrence"
+  label_add_recurring_task: "Add"
   label_no_recurring_tasks: "No recurring issues found on the system."
   label_next_scheduled_run: "Next scheduled run"
   label_no_recurrence:      "No recurrence."