[openstreetmap/openstreetmap-website] Added link to nominatim results in searching results (PR #4895)

[Tom Hughes]

@tomhughes requested changes on this pull request.



> @@ -15,8 +15,32 @@ def search
     if @params[:lat] && @params[:lon]
       @sources.push "latlon"
       @sources.push "osm_nominatim_reverse"
+
+      # get query parameters
+      lat = params[:lat]
+      lon = params[:lon]
+      zoom = params[:zoom]
+
+      # create URL for nominatim reverse search
+      @cached_osm_nominatim_parameters = "reverse?format=html&lat=#{lat}&lon=#{lon}&zoom=#{zoom}&accept-language=#{http_accept_language.user_preferred_languages.join(',')}"

This, and the equivalent code for forward searches, is just duplicating the work that the search methods do.

It would be better to move that code into private `nominatim_reverse_url` and `nominatim_url` methods that can then be used in both places.

> @@ -3,10 +3,17 @@
 <%= render "sidebar_header", :title => t("site.sidebar.search_results") %>
 
 <% @sources.each do |source| %>
-  <h4>
-    <%= t(".title.results_from_html", :results_link => link_to(t(".title.#{source}"),
-                                                               t(".title.#{source}_url"))) %>
-  </h4>
+  <% if source.include?("osm_nominatim") %>

Rather than having this look at the name of the source it would be better to make `@sources` an array of hashes, each with a `:name` and an optional `:url` key.

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

Message ID: <openstreetmap/openstreetmap-website/pull/4895/review/2118759066 at github.com>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.openstreetmap.org/pipermail/rails-dev/attachments/20240614/d69e1b82/attachment.htm>