Merge pull request #5111 from consul/remove_marker_accessibility

Use a button instead of a link to remove a marker
This commit is contained in:
Javi Martín
2023-05-05 15:23:36 +02:00
committed by GitHub
7 changed files with 52 additions and 53 deletions

View File

@@ -43,8 +43,7 @@
newMarker.addTo(map);
return newMarker;
};
removeMarker = function(e) {
e.preventDefault();
removeMarker = function() {
if (marker) {
map.removeLayer(marker);
marker = null;

View File

@@ -1,7 +1,9 @@
.location-map-remove-marker {
.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;
&:hover,
@@ -9,7 +11,6 @@
&:focus {
border-bottom-style: solid;
color: #cf2a0e;
text-decoration: none;
}
}

View File

@@ -1,4 +1,4 @@
<%= tag.div(id: dom_id(map_location), class: "map_location map", data: data) %>
<%= tag.div(id: dom_id(map_location), class: "map-location map", data: data) %>
<% if editable? %>
<%= remove_marker %>

View File

@@ -33,16 +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
tag.div class: "margin-bottom" do
link_to remove_marker_label, "#",
id: remove_marker_link_id,
class: "js-location-map-remove-marker location-map-remove-marker"
end
button_tag remove_marker_label,
id: remove_marker_id,
class: "map-location-remove-marker",
type: "button"
end
def data
@@ -54,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

View File

@@ -18,7 +18,7 @@ shared_examples "mappable" do |mappable_factory_name, mappable_association_name,
send("fill_in_#{mappable_factory_name}_form")
within ".map_location" do
within ".map-location" do
expect(page).not_to have_css(".map-icon")
end
end
@@ -30,7 +30,7 @@ shared_examples "mappable" do |mappable_factory_name, mappable_association_name,
send("fill_in_#{mappable_factory_name}_form")
find("#new_map_location").click
within ".map_location" do
within ".map-location" do
expect(page).to have_css(".map-icon")
end
end
@@ -43,7 +43,7 @@ shared_examples "mappable" do |mappable_factory_name, mappable_association_name,
find("#new_map_location").click
send("submit_#{mappable_factory_name}_form")
within ".map_location" do
within ".map-location" do
expect(page).to have_css(".map-icon")
end
end
@@ -53,10 +53,10 @@ shared_examples "mappable" do |mappable_factory_name, mappable_association_name,
visit send(mappable_new_path, arguments)
send("fill_in_#{mappable_factory_name}_form")
expect(page).to have_css ".map_location"
expect(page).to have_css ".map-location"
send("submit_#{mappable_factory_name}_form")
expect(page).not_to have_css(".map_location")
expect(page).not_to have_css(".map-location")
end
scenario "Can not display map on #{mappable_factory_name} when feature.map is disabled" do
@@ -65,10 +65,10 @@ shared_examples "mappable" do |mappable_factory_name, mappable_association_name,
visit send(mappable_new_path, arguments)
send("fill_in_#{mappable_factory_name}_form")
expect(page).not_to have_css ".map_location"
expect(page).not_to have_css ".map-location"
send("submit_#{mappable_factory_name}_form")
expect(page).not_to have_css(".map_location")
expect(page).not_to have_css(".map-location")
end
describe "When restoring the page from browser history" do
@@ -90,7 +90,7 @@ shared_examples "mappable" do |mappable_factory_name, mappable_association_name,
go_back
within ".map_location" do
within ".map-location" do
expect(page).to have_css(".leaflet-map-pane", count: 1)
end
end
@@ -99,7 +99,7 @@ shared_examples "mappable" do |mappable_factory_name, mappable_association_name,
do_login_for user, management: management
visit send(mappable_new_path, arguments)
within ".map_location" do
within ".map-location" do
expect(page).not_to have_css(".map-icon")
end
expect(page.execute_script("return App.Map.maps[0].getZoom();")).to eq(10)
@@ -107,7 +107,7 @@ shared_examples "mappable" do |mappable_factory_name, mappable_association_name,
map_zoom_in
find("#new_map_location").click
within ".map_location" do
within ".map-location" do
expect(page).to have_css(".map-icon")
end
@@ -123,7 +123,7 @@ shared_examples "mappable" do |mappable_factory_name, mappable_association_name,
go_back
within ".map_location" do
within ".map-location" do
expect(page).to have_css(".map-icon")
expect(page.execute_script("return App.Map.maps[0].getZoom();")).to eq(11)
end
@@ -133,14 +133,14 @@ shared_examples "mappable" do |mappable_factory_name, mappable_association_name,
do_login_for user, management: management
visit send(mappable_new_path, arguments)
within ".map_location" do
within ".map-location" do
expect(page).not_to have_css(".map-icon")
end
place_map_at(-68.592487, -62.391357)
find("#new_map_location").click
within ".map_location" do
within ".map-location" do
expect(page).to have_css(".map-icon")
end
@@ -156,7 +156,7 @@ shared_examples "mappable" do |mappable_factory_name, mappable_association_name,
go_back
within ".map_location" do
within ".map-location" do
expect(page).to have_css(".map-icon")
end
end
@@ -189,13 +189,13 @@ 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)
find(".map_location").click
find(".map-location").click
click_on("Save changes")
mappable.reload
expect(page).to have_css(".map_location")
expect(page).not_to have_selector(".map_location[data-marker-latitude='#{map_location.latitude}']")
expect(page).to have_selector(".map_location[data-marker-latitude='#{mappable.map_location.latitude}']")
expect(page).to have_css(".map-location")
expect(page).not_to have_selector(".map-location[data-marker-latitude='#{map_location.latitude}']")
expect(page).to have_selector(".map-location[data-marker-latitude='#{mappable.map_location.latitude}']")
end
scenario "Should edit mappable on #{mappable_factory_name} without change map" do
@@ -206,19 +206,19 @@ shared_examples "mappable" do |mappable_factory_name, mappable_association_name,
click_on("Save changes")
mappable.reload
expect(page).to have_css(".map_location")
expect(page).to have_selector(".map_location[data-marker-latitude='#{map_location.latitude}']")
expect(page).to have_selector(".map_location[data-marker-latitude='#{mappable.map_location.latitude}']")
expect(page).to have_css(".map-location")
expect(page).to have_selector(".map-location[data-marker-latitude='#{map_location.latitude}']")
expect(page).to have_selector(".map-location[data-marker-latitude='#{mappable.map_location.latitude}']")
end
scenario "Can not display map on #{mappable_factory_name} edit when remove map marker" do
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")
expect(page).not_to have_css(".map-location")
end
scenario "Can not display map on #{mappable_factory_name} edit when feature.map is disabled" do
@@ -229,14 +229,14 @@ shared_examples "mappable" do |mappable_factory_name, mappable_association_name,
fill_in "#{mappable_factory_name.camelize} title", with: "New title"
click_on("Save changes")
expect(page).not_to have_css(".map_location")
expect(page).not_to have_css(".map-location")
end
scenario "No need to skip map on update" do
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"
@@ -254,7 +254,7 @@ shared_examples "mappable" do |mappable_factory_name, mappable_association_name,
do_login_for user, management: management if management
visit send(mappable_show_path, arguments)
within ".map_location" do
within ".map-location" do
expect(page).to have_css(".map-icon")
end
end
@@ -267,7 +267,7 @@ shared_examples "mappable" do |mappable_factory_name, mappable_association_name,
do_login_for user, management: management if management
visit send(mappable_show_path, arguments)
expect(page).not_to have_css(".map_location")
expect(page).not_to have_css(".map-location")
end
scenario "Should not display map on #{mappable_factory_name} show page when feature.map is disable" do
@@ -277,7 +277,7 @@ shared_examples "mappable" do |mappable_factory_name, mappable_association_name,
do_login_for user, management: management if management
visit send(mappable_show_path, arguments)
expect(page).not_to have_css(".map_location")
expect(page).not_to have_css(".map-location")
end
end
end

View File

@@ -270,7 +270,7 @@ describe "Budgets" do
visit budgets_path
within ".map_location" do
within ".map-location" do
expect(page).to have_css(".map-icon", count: 3, visible: :all)
end
end
@@ -290,7 +290,7 @@ describe "Budgets" do
visit budgets_path
within ".map_location" do
within ".map-location" do
expect(page).to have_css(".map-icon", count: 4, visible: :all)
end
end
@@ -310,7 +310,7 @@ describe "Budgets" do
visit budgets_path
within ".map_location" do
within ".map-location" do
expect(page).to have_css(".map-icon", count: 2, visible: :all)
end
end
@@ -338,7 +338,7 @@ describe "Budgets" do
visit budgets_path
within ".map_location" do
within ".map-location" do
expect(page).to have_css(".map-icon", count: 1, visible: :all)
end
end

View File

@@ -122,7 +122,7 @@ describe "Budget Investments" do
create(:budget_investment, heading: heading)
visit budget_investments_path(budget, heading_id: heading.id)
within("#sidebar") do
expect(page).to have_css(".map_location")
expect(page).to have_css(".map-location")
end
unlocated_heading = create(:budget_heading, name: "No Map", price: 500, group: group,
@@ -130,7 +130,7 @@ describe "Budget Investments" do
create(:budget_investment, heading: unlocated_heading)
visit budget_investments_path(budget, heading_id: unlocated_heading.id)
within("#sidebar") do
expect(page).not_to have_css(".map_location")
expect(page).not_to have_css(".map-location")
end
end
@@ -1637,7 +1637,7 @@ describe "Budget Investments" do
visit budget_investments_path(budget, heading_id: heading.id)
within ".map_location" do
within ".map-location" do
expect(page).to have_css(".map-icon", count: 6, visible: :all)
end
end
@@ -1651,7 +1651,7 @@ describe "Budget Investments" do
visit budget_investments_path(budget, heading_id: heading.id)
within ".map_location" do
within ".map-location" do
expect(page).to have_css(".map-icon", count: 2, visible: :all)
end
end
@@ -1675,7 +1675,7 @@ describe "Budget Investments" do
visit budget_investments_path(budget, heading_id: heading.id)
within ".map_location" do
within ".map-location" do
expect(page).to have_css(".map-icon", count: 4, visible: :all)
end
end
@@ -1693,7 +1693,7 @@ describe "Budget Investments" do
visit budget_investments_path(budget, heading_id: heading.id)
within ".map_location" do
within ".map-location" do
expect(page).to have_css(".map-icon", count: 0, visible: :all)
end
end
@@ -1711,7 +1711,7 @@ describe "Budget Investments" do
expect(page).to have_css(".budget-investment", count: 2)
end
within(".map_location") do
within(".map-location") do
expect(page).to have_css(".map-icon", count: 3, visible: :all)
end
end