@AntonKhorev commented on this pull request.


> +
+        "math" # don't know how this ends up, but dir="auto" is probably 
correct
+      ].each do |name|
+        define_method :"convert_#{name}" do |el, indent|
+          attr_bak = el.attr.dup # can't avoid mutating the attr hash, so make 
a backup
+          el.attr["dir"] = "auto" unless el.attr.key?("dir") # if by some 
miracle dir is already defined, don't override it
+          ret = super(el, indent)
+          el.attr.replace(attr_bak) # restore backup
+          ret
+        end
+      end
+
+      # only add dir="auto" to bare links
+      def convert_a(el, indent)
+        attr_bak = el.attr.dup
+        el.attr["dir"] = "auto" if !el.attr.key?("dir") && el.attr["href"] == 
inner(el, indent)

And this check `el.attr["href"] == inner(el, indent)`, is it required? The 
comment says "only add dir="auto" to bare links", which ones are bare? We add 
some links with autolinker later.

-- 
Reply to this email directly or view it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/pull/5840#pullrequestreview-2721429836
You are receiving this because you are subscribed to this thread.

Message ID: 
<openstreetmap/openstreetmap-website/pull/5840/review/2721429...@github.com>
_______________________________________________
rails-dev mailing list
rails-dev@openstreetmap.org
https://lists.openstreetmap.org/listinfo/rails-dev

Reply via email to