From dc76ef612a7cc6108db945c20e0d4036f53fe1a8 Mon Sep 17 00:00:00 2001
From: Jan Schulz-Hofen <jan@plan.io>
Date: Wed, 17 May 2017 15:42:34 +0200
Subject: [PATCH 2/2] =?UTF-8?q?Switch=20default=20behaviour=20on=20Attachm?=
 =?UTF-8?q?ent=20click=20to=20=E2=80=9Cview=E2=80=9D=20instead=20of=20?=
 =?UTF-8?q?=E2=80=9Cdownload=E2=80=9D=20and=20improve=20style=20of=20attac?=
 =?UTF-8?q?hment=20lists?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

---
 app/helpers/issues_helper.rb          | 10 +++-----
 app/models/attachment.rb              |  6 ++++-
 app/views/attachments/_links.html.erb | 45 +++++++++++++++++++----------------
 app/views/files/index.html.erb        |  3 ++-
 4 files changed, 34 insertions(+), 30 deletions(-)

diff --git a/app/helpers/issues_helper.rb b/app/helpers/issues_helper.rb
index 7036079..c049744 100644
--- a/app/helpers/issues_helper.rb
+++ b/app/helpers/issues_helper.rb
@@ -468,14 +468,10 @@ module IssuesHelper
       if detail.property == 'attachment' && value.present? &&
           atta = detail.journal.journalized.attachments.detect {|a| a.id == detail.prop_key.to_i}
         # Link to the attachment if it has not been removed
-        value = link_to_attachment(atta, :download => true, :only_path => options[:only_path])
-        if options[:only_path] != false && (atta.is_text? || atta.is_image?)
+        value = link_to_attachment(atta, only_path: options[:only_path])
+        if options[:only_path] != false
           value += ' '
-          value += link_to(l(:button_view),
-                           { :controller => 'attachments', :action => 'show',
-                             :id => atta, :filename => atta.filename },
-                           :class => 'icon-only icon-magnifier',
-                           :title => l(:button_view))
+          value += link_to_attachment atta, class: 'icon-only icon-save', title: l(:button_download), download: true
         end
       else
         value = content_tag("i", h(value)) if value
diff --git a/app/models/attachment.rb b/app/models/attachment.rb
index 89b9f8a..e2bc570 100644
--- a/app/models/attachment.rb
+++ b/app/models/attachment.rb
@@ -31,7 +31,7 @@ class Attachment < ActiveRecord::Base
   attr_protected :id
 
   acts_as_event :title => :filename,
-                :url => Proc.new {|o| {:controller => 'attachments', :action => 'download', :id => o.id, :filename => o.filename}}
+                :url => Proc.new {|o| {:controller => 'attachments', :action => 'show', :id => o.id, :filename => o.filename}}
 
   acts_as_activity_provider :type => 'files',
                             :permission => :view_files,
@@ -251,6 +251,10 @@ class Attachment < ActiveRecord::Base
     Redmine::MimeType.of(filename) == "application/pdf"
   end
 
+  def previewable?
+    is_text? || is_image?
+  end
+
   # Returns true if the file is readable
   def readable?
     disk_filename.present? && File.readable?(diskfile)
diff --git a/app/views/attachments/_links.html.erb b/app/views/attachments/_links.html.erb
index 797f8bb..6dee6d2 100644
--- a/app/views/attachments/_links.html.erb
+++ b/app/views/attachments/_links.html.erb
@@ -6,29 +6,32 @@
               :class => 'icon-only icon-edit'
              ) if options[:editable] %>
 </div>
+<table>
 <% for attachment in attachments %>
-<p><%= link_to_attachment attachment, :class => 'icon icon-attachment', :download => true -%>
-  <% if attachment.is_text? || attachment.is_image? %>
-    <%= link_to l(:button_view),
-                { :controller => 'attachments', :action => 'show',
-                  :id => attachment, :filename => attachment.filename },
-                :class => 'icon-only icon-magnifier',
-                :title => l(:button_view) %>
-  <% end %>
-  <%= " - #{attachment.description}" unless attachment.description.blank? %>
-  <span class="size">(<%= number_to_human_size attachment.filesize %>)</span>
-  <% if options[:deletable] %>
-    <%= link_to l(:button_delete), attachment_path(attachment),
-                :data => {:confirm => l(:text_are_you_sure)},
-                :method => :delete,
-                :class => 'delete icon-only icon-del',
-                :title => l(:button_delete) %>
-  <% end %>
-  <% if options[:author] %>
-    <span class="author"><%= attachment.author %>, <%= format_time(attachment.created_on) %></span>
-  <% end %>
-  </p>
+<tr>
+  <td>
+    <%= link_to_attachment attachment, class: 'icon icon-attachment' -%>
+    <span class="size">(<%= number_to_human_size attachment.filesize %>)</span>
+  </td>
+  <td><%= attachment.description unless attachment.description.blank? %></td>
+  <td>
+    <% if options[:author] %>
+      <span class="author"><%= attachment.author %>, <%= format_time(attachment.created_on) %></span>
+    <% end %>
+  </td>
+  <td><%= link_to_attachment attachment, class: 'icon-only icon-save', title: l(:button_download), download: true -%></td>
+  <td>
+    <% if options[:deletable] %>
+      <%= link_to l(:button_delete), attachment_path(attachment),
+                  :data => {:confirm => l(:text_are_you_sure)},
+                  :method => :delete,
+                  :class => 'delete icon-only icon-del',
+                  :title => l(:button_delete) %>
+    <% end %>
+  </td>
+</tr>
 <% end %>
+</table>
 <% if defined?(thumbnails) && thumbnails %>
   <% images = attachments.select(&:thumbnailable?) %>
   <% if images.any? %>
diff --git a/app/views/files/index.html.erb b/app/views/files/index.html.erb
index 5c827c1..cfb83e7 100644
--- a/app/views/files/index.html.erb
+++ b/app/views/files/index.html.erb
@@ -27,12 +27,13 @@
   <% end -%>
   <% container.attachments.each do |file| %>
   <tr class="file">
-    <td class="filename"><%= link_to_attachment file, :download => true, :title => file.description %></td>
+    <td class="filename"><%= link_to_attachment file, :title => file.description -%></td>
     <td class="created_on"><%= format_time(file.created_on) %></td>
     <td class="filesize"><%= number_to_human_size(file.filesize) %></td>
     <td class="downloads"><%= file.downloads %></td>
     <td class="digest"><%= file.digest_type %>: <%= file.digest %></td>
     <td class="buttons">
+    <%= link_to_attachment file, class: 'icon-only icon-save', title: l(:button_download), download: true %>
     <%= link_to(image_tag('delete.png'), attachment_path(file),
                                          :data => {:confirm => l(:text_are_you_sure)}, :method => :delete) if delete_allowed %>
     </td>
-- 
2.7.2