Add RTL support to markdown renderer

CHANGELOG

@@ -55,6 +55,7 @@ v 8.13.0 (unreleased)
   - Added soft wrap button to repository file/blob editor
   - Update namespace validation to forbid reserved names (.git and .atom) (Will Starms)
   - Show the time ago a merge request was deployed to an environment
+  - Add RTL support to markdown renderer (Ebrahim Byagowi)
   - Add word-wrap to issue title on issue and milestone boards (ClemMakesApps)
   - Fix todos page mobile viewport layout (ClemMakesApps)
   - Fix inconsistent highlighting of already selected activity nav-links (ClemMakesApps)

app/assets/stylesheets/framework/typography.scss

@@ -90,6 +90,11 @@
     border-left: 3px solid #e7e9ed;
   }
 
+  blockquote:dir(rtl) {
+    border-left: 0;
+    border-right: 3px solid #e7e9ed;
+  }
+
   blockquote p {
     color: #7f8fa4 !important;
     font-size: inherit;
@@ -112,6 +117,10 @@
     }
   }
 
+  table:dir(rtl) th {
+    text-align: right;
+  }
+
   pre {
     margin: 12px 0;
     font-size: 13px;
@@ -129,6 +138,10 @@
     margin: 3px 0 3px 28px !important;
   }
 
+  ul:dir(rtl), ol:dir(rtl) {
+    margin: 3px 28px 3px 0 !important;
+  }
+
   li {
     line-height: 1.6em;
   }

lib/banzai/filter/set_direction_filter.rb

+module Banzai
+  module Filter
+    # HTML filter that sets dir="auto" for RTL languages support
+    class SetDirectionFilter < HTML::Pipeline::Filter
+      def call
+        # select these elements just on top level of the document
+        doc.xpath('p|h1|h2|h3|h4|h5|h6|ol|ul[not(@class="section-nav")]|blockquote|table').each do |el|
+          el['dir'] = 'auto'
+        end
+
+        doc
+      end
+    end
+  end
+end

lib/banzai/pipeline/gfm_pipeline.rb

@@ -25,7 +25,9 @@ module Banzai
           Filter::MilestoneReferenceFilter,
 
           Filter::TaskListFilter,
-          Filter::InlineDiffFilter
+          Filter::InlineDiffFilter,
+
+          Filter::SetDirectionFilter
         ]
       end
 

spec/features/atom/users_spec.rb

@@ -53,7 +53,7 @@ describe "User Feed", feature: true  do
       end
 
       it 'has XHTML summaries in issue descriptions' do
-        expect(body).to match /we have a bug!<\/p>\n\n<hr ?\/>\n\n<p>I guess/
+        expect(body).to match /we have a bug!<\/p>\n\n<hr ?\/>\n\n<p dir="auto">I guess/
       end
 
       it 'has XHTML summaries in notes' do

spec/lib/banzai/object_renderer_spec.rb

@@ -24,7 +24,7 @@ describe Banzai::ObjectRenderer do
         with(an_instance_of(Array)).
         and_call_original
 
-      expect(object).to receive(:redacted_note_html=).with('<p>hello</p>')
+      expect(object).to receive(:redacted_note_html=).with('<p dir="auto">hello</p>')
       expect(object).to receive(:user_visible_reference_count=).with(0)
 
       renderer.render([object], :note)
@@ -92,10 +92,10 @@ describe Banzai::ObjectRenderer do
       docs = renderer.render_attributes(objects, :note)
 
       expect(docs[0]).to be_an_instance_of(Nokogiri::HTML::DocumentFragment)
-      expect(docs[0].to_html).to eq('<p>hello</p>')
+      expect(docs[0].to_html).to eq('<p dir="auto">hello</p>')
 
       expect(docs[1]).to be_an_instance_of(Nokogiri::HTML::DocumentFragment)
-      expect(docs[1].to_html).to eq('<p>bye</p>')
+      expect(docs[1].to_html).to eq('<p dir="auto">bye</p>')
     end
 
     it 'returns when no objects to render' do

spec/lib/banzai/pipeline/description_pipeline_spec.rb

@@ -4,11 +4,11 @@ describe Banzai::Pipeline::DescriptionPipeline do
   def parse(html)
     # When we pass HTML to Redcarpet, it gets wrapped in `p` tags...
     # ...except when we pass it pre-wrapped text. Rabble rabble.
-    unwrap = !html.start_with?('<p>')
+    unwrap = !html.start_with?('<p ')
 
     output = described_class.to_html(html, project: spy)
 
-    output.gsub!(%r{\A<p>(.*)</p>(.*)\z}, '\1\2') if unwrap
+    output.gsub!(%r{\A<p dir="auto">(.*)</p>(.*)\z}, '\1\2') if unwrap
 
     output
   end
@@ -27,11 +27,17 @@ describe Banzai::Pipeline::DescriptionPipeline do
     end
   end
 
-  %w(b i strong em a ins del sup sub p).each do |elem|
+  %w(b i strong em a ins del sup sub).each do |elem|
     it "still allows '#{elem}' elements" do
       exp = act = "<#{elem}>Description</#{elem}>"
 
       expect(parse(act).strip).to eq exp
     end
   end
+
+  it "still allows 'p' elements" do
+    exp = act = "<p dir=\"auto\">Description</p>"
+
+    expect(parse(act).strip).to eq exp
+  end
 end

spec/models/concerns/cache_markdown_field_spec.rb

@@ -64,7 +64,7 @@ describe CacheMarkdownField do
   let(:html) { "<p><code>Foo</code></p>" }
 
   let(:updated_markdown) { "`Bar`" }
-  let(:updated_html) { "<p><code>Bar</code></p>" }
+  let(:updated_html) { "<p dir=\"auto\"><code>Bar</code></p>" }
 
   subject { ThingWithMarkdownFields.new(foo: markdown, foo_html: html) }