[openstreetmap/openstreetmap-website] Define fixed size for trace images (PR #5030)

Anton Khorev notifications at github.com
Tue Jul 30 00:59:10 UTC 2024


@AntonKhorev commented on this pull request.



> +  def trace_image(trace, options = {})
+    options[:class] ||= "trace_image"
+    options[:alt] ||= ""
+
+    link_to(image_tag(trace_icon_path(trace.user, trace),
+                      options.merge(:width => 50, :height => 50)),
+            show_trace_path(trace.user, trace),
+            :class => "d-inline-block")
+  end

Do we really need a helper for this? If it's supposed to be reusable, why `d-inline-block` is baked into the link?

Why the name `trace_image`? There are two trace images, the name doesn't tell which one this is. Why "image" if it returns a link?

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

Message ID: <openstreetmap/openstreetmap-website/pull/5030/review/2206378983 at github.com>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.openstreetmap.org/pipermail/rails-dev/attachments/20240729/d3cdfdcc/attachment.htm>


More information about the rails-dev mailing list