From 3fa3c90db6cca50e8f4cf944921e75c31af4983c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sen=C3=A9n=20Rodero=20Rodr=C3=ADguez?= <15726+Senen@users.noreply.github.com> Date: Fri, 16 Jun 2023 15:48:57 +0200 Subject: [PATCH] Fix map markers navigation for keyboard users By using the bindPopup function instead of the click event popups work when using the keyboard. Note that now we are loading all the map markers in the first request in a single query to the database (needed when there is a lot or markers to show). Because of that we removed the AJAX endpoint. --- app/assets/javascripts/map.js | 15 ++---------- .../budgets/investments/map_component.rb | 2 +- app/components/budgets/map_component.rb | 2 +- .../budgets/investments_controller.rb | 24 ++++--------------- app/models/abilities/everyone.rb | 2 +- app/models/map_location.rb | 19 +++++++++++++++ config/routes/budget.rb | 3 --- 7 files changed, 28 insertions(+), 39 deletions(-) diff --git a/app/assets/javascripts/map.js b/app/assets/javascripts/map.js index 4b2e4a5a2..396ac5418 100644 --- a/app/assets/javascripts/map.js +++ b/app/assets/javascripts/map.js @@ -172,7 +172,7 @@ if (App.Map.validCoordinates(coordinates)) { marker = createMarker(coordinates.lat, coordinates.long); marker.options.id = coordinates.investment_id; - marker.on("click", App.Map.openMarkerPopup); + marker.bindPopup(App.Map.getPopupContent(coordinates)); } }); } @@ -217,19 +217,8 @@ polygon.addTo(map); }, - openMarkerPopup: function(e) { - var marker = e.target; - $.ajax("/investments/" + marker.options.id + "/json_data", { - type: "GET", - dataType: "json", - success: function(data) { - e.target.bindPopup(App.Map.getPopupContent(data)).openPopup(); - } - }); - }, getPopupContent: function(data) { - return "" + - data.investment_title + ""; + return "" + data.title + ""; }, validZoom: function(zoom) { return App.Map.isNumeric(zoom); diff --git a/app/components/budgets/investments/map_component.rb b/app/components/budgets/investments/map_component.rb index 897524ca2..43253dfed 100644 --- a/app/components/budgets/investments/map_component.rb +++ b/app/components/budgets/investments/map_component.rb @@ -18,7 +18,7 @@ class Budgets::Investments::MapComponent < ApplicationComponent end def coordinates - MapLocation.where(investment: investments).map(&:json_data) + MapLocation.investments_json_data(investments.unscope(:order)) end def geozones_data diff --git a/app/components/budgets/map_component.rb b/app/components/budgets/map_component.rb index dcbc427d6..ccf3719d1 100644 --- a/app/components/budgets/map_component.rb +++ b/app/components/budgets/map_component.rb @@ -19,7 +19,7 @@ class Budgets::MapComponent < ApplicationComponent investments = budget.investments end - MapLocation.where(investment_id: investments).map(&:json_data) + MapLocation.investments_json_data(investments) end def geozones_data diff --git a/app/controllers/budgets/investments_controller.rb b/app/controllers/budgets/investments_controller.rb index 84277042f..c192827c4 100644 --- a/app/controllers/budgets/investments_controller.rb +++ b/app/controllers/budgets/investments_controller.rb @@ -11,12 +11,11 @@ module Budgets PER_PAGE = 10 - before_action :authenticate_user!, except: [:index, :show, :json_data] - before_action :load_budget, except: :json_data + before_action :authenticate_user!, except: [:index, :show] + before_action :load_budget - authorize_resource :budget, except: :json_data - load_and_authorize_resource :investment, through: :budget, class: "Budget::Investment", - except: :json_data + authorize_resource :budget + load_and_authorize_resource :investment, through: :budget, class: "Budget::Investment" before_action :load_ballot, only: [:index, :show] before_action :load_heading, only: [:index, :show] @@ -25,8 +24,6 @@ module Budgets before_action :set_default_investment_filter, only: :index before_action :set_view, only: :index - skip_authorization_check only: :json_data - feature_flag :budgets has_orders %w[most_voted newest oldest], only: :show @@ -91,19 +88,6 @@ module Budgets super end - def json_data - investment = Budget::Investment.find(params[:id]) - data = { - investment_id: investment.id, - investment_title: investment.title, - budget_id: investment.budget.id - }.to_json - - respond_to do |format| - format.json { render json: data } - end - end - private def resource_model diff --git a/app/models/abilities/everyone.rb b/app/models/abilities/everyone.rb index 7052c48d5..252ce3896 100644 --- a/app/models/abilities/everyone.rb +++ b/app/models/abilities/everyone.rb @@ -14,7 +14,7 @@ module Abilities can [:read, :welcome], Budget can [:read], Budget can [:read], Budget::Group - can [:read, :print, :json_data], Budget::Investment + can [:read, :print], Budget::Investment can :read_results, Budget, id: Budget.finished.results_enabled.ids can :read_stats, Budget, id: Budget.valuating_or_later.stats_enabled.ids can :read_executions, Budget, phase: "finished" diff --git a/app/models/map_location.rb b/app/models/map_location.rb index 6ed4fc9cc..7adc6d639 100644 --- a/app/models/map_location.rb +++ b/app/models/map_location.rb @@ -24,4 +24,23 @@ class MapLocation < ApplicationRecord longitude: (heading.longitude.to_f if heading.longitude.present?) ) end + + def self.investments_json_data(investments) + return [] unless investments.any? + + budget_id = investments.first.budget_id + + data = investments.joins(:map_location) + .with_fallback_translation + .pluck(:id, :title, :latitude, :longitude) + + data.map do |values| + { + title: values[1], + link: "/budgets/#{budget_id}/investments/#{values[0]}", + lat: values[2], + long: values[3] + } + end + end end diff --git a/config/routes/budget.rb b/config/routes/budget.rb index 6d173af87..f6054d95f 100644 --- a/config/routes/budget.rb +++ b/config/routes/budget.rb @@ -23,6 +23,3 @@ end resolve "Budget::Investment" do |investment, options| [investment.budget, :investment, options.merge(id: investment)] end - -get "investments/:id/json_data", action: :json_data, controller: "budgets/investments" -get "/budgets/:budget_id/investments/:id/json_data", action: :json_data, controller: "budgets/investments"