From 8b14522bf552c9e465a99967a3adaf69fd175192 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Javi=20Mart=C3=ADn?= <javim@elretirao.net>
Date: Tue, 2 May 2023 16:02:27 +0200
Subject: [PATCH] Use a button instead of a link to remove a marker

Using a button for interactive elements is better, as explained in
commit 5311daadf.

Since buttons with "type=button" do nothing by default, we no longer
need to call `preventDefault()` when clicking it.
---
 app/assets/javascripts/map.js                   |  3 +--
 app/assets/stylesheets/map_location.scss        |  4 ++--
 app/components/shared/map_location_component.rb | 13 +++++++------
 spec/shared/system/mappable.rb                  |  4 ++--
 4 files changed, 12 insertions(+), 12 deletions(-)

diff --git a/app/assets/javascripts/map.js b/app/assets/javascripts/map.js
index 2058a0791..80a8c6af6 100644
--- a/app/assets/javascripts/map.js
+++ b/app/assets/javascripts/map.js
@@ -43,8 +43,7 @@
         newMarker.addTo(map);
         return newMarker;
       };
-      removeMarker = function(e) {
-        e.preventDefault();
+      removeMarker = function() {
         if (marker) {
           map.removeLayer(marker);
           marker = null;
diff --git a/app/assets/stylesheets/map_location.scss b/app/assets/stylesheets/map_location.scss
index b9e45f4c9..00f322efe 100644
--- a/app/assets/stylesheets/map_location.scss
+++ b/app/assets/stylesheets/map_location.scss
@@ -1,7 +1,8 @@
 .map-location-remove-marker {
   border-bottom: 1px dotted #cf2a0e;
+  border-radius: 0;
   color: $delete;
-  display: inline-block;
+  cursor: pointer;
   margin-bottom: $line-height;
   margin-top: $line-height / 2;
 
@@ -10,7 +11,6 @@
   &:focus {
     border-bottom-style: solid;
     color: #cf2a0e;
-    text-decoration: none;
   }
 }
 
diff --git a/app/components/shared/map_location_component.rb b/app/components/shared/map_location_component.rb
index f26acb253..69371228b 100644
--- a/app/components/shared/map_location_component.rb
+++ b/app/components/shared/map_location_component.rb
@@ -33,14 +33,15 @@ class Shared::MapLocationComponent < ApplicationComponent
       t("proposals.form.map_remove_marker")
     end
 
-    def remove_marker_link_id
-      "remove-marker-link-#{dom_id(map_location)}"
+    def remove_marker_id
+      "remove-marker-#{dom_id(map_location)}"
     end
 
     def remove_marker
-      link_to remove_marker_label, "#",
-        id: remove_marker_link_id,
-        class: "map-location-remove-marker"
+      button_tag remove_marker_label,
+        id: remove_marker_id,
+        class: "map-location-remove-marker",
+        type: "button"
     end
 
     def data
@@ -52,7 +53,7 @@ class Shared::MapLocationComponent < ApplicationComponent
         map_tiles_provider: Rails.application.secrets.map_tiles_provider,
         map_tiles_provider_attribution: Rails.application.secrets.map_tiles_provider_attribution,
         marker_editable: editable?,
-        marker_remove_selector: "##{remove_marker_link_id}",
+        marker_remove_selector: "##{remove_marker_id}",
         marker_investments_coordinates: investments_coordinates,
         marker_latitude: map_location.latitude.presence,
         marker_longitude: map_location.longitude.presence
diff --git a/spec/shared/system/mappable.rb b/spec/shared/system/mappable.rb
index 4b49e7d07..cf12103db 100644
--- a/spec/shared/system/mappable.rb
+++ b/spec/shared/system/mappable.rb
@@ -215,7 +215,7 @@ shared_examples "mappable" do |mappable_factory_name, mappable_association_name,
       do_login_for mappable.author, management: management
 
       visit send(mappable_edit_path, id: mappable.id)
-      click_link "Remove map marker"
+      click_button "Remove map marker"
       click_on "Save changes"
 
       expect(page).not_to have_css(".map-location")
@@ -236,7 +236,7 @@ shared_examples "mappable" do |mappable_factory_name, mappable_association_name,
       do_login_for mappable.author, management: management
 
       visit send(mappable_edit_path, id: mappable.id)
-      click_link "Remove map marker"
+      click_button "Remove map marker"
       click_on "Save changes"
 
       expect(page).not_to have_content "Map location can't be blank"