From b3b286deb56d8d3add5cbe1308845fc6c2f28cb3 Mon Sep 17 00:00:00 2001
From: Marwin Hochfelsner <50826859+hlfan@users.noreply.github.com>
Date: Sun, 20 Jul 2025 14:41:17 +0200
Subject: [PATCH] Use more Rails' form tag helpers in directions form

---
 app/assets/javascripts/index/directions.js |  8 ++---
 app/views/layouts/_search.html.erb         | 40 ++++++++++------------
 test/system/directions_test.rb             |  2 +-
 3 files changed, 23 insertions(+), 27 deletions(-)

diff --git a/app/assets/javascripts/index/directions.js b/app/assets/javascripts/index/directions.js
index 349462e1b..d7658af16 100644
--- a/app/assets/javascripts/index/directions.js
+++ b/app/assets/javascripts/index/directions.js
@@ -32,7 +32,7 @@ OSM.Directions = function (map) {
   expiry.setYear(expiry.getFullYear() + 10);
 
   const modeGroup = $(".routing_modes");
-  const select = $("select.routing_engines");
+  const select = $("select#routing_engines");
 
   $(".directions_form .reverse_directions").on("click", function () {
     const coordFrom = endpoints[0].latlng,
@@ -70,10 +70,10 @@ OSM.Directions = function (map) {
     modeGroup
       .find("input[id]")
       .prop("disabled", function () {
-        return !modes.includes(this.id);
+        return !modes.includes(this.value);
       })
       .prop("checked", function () {
-        return this.id === chosenEngine.mode;
+        return this.value === chosenEngine.mode;
       });
 
     const providers = engines
@@ -140,7 +140,7 @@ OSM.Directions = function (map) {
   setEngine(Cookies.get("_osm_directions_engine"));
 
   modeGroup.on("change", "input[name='modes']", function (e) {
-    setEngine(chosenEngine.provider + "_" + e.target.id);
+    setEngine(chosenEngine.provider + "_" + e.target.value);
     Cookies.set("_osm_directions_engine", chosenEngine.id, { secure: true, expires: expiry, path: "/", samesite: "lax" });
     getRoute(true, true);
   });
diff --git a/app/views/layouts/_search.html.erb b/app/views/layouts/_search.html.erb
index 9c32e1701..bef615e69 100644
--- a/app/views/layouts/_search.html.erb
+++ b/app/views/layouts/_search.html.erb
@@ -26,26 +26,21 @@
     <div class="d-flex flex-column gap-2">
       <div class="d-flex gap-2 align-items-center">
         <div class="btn-group routing_modes" role="group">
-          <input type="radio" class="btn-check" name="modes" id="car" autocomplete="off" disabled>
-          <label class="btn btn-outline-secondary px-2" for="car" title="<%= t("site.search.modes.car") %>">
-            <%= inline_svg_tag "search/car.svg", :class => "d-block" %>
-          </label>
-          <input type="radio" class="btn-check" name="modes" id="bicycle" autocomplete="off" disabled>
-          <label class="btn btn-outline-secondary px-2" for="bicycle" title="<%= t("site.search.modes.bicycle") %>">
-            <%= inline_svg_tag "search/bicycle.svg", :class => "d-block" %>
-          </label>
-          <input type="radio" class="btn-check" name="modes" id="foot" autocomplete="off" disabled>
-          <label class="btn btn-outline-secondary px-2" for="foot" title="<%= t("site.search.modes.foot") %>">
-            <%= inline_svg_tag "search/foot.svg", :class => "d-block" %>
-          </label>
+          <% %w[car bicycle foot].each do |mode| %>
+            <%= radio_button_tag "modes", mode, false, { :class => "btn-check", :autocomplete => "off", :disabled => true } %>
+            <%= label_tag "modes_#{mode}",
+                          inline_svg_tag("search/#{mode}.svg", :class => "d-block"),
+                          :class => "btn btn-outline-secondary px-2",
+                          :title => t("site.search.modes.#{mode}") %>
+          <% end %>
         </div>
-        <select class="routing_engines form-select py-1 px-2" name="routing_engines" title="<%= t("site.search.providers.description") %>">
-          <optgroup label="<%= t("site.search.providers.description") %>">
-            <option value="graphhopper" disabled><%= t("site.search.providers.graphhopper") %></option>
-            <option value="fossgis_osrm" disabled><%= t("site.search.providers.fossgis_osrm") %></option>
-            <option value="fossgis_valhalla" disabled><%= t("site.search.providers.fossgis_valhalla") %></option>
-          </optgroup>
-        </select>
+        <%= options = %w[graphhopper fossgis_osrm fossgis_valhalla].map do |engine|
+              [t("site.search.providers.#{engine}"), engine, { :disabled => true }]
+            end
+            select_tag "routing_engines",
+                       grouped_options_for_select({ t("site.search.providers.description") => options }),
+                       :class => "form-select py-1 px-2",
+                       :title => t("site.search.providers.description") %>
         <button type="button" class="btn-close flex-shrink-0 p-2 rounded-5" aria-label="<%= t("javascripts.close") %>"></button>
       </div>
       <div class="d-flex gap-2 align-items-center">
@@ -63,9 +58,10 @@
             <%= text_field_tag "route_to", params[:to], :placeholder => t("site.search.to"), :autocomplete => "on", :class => "form-control py-1 px-2 ps-4", :dir => "auto" %>
           </div>
         </div>
-        <button type="button" class="reverse_directions btn btn-outline-secondary border-0 p-1" title="<%= t("site.search.reverse_directions_text") %>">
-          <%= inline_svg_tag "search/reverse_directions.svg", :class => "d-block" %>
-        </button>
+        <%= button_tag inline_svg_tag("search/reverse_directions.svg", :class => "d-block"),
+                       :type => "button",
+                       :class => "reverse_directions btn btn-outline-secondary border-0 p-1",
+                       :title => t("site.search.reverse_directions_text") %>
       </div>
     </div>
   </form>
diff --git a/test/system/directions_test.rb b/test/system/directions_test.rb
index 4122be249..8d77bd24c 100644
--- a/test/system/directions_test.rb
+++ b/test/system/directions_test.rb
@@ -12,7 +12,7 @@ class DirectionsSystemTest < ApplicationSystemTestCase
       assert_content "Start popup text (car)"
     end
 
-    choose "bicycle", :allow_label_click => true
+    choose :option => "bicycle", :allow_label_click => true
 
     within "#sidebar" do
       assert_content "Start popup text (bicycle)"
-- 
2.39.5