Feature #29752

Render Textile and Markdown attachments on the preview page

Added by Go MAEDA about 1 year ago. Updated 18 days ago.

Status:ClosedStart date:
Priority:NormalDue date:
Assignee:Go MAEDA% Done:

0%

Category:Attachments
Target version:4.1.0
Resolution:Fixed

Description

Currently, Textile and Markdown files are treated as plain text when previewing an attachment.

I think it is more convenient for users to render those as HTML as if they are written in a Wiki page. Raw markups such as "h1.", "h2.", and "h3." are not friendly for humans.

29752.patch Magnifier (4.14 KB) Takenori TAKAKI, 2018-10-16 08:15

add-border-before@2x.png (68.8 KB) Go MAEDA, 2019-10-02 17:46

add-border-after@2x.png (69 KB) Go MAEDA, 2019-10-02 17:46


Related issues

Related to Redmine - Feature #13431: it would be nice to render the markup language in the "Re... New
Related to Redmine - Feature #16849: Add Markdown rendering in Reposotory -> View | Annotate New

Associated revisions

Revision 18584
Added by Go MAEDA 19 days ago

Render Textile and Markdown attachments on the preview page (#29752).

Patch by Takenori TAKAKI.

Revision 18585
Added by Go MAEDA 19 days ago

Add styles for Textile/Markdown attachments preview (#29752).

Patch by Go MAEDA.

Revision 18586
Added by Go MAEDA 18 days ago

Use Redmine::MimeType.of instead of regexp to check if the attachment is Textile (#29752).

History

#1 Updated by Go MAEDA about 1 year ago

  • Related to Feature #13431: it would be nice to render the markup language in the "Repository" view added

#2 Updated by Takenori TAKAKI about 1 year ago

+1
I think it is very useful that the feature proposed by Go MAEDA.
I made a patch to add the feature, and attach it.

#3 Updated by Go MAEDA 11 months ago

  • Target version set to 4.1.0

LGTM. Setting the target version to 4.1.0.

#4 Updated by Go MAEDA 6 months ago

  • Related to Feature #16849: Add Markdown rendering in Reposotory -> View | Annotate added

#5 Updated by Marius BALTEANU 23 days ago

  • Status changed from New to Needs feedback

The patch looks good to me with one small observation, do you see any problem if we add 'text/textile' => 'textile' to MIME_TYPES? In this way, we can replace in the patch self.filename =~ /\.(textile)$/i with Redmine::MimeType.of(filename) == "text/textile".

#6 Updated by Marius BALTEANU 23 days ago

  • Assignee set to Go MAEDA

#7 Updated by Go MAEDA 19 days ago

I suggest adding the following change to the patch in order to make distinguishable between the UI of Redmine and the content of the file by adding a border around the content.

diff --git a/app/views/common/_markup.html.erb b/app/views/common/_markup.html.erb
index dc6c7f474..14786a87c 100644
--- a/app/views/common/_markup.html.erb
+++ b/app/views/common/_markup.html.erb
@@ -1,3 +1,3 @@
-<div class="wiki">
+<div class="filecontent wiki">
   <%= Redmine::WikiFormatting.to_html(markup_text_formatting, Redmine::CodesetUtil.to_utf8_by_setting(markup_text)).html_safe %>
 </div>
diff --git a/public/stylesheets/application.css b/public/stylesheets/application.css
index 5f8ade106..8c9402142 100644
--- a/public/stylesheets/application.css
+++ b/public/stylesheets/application.css
@@ -1740,6 +1740,13 @@ img {
   max-width: 100%;
 }

+.filecontent-container > .filecontent.wiki {
+  position: relative;
+  border: 1px solid #e2e2e2;
+  padding: 8px;
+  border-radius: 3px;
+}
+
 /* Fixes for IE 11 */
 @media all and (-ms-high-contrast: none), (-ms-high-contrast: active) {
   select::-ms-expand {

Before:

After:

#8 Updated by Go MAEDA 19 days ago

  • Status changed from Needs feedback to Closed
  • Resolution set to Fixed

Committed the patch. Thank you all for contributing and reviewing the patch.

#9 Updated by Go MAEDA 19 days ago

  • Subject changed from Render Textile and Markdown attachments as HTML on the preview page to Render Textile and Markdown attachments on the preview page

#10 Updated by Mischa The Evil 19 days ago

Go MAEDA wrote:

Committed the patch. [...]

What about the note Marius made in #29752#note-5?

#11 Updated by Go MAEDA 18 days ago

  • Status changed from Closed to Reopened

Mischa The Evil wrote:

What about the note Marius made in #29752#note-5?

Oh, I forgot to handle that. Thank you for pointing it out.

Marius BALTEANU wrote:

The patch looks good to me with one small observation, do you see any problem if we add 'text/textile' => 'textile' to MIME_TYPES? In this way, we can replace in the patch self.filename =~ /\.(textile)$/i with Redmine::MimeType.of(filename) == "text/textile".

I like the approach but there is only one problem that "text/textile" is not a valid media type (see https://www.iana.org/assignments/media-types/media-types.xhtml). Maybe we can use "text/x-textile" instead, what do you think?

#12 Updated by Marius BALTEANU 18 days ago

Go MAEDA wrote:

Marius BALTEANU wrote:

The patch looks good to me with one small observation, do you see any problem if we add 'text/textile' => 'textile' to MIME_TYPES? In this way, we can replace in the patch self.filename =~ /\.(textile)$/i with Redmine::MimeType.of(filename) == "text/textile".

I like the approach but there is only one problem that "text/textile" is not a valid media type (see https://www.iana.org/assignments/media-types/media-types.xhtml). Maybe we can use "text/x-textile" instead, what do you think?

I like your idea to use "text/x-textile".

#13 Updated by Go MAEDA 18 days ago

This patch replaces the regexp that checks '.textile' extension with Redmine::MimeType.of.

Index: app/models/attachment.rb
===================================================================
--- app/models/attachment.rb    (リビジョン 18585)
+++ app/models/attachment.rb    (作業コピー)
@@ -245,7 +245,7 @@
   end

   def is_textile?
-    self.filename =~ /\.textile$/i
+    Redmine::MimeType.of(filename) == 'text/x-textile'
   end

   def is_image?
Index: lib/redmine/mime_type.rb
===================================================================
--- lib/redmine/mime_type.rb    (リビジョン 18585)
+++ lib/redmine/mime_type.rb    (作業コピー)
@@ -35,6 +35,7 @@
       'text/x-ruby' => 'rb,rbw,ruby,rake,erb',
       'text/x-csh' => 'csh',
       'text/x-sh' => 'sh',
+      'text/x-textile' => 'textile',
       'text/xml' => 'xml,xsd,mxml',
       'text/yaml' => 'yml,yaml',
       'text/csv' => 'csv',

#14 Updated by Go MAEDA 18 days ago

  • Status changed from Reopened to Closed

Committed the patch #29752#note-13 in r18586.

Also available in: Atom PDF