From b0b7d0f25b54a2df07e56b668bdf7becf70ad303 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Mon, 6 Mar 2023 17:15:15 +0100 Subject: [PATCH 01/18] Remove unused legislation proposals map action The only view that linked to this action was never used and so it was deleted in commit 0bacd5baf. Since now the proposals controller is the only one place rendering the `shared/map` partial, we're moving it to the proposals views. --- .../concerns/commentable_actions.rb | 5 -- .../legislation/proposals_controller.rb | 6 +-- app/controllers/proposals_controller.rb | 5 ++ app/models/abilities/everyone.rb | 2 +- app/views/legislation/proposals/map.html.erb | 1 - app/views/proposals/map.html.erb | 51 ++++++++++++++++++- app/views/shared/_map.html.erb | 50 ------------------ config/routes/legislation.rb | 1 - 8 files changed, 59 insertions(+), 62 deletions(-) delete mode 100644 app/views/legislation/proposals/map.html.erb delete mode 100644 app/views/shared/_map.html.erb diff --git a/app/controllers/concerns/commentable_actions.rb b/app/controllers/concerns/commentable_actions.rb index dd8611366..1acb07778 100644 --- a/app/controllers/concerns/commentable_actions.rb +++ b/app/controllers/concerns/commentable_actions.rb @@ -68,11 +68,6 @@ module CommentableActions end end - def map - @resource = resource_model.new - @tag_cloud = tag_cloud - end - private def track_event diff --git a/app/controllers/legislation/proposals_controller.rb b/app/controllers/legislation/proposals_controller.rb index ce7e68e95..4d68ac201 100644 --- a/app/controllers/legislation/proposals_controller.rb +++ b/app/controllers/legislation/proposals_controller.rb @@ -3,10 +3,10 @@ class Legislation::ProposalsController < Legislation::BaseController include FlagActions include ImageAttributes - before_action :load_categories, only: [:new, :create, :edit, :map, :summary] - before_action :load_geozones, only: [:edit, :map, :summary] + before_action :load_categories, only: [:new, :create, :edit, :summary] + before_action :load_geozones, only: [:edit, :summary] - before_action :authenticate_user!, except: [:show, :map, :summary] + before_action :authenticate_user!, except: [:show, :summary] load_and_authorize_resource :process, class: "Legislation::Process" load_and_authorize_resource :proposal, class: "Legislation::Proposal", through: :process diff --git a/app/controllers/proposals_controller.rb b/app/controllers/proposals_controller.rb index 0f6ff5f60..23a186151 100644 --- a/app/controllers/proposals_controller.rb +++ b/app/controllers/proposals_controller.rb @@ -77,6 +77,11 @@ class ProposalsController < ApplicationController @tag_cloud = tag_cloud end + def map + @proposal = Proposal.new + @tag_cloud = tag_cloud + end + def disable_recommendations if current_user.update(recommended_proposals: false) redirect_to proposals_path, notice: t("proposals.index.recommendations.actions.success") diff --git a/app/models/abilities/everyone.rb b/app/models/abilities/everyone.rb index 216b101f1..7052c48d5 100644 --- a/app/models/abilities/everyone.rb +++ b/app/models/abilities/everyone.rb @@ -25,7 +25,7 @@ module Abilities id: Legislation::Process.past.published.where(result_publication_enabled: true).ids can [:read, :changes, :go_to_version], Legislation::DraftVersion can [:read], Legislation::Question - can [:read, :map, :share], Legislation::Proposal + can [:read, :share], Legislation::Proposal can [:search, :comments, :read, :create, :new_comment], Legislation::Annotation can [:read, :help], ::SDG::Goal diff --git a/app/views/legislation/proposals/map.html.erb b/app/views/legislation/proposals/map.html.erb deleted file mode 100644 index 10dbf795f..000000000 --- a/app/views/legislation/proposals/map.html.erb +++ /dev/null @@ -1 +0,0 @@ -<%= render "shared/map", new_url_path: new_proposal_path %> diff --git a/app/views/proposals/map.html.erb b/app/views/proposals/map.html.erb index 10dbf795f..dde4a610f 100644 --- a/app/views/proposals/map.html.erb +++ b/app/views/proposals/map.html.erb @@ -1 +1,50 @@ -<%= render "shared/map", new_url_path: new_proposal_path %> +
+
+ +

<%= t("map.title") %>

+ +
+
+
    + <% @geozones.each do |geozone| %> +
  • <%= link_to geozone.name, proposals_path(search: geozone.name) %>
  • + <% end %> +
+
+ +
+ <%= image_tag(image_path_for("map.jpg"), usemap: "#map") %> +
+ + + <% @geozones.each do |geozone| %> + <%= geozone.name %> + <% end %> + +
+ +

<%= t("map.proposal_for_district") %>

+ + <%= form_for(@proposal, url: new_proposal_path, method: :get) do |f| %> +
+ <%= f.select :geozone_id, geozone_select_options, include_blank: t("geozones.none") %> +
+ +
+ <%= f.submit(class: "button radius", value: t("map.start_proposal")) %> +
+ <% end %> +
+ +
+ +
+
diff --git a/app/views/shared/_map.html.erb b/app/views/shared/_map.html.erb deleted file mode 100644 index 004508c77..000000000 --- a/app/views/shared/_map.html.erb +++ /dev/null @@ -1,50 +0,0 @@ -
-
- -

<%= t("map.title") %>

- -
-
-
    - <% @geozones.each do |geozone| %> -
  • <%= link_to geozone.name, proposals_path(search: geozone.name) %>
  • - <% end %> -
-
- -
- <%= image_tag(image_path_for("map.jpg"), usemap: "#map") %> -
- - - <% @geozones.each do |geozone| %> - <%= geozone.name %> - <% end %> - -
- -

<%= t("map.proposal_for_district") %>

- - <%= form_for(@resource, url: new_url_path, method: :get) do |f| %> -
- <%= f.select :geozone_id, geozone_select_options, include_blank: t("geozones.none") %> -
- -
- <%= f.submit(class: "button radius", value: t("map.start_proposal")) %> -
- <% end %> -
- -
- -
-
diff --git a/config/routes/legislation.rb b/config/routes/legislation.rb index 0a06ad511..4a18ef1cd 100644 --- a/config/routes/legislation.rb +++ b/config/routes/legislation.rb @@ -21,7 +21,6 @@ namespace :legislation do put :unflag end collection do - get :map get :suggest end end From c7de42ab967ddd18cd5d21b81095c1f6833e36d0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Mon, 6 Mar 2023 17:44:33 +0100 Subject: [PATCH 02/18] Simplify map location coordinates helpers We're calling this method after setting the map location with `map_location = MapLocation.new if map_location.nil?`, so the condition `map_location.present?` is always going to be true. --- app/helpers/map_locations_helper.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/app/helpers/map_locations_helper.rb b/app/helpers/map_locations_helper.rb index 0cbee2b69..8bc28ce7d 100644 --- a/app/helpers/map_locations_helper.rb +++ b/app/helpers/map_locations_helper.rb @@ -4,15 +4,15 @@ module MapLocationsHelper end def map_location_latitude(map_location) - map_location.present? && map_location.latitude.present? ? map_location.latitude : Setting["map.latitude"] + map_location.latitude.presence || Setting["map.latitude"] end def map_location_longitude(map_location) - map_location.present? && map_location.longitude.present? ? map_location.longitude : Setting["map.longitude"] + map_location.longitude.presence || Setting["map.longitude"] end def map_location_zoom(map_location) - map_location.present? && map_location.zoom.present? ? map_location.zoom : Setting["map.zoom"] + map_location.zoom.presence || Setting["map.zoom"] end def map_location_input_id(prefix, attribute) From 9cc3c553ffcf31ea1a5882d96d60bf883a12976e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Mon, 6 Mar 2023 17:36:50 +0100 Subject: [PATCH 03/18] Extract component to render a map This way it'll be easier to refactor it. --- app/assets/stylesheets/application.scss | 1 + app/assets/stylesheets/layout.scss | 43 ------------- app/assets/stylesheets/map_location.scss | 38 ++++++++++++ .../shared/map_location_component.html.erb | 5 ++ .../shared/map_location_component.rb | 62 +++++++++++++++++++ app/helpers/map_locations_helper.rb | 55 +--------------- 6 files changed, 108 insertions(+), 96 deletions(-) create mode 100644 app/assets/stylesheets/map_location.scss create mode 100644 app/components/shared/map_location_component.html.erb create mode 100644 app/components/shared/map_location_component.rb diff --git a/app/assets/stylesheets/application.scss b/app/assets/stylesheets/application.scss index de9749210..383d13091 100644 --- a/app/assets/stylesheets/application.scss +++ b/app/assets/stylesheets/application.scss @@ -34,6 +34,7 @@ @import "legislation"; @import "legislation_process"; @import "legislation_process_form"; +@import "map_location"; @import "moderation_actions"; @import "notification_item"; @import "community"; diff --git a/app/assets/stylesheets/layout.scss b/app/assets/stylesheets/layout.scss index 96f9b4bdf..568e36891 100644 --- a/app/assets/stylesheets/layout.scss +++ b/app/assets/stylesheets/layout.scss @@ -19,7 +19,6 @@ // 20. Documents // 21. Related content // 22. Images -// 23. Maps // 24. Homepage // 25. LocalCensusRecords // @@ -2152,48 +2151,6 @@ table { } } -// 23. Maps -// ----------------- - -.location-map-remove-marker { - border-bottom: 1px dotted #cf2a0e; - color: $delete; - display: inline-block; - margin-top: $line-height / 2; - - &:hover, - &:active, - &:focus { - border-bottom-style: solid; - color: #cf2a0e; - text-decoration: none; - } -} - -.leaflet-bar a { - - &.leaflet-disabled { - color: #525252; - } -} - -.leaflet-container { - - .leaflet-control-attribution { - background: rgba(255, 255, 255, 0.9); - } - - a { - @include link; - } -} - -.leaflet-bottom, -.leaflet-pane, -.leaflet-top { - z-index: 4; -} - // 24. Homepage // ------------ diff --git a/app/assets/stylesheets/map_location.scss b/app/assets/stylesheets/map_location.scss new file mode 100644 index 000000000..282744fe8 --- /dev/null +++ b/app/assets/stylesheets/map_location.scss @@ -0,0 +1,38 @@ +.location-map-remove-marker { + border-bottom: 1px dotted #cf2a0e; + color: $delete; + display: inline-block; + margin-top: $line-height / 2; + + &:hover, + &:active, + &:focus { + border-bottom-style: solid; + color: #cf2a0e; + text-decoration: none; + } +} + +.leaflet-bar a { + + &.leaflet-disabled { + color: #525252; + } +} + +.leaflet-container { + + .leaflet-control-attribution { + background: rgba(255, 255, 255, 0.9); + } + + a { + @include link; + } +} + +.leaflet-bottom, +.leaflet-pane, +.leaflet-top { + z-index: 4; +} diff --git a/app/components/shared/map_location_component.html.erb b/app/components/shared/map_location_component.html.erb new file mode 100644 index 000000000..6a5084f95 --- /dev/null +++ b/app/components/shared/map_location_component.html.erb @@ -0,0 +1,5 @@ +<%= tag.div(id: dom_id(map_location), class: "map_location map", data: prepare_map_settings) %> + +<% if editable %> + <%= map_location_remove_marker %> +<% end %> diff --git a/app/components/shared/map_location_component.rb b/app/components/shared/map_location_component.rb new file mode 100644 index 000000000..4b317c51e --- /dev/null +++ b/app/components/shared/map_location_component.rb @@ -0,0 +1,62 @@ +class Shared::MapLocationComponent < ApplicationComponent + attr_reader :parent_class, :editable, :remove_marker_label, :investments_coordinates + delegate :map_location_input_id, to: :helpers + + def initialize(map_location, parent_class, editable, remove_marker_label, investments_coordinates = nil) + @map_location = map_location + @parent_class = parent_class + @editable = editable + @remove_marker_label = remove_marker_label + @investments_coordinates = investments_coordinates + end + + def map_location + @map_location ||= MapLocation.new + end + + private + + def map_location_latitude + map_location.latitude.presence || Setting["map.latitude"] + end + + def map_location_longitude + map_location.longitude.presence || Setting["map.longitude"] + end + + def map_location_zoom + map_location.zoom.presence || Setting["map.zoom"] + end + + def map_location_remove_marker_link_id + "remove-marker-link-#{dom_id(map_location)}" + end + + def map_location_remove_marker + tag.div class: "margin-bottom" do + link_to remove_marker_label, "#", + id: map_location_remove_marker_link_id, + class: "js-location-map-remove-marker location-map-remove-marker" + end + end + + def prepare_map_settings + options = { + map: "", + map_center_latitude: map_location_latitude, + map_center_longitude: map_location_longitude, + map_zoom: map_location_zoom, + 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: "##{map_location_remove_marker_link_id}", + latitude_input_selector: "##{map_location_input_id(parent_class, "latitude")}", + longitude_input_selector: "##{map_location_input_id(parent_class, "longitude")}", + zoom_input_selector: "##{map_location_input_id(parent_class, "zoom")}", + marker_investments_coordinates: investments_coordinates + } + options[:marker_latitude] = map_location.latitude if map_location.latitude.present? + options[:marker_longitude] = map_location.longitude if map_location.longitude.present? + options + end +end diff --git a/app/helpers/map_locations_helper.rb b/app/helpers/map_locations_helper.rb index 8bc28ce7d..1e6111b02 100644 --- a/app/helpers/map_locations_helper.rb +++ b/app/helpers/map_locations_helper.rb @@ -3,62 +3,11 @@ module MapLocationsHelper map_location.present? && map_location.available? end - def map_location_latitude(map_location) - map_location.latitude.presence || Setting["map.latitude"] - end - - def map_location_longitude(map_location) - map_location.longitude.presence || Setting["map.longitude"] - end - - def map_location_zoom(map_location) - map_location.zoom.presence || Setting["map.zoom"] - end - def map_location_input_id(prefix, attribute) "#{prefix}_map_location_attributes_#{attribute}" end - def map_location_remove_marker_link_id(map_location) - "remove-marker-link-#{dom_id(map_location)}" + def render_map(...) + render Shared::MapLocationComponent.new(...) end - - def render_map(map_location, parent_class, editable, remove_marker_label, investments_coordinates = nil) - map_location = MapLocation.new if map_location.nil? - map = tag.div id: dom_id(map_location), - class: "map_location map", - data: prepare_map_settings(map_location, editable, parent_class, investments_coordinates) - map += map_location_remove_marker(map_location, remove_marker_label) if editable - map - end - - def map_location_remove_marker(map_location, text) - tag.div class: "margin-bottom" do - link_to text, "#", - id: map_location_remove_marker_link_id(map_location), - class: "js-location-map-remove-marker location-map-remove-marker" - end - end - - private - - def prepare_map_settings(map_location, editable, parent_class, investments_coordinates = nil) - options = { - map: "", - map_center_latitude: map_location_latitude(map_location), - map_center_longitude: map_location_longitude(map_location), - map_zoom: map_location_zoom(map_location), - 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: "##{map_location_remove_marker_link_id(map_location)}", - latitude_input_selector: "##{map_location_input_id(parent_class, "latitude")}", - longitude_input_selector: "##{map_location_input_id(parent_class, "longitude")}", - zoom_input_selector: "##{map_location_input_id(parent_class, "zoom")}", - marker_investments_coordinates: investments_coordinates - } - options[:marker_latitude] = map_location.latitude if map_location.latitude.present? - options[:marker_longitude] = map_location.longitude if map_location.longitude.present? - options - end end From 19adae993efbbc4daaa57303d3b1078d2eff6d42 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Mon, 6 Mar 2023 17:40:42 +0100 Subject: [PATCH 04/18] Simplify method names in map component Since we aren't using helpers anymore, we don't need the `map_location` prefix. --- .../shared/map_location_component.html.erb | 4 ++-- .../shared/map_location_component.rb | 22 +++++++++---------- 2 files changed, 13 insertions(+), 13 deletions(-) diff --git a/app/components/shared/map_location_component.html.erb b/app/components/shared/map_location_component.html.erb index 6a5084f95..38cf42d35 100644 --- a/app/components/shared/map_location_component.html.erb +++ b/app/components/shared/map_location_component.html.erb @@ -1,5 +1,5 @@ -<%= tag.div(id: dom_id(map_location), class: "map_location map", data: prepare_map_settings) %> +<%= tag.div(id: dom_id(map_location), class: "map_location map", data: data) %> <% if editable %> - <%= map_location_remove_marker %> + <%= remove_marker %> <% end %> diff --git a/app/components/shared/map_location_component.rb b/app/components/shared/map_location_component.rb index 4b317c51e..1db74b044 100644 --- a/app/components/shared/map_location_component.rb +++ b/app/components/shared/map_location_component.rb @@ -16,40 +16,40 @@ class Shared::MapLocationComponent < ApplicationComponent private - def map_location_latitude + def latitude map_location.latitude.presence || Setting["map.latitude"] end - def map_location_longitude + def longitude map_location.longitude.presence || Setting["map.longitude"] end - def map_location_zoom + def zoom map_location.zoom.presence || Setting["map.zoom"] end - def map_location_remove_marker_link_id + def remove_marker_link_id "remove-marker-link-#{dom_id(map_location)}" end - def map_location_remove_marker + def remove_marker tag.div class: "margin-bottom" do link_to remove_marker_label, "#", - id: map_location_remove_marker_link_id, + id: remove_marker_link_id, class: "js-location-map-remove-marker location-map-remove-marker" end end - def prepare_map_settings + def data options = { map: "", - map_center_latitude: map_location_latitude, - map_center_longitude: map_location_longitude, - map_zoom: map_location_zoom, + map_center_latitude: latitude, + map_center_longitude: longitude, + map_zoom: zoom, 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: "##{map_location_remove_marker_link_id}", + marker_remove_selector: "##{remove_marker_link_id}", latitude_input_selector: "##{map_location_input_id(parent_class, "latitude")}", longitude_input_selector: "##{map_location_input_id(parent_class, "longitude")}", zoom_input_selector: "##{map_location_input_id(parent_class, "zoom")}", From deb965bcce89b8b6ee4c0b0acd7e92bebc036bcc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Tue, 7 Mar 2023 15:59:24 +0100 Subject: [PATCH 05/18] Remove editable parameter in render_map The calls to `render_map` are confusing since there are so many parameters. We can assume that the map is editable if we pass the remove marker label. --- app/components/budgets/map_component.html.erb | 2 +- app/components/shared/map_location_component.html.erb | 2 +- app/components/shared/map_location_component.rb | 11 +++++++---- .../budgets/investments/_investment_detail.html.erb | 2 +- app/views/budgets/investments/_map.html.erb | 2 +- app/views/map_locations/_form_fields.html.erb | 2 +- app/views/proposals/_info.html.erb | 2 +- 7 files changed, 13 insertions(+), 10 deletions(-) diff --git a/app/components/budgets/map_component.html.erb b/app/components/budgets/map_component.html.erb index 733ce56af..b839defa3 100644 --- a/app/components/budgets/map_component.html.erb +++ b/app/components/budgets/map_component.html.erb @@ -1,4 +1,4 @@

<%= t("budgets.index.map") %>

- <%= render_map(nil, "budgets", false, nil, coordinates) %> + <%= render_map(nil, "budgets", nil, coordinates) %>
diff --git a/app/components/shared/map_location_component.html.erb b/app/components/shared/map_location_component.html.erb index 38cf42d35..9283a2e3f 100644 --- a/app/components/shared/map_location_component.html.erb +++ b/app/components/shared/map_location_component.html.erb @@ -1,5 +1,5 @@ <%= tag.div(id: dom_id(map_location), class: "map_location map", data: data) %> -<% if editable %> +<% if editable? %> <%= remove_marker %> <% end %> diff --git a/app/components/shared/map_location_component.rb b/app/components/shared/map_location_component.rb index 1db74b044..9be1c86c0 100644 --- a/app/components/shared/map_location_component.rb +++ b/app/components/shared/map_location_component.rb @@ -1,11 +1,10 @@ class Shared::MapLocationComponent < ApplicationComponent - attr_reader :parent_class, :editable, :remove_marker_label, :investments_coordinates + attr_reader :parent_class, :remove_marker_label, :investments_coordinates delegate :map_location_input_id, to: :helpers - def initialize(map_location, parent_class, editable, remove_marker_label, investments_coordinates = nil) + def initialize(map_location, parent_class, remove_marker_label, investments_coordinates = nil) @map_location = map_location @parent_class = parent_class - @editable = editable @remove_marker_label = remove_marker_label @investments_coordinates = investments_coordinates end @@ -16,6 +15,10 @@ class Shared::MapLocationComponent < ApplicationComponent private + def editable? + remove_marker_label.present? + end + def latitude map_location.latitude.presence || Setting["map.latitude"] end @@ -48,7 +51,7 @@ class Shared::MapLocationComponent < ApplicationComponent map_zoom: zoom, map_tiles_provider: Rails.application.secrets.map_tiles_provider, map_tiles_provider_attribution: Rails.application.secrets.map_tiles_provider_attribution, - marker_editable: editable, + marker_editable: editable?, marker_remove_selector: "##{remove_marker_link_id}", latitude_input_selector: "##{map_location_input_id(parent_class, "latitude")}", longitude_input_selector: "##{map_location_input_id(parent_class, "longitude")}", diff --git a/app/views/budgets/investments/_investment_detail.html.erb b/app/views/budgets/investments/_investment_detail.html.erb index 046d1e659..456aac402 100644 --- a/app/views/budgets/investments/_investment_detail.html.erb +++ b/app/views/budgets/investments/_investment_detail.html.erb @@ -28,7 +28,7 @@ <% if feature?(:map) && map_location_available?(@investment.map_location) %>
- <%= render_map(investment.map_location, "budget_investment", false, nil) %> + <%= render_map(investment.map_location, "budget_investment", nil) %>
<% end %> diff --git a/app/views/budgets/investments/_map.html.erb b/app/views/budgets/investments/_map.html.erb index 3d26ee7bc..085e4fe90 100644 --- a/app/views/budgets/investments/_map.html.erb +++ b/app/views/budgets/investments/_map.html.erb @@ -1,3 +1,3 @@
- <%= render_map(@map_location, "budgets", false, nil, @investments_map_coordinates) %> + <%= render_map(@map_location, "budgets", nil, @investments_map_coordinates) %>
diff --git a/app/views/map_locations/_form_fields.html.erb b/app/views/map_locations/_form_fields.html.erb index 75236f140..07c0fba17 100644 --- a/app/views/map_locations/_form_fields.html.erb +++ b/app/views/map_locations/_form_fields.html.erb @@ -1,7 +1,7 @@ <%= form.label :map_location, label %>

<%= help %>

-<%= render_map(map_location, parent_class, editable = true, remove_marker_label) %> +<%= render_map(map_location, parent_class, remove_marker_label) %> <%= form.fields_for :map_location, map_location do |m_l_fields| %> <%= m_l_fields.hidden_field :latitude, diff --git a/app/views/proposals/_info.html.erb b/app/views/proposals/_info.html.erb index 88b31d6e2..686e8dfa1 100644 --- a/app/views/proposals/_info.html.erb +++ b/app/views/proposals/_info.html.erb @@ -42,7 +42,7 @@ <% if feature?(:map) && map_location_available?(@proposal.map_location) %>
- <%= render_map(@proposal.map_location, "proposal", false, nil) %> + <%= render_map(@proposal.map_location, "proposal", nil) %>
<% end %> From aa5f5235dee6f3ae95d3ad6f23320d0d03152222 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Tue, 7 Mar 2023 17:47:05 +0100 Subject: [PATCH 06/18] Simplify creating a map location from a heading --- app/controllers/budgets/investments_controller.rb | 2 +- app/models/map_location.rb | 12 ++++++------ 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/app/controllers/budgets/investments_controller.rb b/app/controllers/budgets/investments_controller.rb index e2dc790ee..ca9a18d0c 100644 --- a/app/controllers/budgets/investments_controller.rb +++ b/app/controllers/budgets/investments_controller.rb @@ -181,7 +181,7 @@ module Budgets end def load_map - @map_location = MapLocation.load_from_heading(@heading) if @heading.present? + @map_location = MapLocation.from_heading(@heading) if @heading.present? end end end diff --git a/app/models/map_location.rb b/app/models/map_location.rb index 3ab33af42..6ed4fc9cc 100644 --- a/app/models/map_location.rb +++ b/app/models/map_location.rb @@ -17,11 +17,11 @@ class MapLocation < ApplicationRecord } end - def self.load_from_heading(heading) - map = new - map.zoom = Budget::Heading::OSM_DISTRICT_LEVEL_ZOOM - map.latitude = heading.latitude.to_f if heading.latitude.present? - map.longitude = heading.longitude.to_f if heading.longitude.present? - map + def self.from_heading(heading) + new( + zoom: Budget::Heading::OSM_DISTRICT_LEVEL_ZOOM, + latitude: (heading.latitude.to_f if heading.latitude.present?), + longitude: (heading.longitude.to_f if heading.longitude.present?) + ) end end From e00aa807b958e1a016676b07c3fbacf840627ead Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Tue, 7 Mar 2023 18:10:33 +0100 Subject: [PATCH 07/18] Make remove marker label parameter optional We were passing `nil` in some calls, which was confusing. Since now we've got two optional parameters, we're using named parameters. --- app/components/budgets/map_component.html.erb | 2 +- app/components/shared/map_location_component.rb | 2 +- app/views/budgets/investments/_investment_detail.html.erb | 2 +- app/views/budgets/investments/_map.html.erb | 2 +- app/views/map_locations/_form_fields.html.erb | 2 +- app/views/proposals/_info.html.erb | 2 +- 6 files changed, 6 insertions(+), 6 deletions(-) diff --git a/app/components/budgets/map_component.html.erb b/app/components/budgets/map_component.html.erb index b839defa3..f6f595b3b 100644 --- a/app/components/budgets/map_component.html.erb +++ b/app/components/budgets/map_component.html.erb @@ -1,4 +1,4 @@

<%= t("budgets.index.map") %>

- <%= render_map(nil, "budgets", nil, coordinates) %> + <%= render_map(nil, "budgets", investments_coordinates: coordinates) %>
diff --git a/app/components/shared/map_location_component.rb b/app/components/shared/map_location_component.rb index 9be1c86c0..693589541 100644 --- a/app/components/shared/map_location_component.rb +++ b/app/components/shared/map_location_component.rb @@ -2,7 +2,7 @@ class Shared::MapLocationComponent < ApplicationComponent attr_reader :parent_class, :remove_marker_label, :investments_coordinates delegate :map_location_input_id, to: :helpers - def initialize(map_location, parent_class, remove_marker_label, investments_coordinates = nil) + def initialize(map_location, parent_class, remove_marker_label: nil, investments_coordinates: nil) @map_location = map_location @parent_class = parent_class @remove_marker_label = remove_marker_label diff --git a/app/views/budgets/investments/_investment_detail.html.erb b/app/views/budgets/investments/_investment_detail.html.erb index 456aac402..7c758dcc3 100644 --- a/app/views/budgets/investments/_investment_detail.html.erb +++ b/app/views/budgets/investments/_investment_detail.html.erb @@ -28,7 +28,7 @@ <% if feature?(:map) && map_location_available?(@investment.map_location) %>
- <%= render_map(investment.map_location, "budget_investment", nil) %> + <%= render_map(investment.map_location, "budget_investment") %>
<% end %> diff --git a/app/views/budgets/investments/_map.html.erb b/app/views/budgets/investments/_map.html.erb index 085e4fe90..cc549a90e 100644 --- a/app/views/budgets/investments/_map.html.erb +++ b/app/views/budgets/investments/_map.html.erb @@ -1,3 +1,3 @@
- <%= render_map(@map_location, "budgets", nil, @investments_map_coordinates) %> + <%= render_map(@map_location, "budgets", investments_coordinates: @investments_map_coordinates) %>
diff --git a/app/views/map_locations/_form_fields.html.erb b/app/views/map_locations/_form_fields.html.erb index 07c0fba17..4a26a5758 100644 --- a/app/views/map_locations/_form_fields.html.erb +++ b/app/views/map_locations/_form_fields.html.erb @@ -1,7 +1,7 @@ <%= form.label :map_location, label %>

<%= help %>

-<%= render_map(map_location, parent_class, remove_marker_label) %> +<%= render_map(map_location, parent_class, remove_marker_label: remove_marker_label) %> <%= form.fields_for :map_location, map_location do |m_l_fields| %> <%= m_l_fields.hidden_field :latitude, diff --git a/app/views/proposals/_info.html.erb b/app/views/proposals/_info.html.erb index 686e8dfa1..b5a5ebe79 100644 --- a/app/views/proposals/_info.html.erb +++ b/app/views/proposals/_info.html.erb @@ -42,7 +42,7 @@ <% if feature?(:map) && map_location_available?(@proposal.map_location) %>
- <%= render_map(@proposal.map_location, "proposal", nil) %> + <%= render_map(@proposal.map_location, "proposal") %>
<% end %> From 84fff2e9fbd97ae0704d6a5eb906043cbf40b94a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Tue, 7 Mar 2023 18:45:15 +0100 Subject: [PATCH 08/18] Simplify code setting marker data in maps We were probably setting them separately to avoid having blank data attributes in the HTML. However, when a data attribute is `nil`, Rails doesn't write it in the HTML in the first place. --- app/components/shared/map_location_component.rb | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/app/components/shared/map_location_component.rb b/app/components/shared/map_location_component.rb index 693589541..c0f88831d 100644 --- a/app/components/shared/map_location_component.rb +++ b/app/components/shared/map_location_component.rb @@ -44,7 +44,7 @@ class Shared::MapLocationComponent < ApplicationComponent end def data - options = { + { map: "", map_center_latitude: latitude, map_center_longitude: longitude, @@ -56,10 +56,9 @@ class Shared::MapLocationComponent < ApplicationComponent latitude_input_selector: "##{map_location_input_id(parent_class, "latitude")}", longitude_input_selector: "##{map_location_input_id(parent_class, "longitude")}", zoom_input_selector: "##{map_location_input_id(parent_class, "zoom")}", - marker_investments_coordinates: investments_coordinates + marker_investments_coordinates: investments_coordinates, + marker_latitude: map_location.latitude.presence, + marker_longitude: map_location.longitude.presence } - options[:marker_latitude] = map_location.latitude if map_location.latitude.present? - options[:marker_longitude] = map_location.longitude if map_location.longitude.present? - options end end From b9518d64e14de07fdd84a1d9f436af61f4ad9da1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Tue, 7 Mar 2023 19:18:45 +0100 Subject: [PATCH 09/18] Use Rails methods to get map location input IDs We were manually generating the IDs in order to pass them as data attributes in the HTML in a component where we don't have access to the form which has the inputs. However, these data attributes only make sense when there's a form present, so we can pass the form as a parameter and use it to get the IDs. We can now define a map as editable when there's an associated form, which makes sense IMHO. --- .../investments/form_component.html.erb | 1 - app/components/budgets/map_component.html.erb | 2 +- .../proposals/form_component.html.erb | 1 - .../shared/map_location_component.rb | 30 +++++++++++++------ app/helpers/map_locations_helper.rb | 4 --- .../investments/_investment_detail.html.erb | 2 +- app/views/budgets/investments/_map.html.erb | 2 +- app/views/map_locations/_form_fields.html.erb | 16 ++++------ app/views/proposals/_info.html.erb | 2 +- 9 files changed, 30 insertions(+), 30 deletions(-) diff --git a/app/components/budgets/investments/form_component.html.erb b/app/components/budgets/investments/form_component.html.erb index 3705bc10e..e065b95cf 100644 --- a/app/components/budgets/investments/form_component.html.erb +++ b/app/components/budgets/investments/form_component.html.erb @@ -56,7 +56,6 @@ label: t("budgets.investments.form.map_location"), help: t("budgets.investments.form.map_location_instructions"), remove_marker_label: t("budgets.investments.form.map_remove_marker"), - parent_class: "budget_investment", i18n_namespace: "budgets.investments" %> <% end %> diff --git a/app/components/budgets/map_component.html.erb b/app/components/budgets/map_component.html.erb index f6f595b3b..fa2284472 100644 --- a/app/components/budgets/map_component.html.erb +++ b/app/components/budgets/map_component.html.erb @@ -1,4 +1,4 @@

<%= t("budgets.index.map") %>

- <%= render_map(nil, "budgets", investments_coordinates: coordinates) %> + <%= render_map(nil, investments_coordinates: coordinates) %>
diff --git a/app/components/proposals/form_component.html.erb b/app/components/proposals/form_component.html.erb index 566391e58..ace70b75f 100644 --- a/app/components/proposals/form_component.html.erb +++ b/app/components/proposals/form_component.html.erb @@ -62,7 +62,6 @@ label: t("proposals.form.map_location"), help: t("proposals.form.map_location_instructions"), remove_marker_label: t("proposals.form.map_remove_marker"), - parent_class: "proposal", i18n_namespace: "proposals" %> <% end %> diff --git a/app/components/shared/map_location_component.rb b/app/components/shared/map_location_component.rb index c0f88831d..aafdcfd01 100644 --- a/app/components/shared/map_location_component.rb +++ b/app/components/shared/map_location_component.rb @@ -1,12 +1,11 @@ class Shared::MapLocationComponent < ApplicationComponent - attr_reader :parent_class, :remove_marker_label, :investments_coordinates - delegate :map_location_input_id, to: :helpers + attr_reader :remove_marker_label, :investments_coordinates, :form - def initialize(map_location, parent_class, remove_marker_label: nil, investments_coordinates: nil) + def initialize(map_location, remove_marker_label: nil, investments_coordinates: nil, form: nil) @map_location = map_location - @parent_class = parent_class @remove_marker_label = remove_marker_label @investments_coordinates = investments_coordinates + @form = form end def map_location @@ -16,7 +15,7 @@ class Shared::MapLocationComponent < ApplicationComponent private def editable? - remove_marker_label.present? + form.present? end def latitude @@ -53,12 +52,25 @@ class Shared::MapLocationComponent < ApplicationComponent map_tiles_provider_attribution: Rails.application.secrets.map_tiles_provider_attribution, marker_editable: editable?, marker_remove_selector: "##{remove_marker_link_id}", - latitude_input_selector: "##{map_location_input_id(parent_class, "latitude")}", - longitude_input_selector: "##{map_location_input_id(parent_class, "longitude")}", - zoom_input_selector: "##{map_location_input_id(parent_class, "zoom")}", marker_investments_coordinates: investments_coordinates, marker_latitude: map_location.latitude.presence, marker_longitude: map_location.longitude.presence - } + }.merge(input_selectors) + end + + def input_selectors + if form + { + latitude_input_selector: "##{input_id(:latitude)}", + longitude_input_selector: "##{input_id(:longitude)}", + zoom_input_selector: "##{input_id(:zoom)}" + } + else + {} + end + end + + def input_id(attribute) + form.hidden_field(attribute).match(/ id="([^"]+)"/)[1] end end diff --git a/app/helpers/map_locations_helper.rb b/app/helpers/map_locations_helper.rb index 1e6111b02..3b56ace3d 100644 --- a/app/helpers/map_locations_helper.rb +++ b/app/helpers/map_locations_helper.rb @@ -3,10 +3,6 @@ module MapLocationsHelper map_location.present? && map_location.available? end - def map_location_input_id(prefix, attribute) - "#{prefix}_map_location_attributes_#{attribute}" - end - def render_map(...) render Shared::MapLocationComponent.new(...) end diff --git a/app/views/budgets/investments/_investment_detail.html.erb b/app/views/budgets/investments/_investment_detail.html.erb index 7c758dcc3..c7d23dae7 100644 --- a/app/views/budgets/investments/_investment_detail.html.erb +++ b/app/views/budgets/investments/_investment_detail.html.erb @@ -28,7 +28,7 @@ <% if feature?(:map) && map_location_available?(@investment.map_location) %>
- <%= render_map(investment.map_location, "budget_investment") %> + <%= render_map(investment.map_location) %>
<% end %> diff --git a/app/views/budgets/investments/_map.html.erb b/app/views/budgets/investments/_map.html.erb index cc549a90e..be68bcdbe 100644 --- a/app/views/budgets/investments/_map.html.erb +++ b/app/views/budgets/investments/_map.html.erb @@ -1,3 +1,3 @@
- <%= render_map(@map_location, "budgets", investments_coordinates: @investments_map_coordinates) %> + <%= render_map(@map_location, investments_coordinates: @investments_map_coordinates) %>
diff --git a/app/views/map_locations/_form_fields.html.erb b/app/views/map_locations/_form_fields.html.erb index 4a26a5758..3f8233f43 100644 --- a/app/views/map_locations/_form_fields.html.erb +++ b/app/views/map_locations/_form_fields.html.erb @@ -1,16 +1,10 @@ <%= form.label :map_location, label %>

<%= help %>

-<%= render_map(map_location, parent_class, remove_marker_label: remove_marker_label) %> - <%= form.fields_for :map_location, map_location do |m_l_fields| %> - <%= m_l_fields.hidden_field :latitude, - value: map_location.latitude, - id: map_location_input_id(parent_class, "latitude") %> - <%= m_l_fields.hidden_field :longitude, - value: map_location.longitude, - id: map_location_input_id(parent_class, "longitude") %> - <%= m_l_fields.hidden_field :zoom, - value: map_location.zoom, - id: map_location_input_id(parent_class, "zoom") %> + <%= render_map(map_location, remove_marker_label: remove_marker_label, form: m_l_fields) %> + + <%= m_l_fields.hidden_field :latitude, value: map_location.latitude %> + <%= m_l_fields.hidden_field :longitude, value: map_location.longitude %> + <%= m_l_fields.hidden_field :zoom, value: map_location.zoom %> <% end %> diff --git a/app/views/proposals/_info.html.erb b/app/views/proposals/_info.html.erb index b5a5ebe79..71060a514 100644 --- a/app/views/proposals/_info.html.erb +++ b/app/views/proposals/_info.html.erb @@ -42,7 +42,7 @@ <% if feature?(:map) && map_location_available?(@proposal.map_location) %>
- <%= render_map(@proposal.map_location, "proposal") %> + <%= render_map(@proposal.map_location) %>
<% end %> From c667582c989e991175ad9f58287825d0836c6e17 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Tue, 7 Mar 2023 19:21:37 +0100 Subject: [PATCH 10/18] Remove redundant value assignments in map fields Rails forms automatically take the value from the object related to the form. --- app/views/map_locations/_form_fields.html.erb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/app/views/map_locations/_form_fields.html.erb b/app/views/map_locations/_form_fields.html.erb index 3f8233f43..072fc82ff 100644 --- a/app/views/map_locations/_form_fields.html.erb +++ b/app/views/map_locations/_form_fields.html.erb @@ -4,7 +4,7 @@ <%= form.fields_for :map_location, map_location do |m_l_fields| %> <%= render_map(map_location, remove_marker_label: remove_marker_label, form: m_l_fields) %> - <%= m_l_fields.hidden_field :latitude, value: map_location.latitude %> - <%= m_l_fields.hidden_field :longitude, value: map_location.longitude %> - <%= m_l_fields.hidden_field :zoom, value: map_location.zoom %> + <%= m_l_fields.hidden_field :latitude %> + <%= m_l_fields.hidden_field :longitude %> + <%= m_l_fields.hidden_field :zoom %> <% end %> From e75211125abcfb1e6bbc0ead44843cd82fbc0daa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Tue, 7 Mar 2023 19:36:52 +0100 Subject: [PATCH 11/18] Remove duplicate i18n text to remove marker We had two different keys with the same text and were passing it as a parameter. Since the text is the same in any case, we don't need a parameter for it. Note we are using the `proposals` i18n key instead of creating a new one in a `shared` namespace one because creating a new key would mean that we'd lose the already existing translations in Crowdin. --- .../budgets/investments/form_component.html.erb | 1 - app/components/proposals/form_component.html.erb | 1 - app/components/shared/map_location_component.rb | 9 ++++++--- app/views/map_locations/_form_fields.html.erb | 2 +- config/locales/en/budgets.yml | 1 - config/locales/es/budgets.yml | 1 - 6 files changed, 7 insertions(+), 8 deletions(-) diff --git a/app/components/budgets/investments/form_component.html.erb b/app/components/budgets/investments/form_component.html.erb index e065b95cf..20b15c201 100644 --- a/app/components/budgets/investments/form_component.html.erb +++ b/app/components/budgets/investments/form_component.html.erb @@ -55,7 +55,6 @@ map_location: investment.map_location || MapLocation.new, label: t("budgets.investments.form.map_location"), help: t("budgets.investments.form.map_location_instructions"), - remove_marker_label: t("budgets.investments.form.map_remove_marker"), i18n_namespace: "budgets.investments" %> <% end %> diff --git a/app/components/proposals/form_component.html.erb b/app/components/proposals/form_component.html.erb index ace70b75f..88d7a2cf7 100644 --- a/app/components/proposals/form_component.html.erb +++ b/app/components/proposals/form_component.html.erb @@ -61,7 +61,6 @@ map_location: proposal.map_location || MapLocation.new, label: t("proposals.form.map_location"), help: t("proposals.form.map_location_instructions"), - remove_marker_label: t("proposals.form.map_remove_marker"), i18n_namespace: "proposals" %> <% end %> diff --git a/app/components/shared/map_location_component.rb b/app/components/shared/map_location_component.rb index aafdcfd01..334375d08 100644 --- a/app/components/shared/map_location_component.rb +++ b/app/components/shared/map_location_component.rb @@ -1,9 +1,8 @@ class Shared::MapLocationComponent < ApplicationComponent - attr_reader :remove_marker_label, :investments_coordinates, :form + attr_reader :investments_coordinates, :form - def initialize(map_location, remove_marker_label: nil, investments_coordinates: nil, form: nil) + def initialize(map_location, investments_coordinates: nil, form: nil) @map_location = map_location - @remove_marker_label = remove_marker_label @investments_coordinates = investments_coordinates @form = form end @@ -30,6 +29,10 @@ class Shared::MapLocationComponent < ApplicationComponent map_location.zoom.presence || Setting["map.zoom"] end + def remove_marker_label + t("proposals.form.map_remove_marker") + end + def remove_marker_link_id "remove-marker-link-#{dom_id(map_location)}" end diff --git a/app/views/map_locations/_form_fields.html.erb b/app/views/map_locations/_form_fields.html.erb index 072fc82ff..403368b1b 100644 --- a/app/views/map_locations/_form_fields.html.erb +++ b/app/views/map_locations/_form_fields.html.erb @@ -2,7 +2,7 @@

<%= help %>

<%= form.fields_for :map_location, map_location do |m_l_fields| %> - <%= render_map(map_location, remove_marker_label: remove_marker_label, form: m_l_fields) %> + <%= render_map(map_location, form: m_l_fields) %> <%= m_l_fields.hidden_field :latitude %> <%= m_l_fields.hidden_field :longitude %> diff --git a/config/locales/en/budgets.yml b/config/locales/en/budgets.yml index ab3b055d0..63c47bf4a 100644 --- a/config/locales/en/budgets.yml +++ b/config/locales/en/budgets.yml @@ -82,7 +82,6 @@ en: tags_placeholder: "Enter the tags you would like to use, separated by commas (',')" map_location: "Map location" map_location_instructions: "Navigate the map to the location and place the marker." - map_remove_marker: "Remove map marker" index: title: Participatory budgeting unfeasible: Unfeasible investment projects diff --git a/config/locales/es/budgets.yml b/config/locales/es/budgets.yml index 7991b9dd2..53c5a4b35 100644 --- a/config/locales/es/budgets.yml +++ b/config/locales/es/budgets.yml @@ -82,7 +82,6 @@ es: tags_placeholder: "Escribe las etiquetas que desees separadas por coma (',')" map_location: "Ubicación en el mapa" map_location_instructions: "Navega por el mapa hasta la ubicación y coloca el marcador." - map_remove_marker: "Eliminar el marcador" index: title: Presupuestos participativos unfeasible: Proyectos de gasto no viables From 00cd91c6b2e8581b721b8f320641e47aee9b8467 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Mon, 24 Apr 2023 16:40:15 +0200 Subject: [PATCH 12/18] Extract functions to get coordinates in map JS We had 130 lines long function, and we're trying to reduce its size so it's easier to follow the code. --- app/assets/javascripts/map.js | 127 ++++++++++++++++++++++------------ 1 file changed, 81 insertions(+), 46 deletions(-) diff --git a/app/assets/javascripts/map.js b/app/assets/javascripts/map.js index 15d558dc2..7f769a86f 100644 --- a/app/assets/javascripts/map.js +++ b/app/assets/javascripts/map.js @@ -18,45 +18,13 @@ App.Map.maps = []; }, initializeMap: function(element) { - var addMarkerInvestments, clearFormfields, createMarker, dataCoordinates, editable, formCoordinates, - getPopupContent, latitudeInputSelector, longitudeInputSelector, map, mapAttribution, mapCenterLatLng, - mapCenterLatitude, mapCenterLongitude, mapTilesProvider, marker, markerIcon, markerLatitude, - markerLongitude, moveOrPlaceMarker, openMarkerPopup, removeMarker, removeMarkerSelector, - updateFormfields, zoom, zoomInputSelector; + var addMarkerInvestments, centerData, clearFormfields, createMarker, + editable, getPopupContent, markerData, map, mapAttribution, + mapCenterLatLng, mapTilesProvider, marker, markerIcon, moveOrPlaceMarker, + openMarkerPopup, removeMarker, removeMarkerSelector, updateFormfields; App.Map.cleanInvestmentCoordinates(element); mapTilesProvider = $(element).data("map-tiles-provider"); mapAttribution = $(element).data("map-tiles-provider-attribution"); - latitudeInputSelector = $(element).data("latitude-input-selector"); - longitudeInputSelector = $(element).data("longitude-input-selector"); - zoomInputSelector = $(element).data("zoom-input-selector"); - formCoordinates = { - lat: $(latitudeInputSelector).val(), - long: $(longitudeInputSelector).val(), - zoom: $(zoomInputSelector).val() - }; - dataCoordinates = { - lat: $(element).data("marker-latitude"), - long: $(element).data("marker-longitude") - }; - if (App.Map.validCoordinates(formCoordinates)) { - markerLatitude = formCoordinates.lat; - markerLongitude = formCoordinates.long; - mapCenterLatitude = formCoordinates.lat; - mapCenterLongitude = formCoordinates.long; - } else if (App.Map.validCoordinates(dataCoordinates)) { - markerLatitude = dataCoordinates.lat; - markerLongitude = dataCoordinates.long; - mapCenterLatitude = dataCoordinates.lat; - mapCenterLongitude = dataCoordinates.long; - } else { - mapCenterLatitude = $(element).data("map-center-latitude"); - mapCenterLongitude = $(element).data("map-center-longitude"); - } - if (App.Map.validZoom(formCoordinates.zoom)) { - zoom = formCoordinates.zoom; - } else { - zoom = $(element).data("map-zoom"); - } removeMarkerSelector = $(element).data("marker-remove-selector"); addMarkerInvestments = $(element).data("marker-investments-coordinates"); editable = $(element).data("marker-editable"); @@ -97,14 +65,18 @@ updateFormfields(); }; updateFormfields = function() { - $(latitudeInputSelector).val(marker.getLatLng().lat); - $(longitudeInputSelector).val(marker.getLatLng().lng); - $(zoomInputSelector).val(map.getZoom()); + var inputs = App.Map.coordinatesInputs(element); + + inputs.lat.val(marker.getLatLng().lat); + inputs.long.val(marker.getLatLng().lng); + inputs.zoom.val(map.getZoom()); }; clearFormfields = function() { - $(latitudeInputSelector).val(""); - $(longitudeInputSelector).val(""); - $(zoomInputSelector).val(""); + var inputs = App.Map.coordinatesInputs(element); + + inputs.lat.val(""); + inputs.long.val(""); + inputs.zoom.val(""); }; openMarkerPopup = function(e) { marker = e.target; @@ -119,15 +91,19 @@ getPopupContent = function(data) { return "" + data.investment_title + ""; }; - mapCenterLatLng = new L.LatLng(mapCenterLatitude, mapCenterLongitude); - map = L.map(element.id, { scrollWheelZoom: false }).setView(mapCenterLatLng, zoom); + + centerData = App.Map.centerData(element); + mapCenterLatLng = new L.LatLng(centerData.lat, centerData.long); + map = L.map(element.id, { scrollWheelZoom: false }).setView(mapCenterLatLng, centerData.zoom); map.attributionControl.setPrefix(App.Map.attributionPrefix()); App.Map.maps.push(map); L.tileLayer(mapTilesProvider, { attribution: mapAttribution }).addTo(map); - if (markerLatitude && markerLongitude && !addMarkerInvestments) { - marker = createMarker(markerLatitude, markerLongitude); + + markerData = App.Map.markerData(element); + if (markerData.lat && markerData.long && !addMarkerInvestments) { + marker = createMarker(markerData.lat, markerData.long); } if (editable) { $(removeMarkerSelector).on("click", removeMarker); @@ -148,6 +124,65 @@ }); } }, + markerData: function(element) { + var dataCoordinates, formCoordinates, inputs, latitude, longitude; + inputs = App.Map.coordinatesInputs(element); + + dataCoordinates = { + lat: $(element).data("marker-latitude"), + long: $(element).data("marker-longitude") + }; + formCoordinates = { + lat: inputs.lat.val(), + long: inputs.long.val(), + zoom: inputs.zoom.val() + }; + if (App.Map.validCoordinates(formCoordinates)) { + latitude = formCoordinates.lat; + longitude = formCoordinates.long; + } else if (App.Map.validCoordinates(dataCoordinates)) { + latitude = dataCoordinates.lat; + longitude = dataCoordinates.long; + } + + return { + lat: latitude, + long: longitude, + zoom: formCoordinates.zoom + }; + }, + centerData: function(element) { + var markerCoordinates, latitude, longitude, zoom; + + markerCoordinates = App.Map.markerData(element); + + if (App.Map.validCoordinates(markerCoordinates)) { + latitude = markerCoordinates.lat; + longitude = markerCoordinates.long; + } else { + latitude = $(element).data("map-center-latitude"); + longitude = $(element).data("map-center-longitude"); + } + + if (App.Map.validZoom(markerCoordinates.zoom)) { + zoom = markerCoordinates.zoom; + } else { + zoom = $(element).data("map-zoom"); + } + + return { + lat: latitude, + long: longitude, + zoom: zoom + }; + }, + coordinatesInputs: function(element) { + return { + lat: $($(element).data("latitude-input-selector")), + long: $($(element).data("longitude-input-selector")), + zoom: $($(element).data("zoom-input-selector")) + }; + }, cleanInvestmentCoordinates: function(element) { var clean_markers, markers; markers = $(element).attr("data-marker-investments-coordinates"); From 4087066c598eb5ccdf75c365dcb0ec10562f51d3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Mon, 24 Apr 2023 17:08:19 +0200 Subject: [PATCH 13/18] Extract function to add map attribution --- app/assets/javascripts/map.js | 29 +++++++++++++++++------------ 1 file changed, 17 insertions(+), 12 deletions(-) diff --git a/app/assets/javascripts/map.js b/app/assets/javascripts/map.js index 7f769a86f..093ee7980 100644 --- a/app/assets/javascripts/map.js +++ b/app/assets/javascripts/map.js @@ -7,9 +7,6 @@ App.Map.initializeMap(this); }); }, - attributionPrefix: function() { - return 'Leaflet'; - }, destroy: function() { App.Map.maps.forEach(function(map) { map.off(); @@ -19,12 +16,10 @@ }, initializeMap: function(element) { var addMarkerInvestments, centerData, clearFormfields, createMarker, - editable, getPopupContent, markerData, map, mapAttribution, - mapCenterLatLng, mapTilesProvider, marker, markerIcon, moveOrPlaceMarker, - openMarkerPopup, removeMarker, removeMarkerSelector, updateFormfields; + editable, getPopupContent, markerData, map, mapCenterLatLng, marker, + markerIcon, moveOrPlaceMarker, openMarkerPopup, removeMarker, + removeMarkerSelector, updateFormfields; App.Map.cleanInvestmentCoordinates(element); - mapTilesProvider = $(element).data("map-tiles-provider"); - mapAttribution = $(element).data("map-tiles-provider-attribution"); removeMarkerSelector = $(element).data("marker-remove-selector"); addMarkerInvestments = $(element).data("marker-investments-coordinates"); editable = $(element).data("marker-editable"); @@ -95,11 +90,8 @@ centerData = App.Map.centerData(element); mapCenterLatLng = new L.LatLng(centerData.lat, centerData.long); map = L.map(element.id, { scrollWheelZoom: false }).setView(mapCenterLatLng, centerData.zoom); - map.attributionControl.setPrefix(App.Map.attributionPrefix()); App.Map.maps.push(map); - L.tileLayer(mapTilesProvider, { - attribution: mapAttribution - }).addTo(map); + App.Map.addAttribution(map); markerData = App.Map.markerData(element); if (markerData.lat && markerData.long && !addMarkerInvestments) { @@ -124,6 +116,9 @@ }); } }, + attributionPrefix: function() { + return 'Leaflet'; + }, markerData: function(element) { var dataCoordinates, formCoordinates, inputs, latitude, longitude; inputs = App.Map.coordinatesInputs(element); @@ -191,6 +186,16 @@ $(element).attr("data-marker-investments-coordinates", clean_markers); } }, + addAttribution: function(map) { + var element, mapAttribution, mapTilesProvider; + + element = map._container; + mapTilesProvider = $(element).data("map-tiles-provider"); + mapAttribution = $(element).data("map-tiles-provider-attribution"); + + map.attributionControl.setPrefix(App.Map.attributionPrefix()); + L.tileLayer(mapTilesProvider, { attribution: mapAttribution }).addTo(map); + }, validZoom: function(zoom) { return App.Map.isNumeric(zoom); }, From f8053c9532e2c933eb7db3a6cbe929bcd1c0a78a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Mon, 24 Apr 2023 17:11:50 +0200 Subject: [PATCH 14/18] Extract function to get popup in map JavaScript --- app/assets/javascripts/map.js | 34 +++++++++++++++++----------------- 1 file changed, 17 insertions(+), 17 deletions(-) diff --git a/app/assets/javascripts/map.js b/app/assets/javascripts/map.js index 093ee7980..df9fd156b 100644 --- a/app/assets/javascripts/map.js +++ b/app/assets/javascripts/map.js @@ -16,9 +16,8 @@ }, initializeMap: function(element) { var addMarkerInvestments, centerData, clearFormfields, createMarker, - editable, getPopupContent, markerData, map, mapCenterLatLng, marker, - markerIcon, moveOrPlaceMarker, openMarkerPopup, removeMarker, - removeMarkerSelector, updateFormfields; + editable, markerData, map, mapCenterLatLng, marker, markerIcon, + moveOrPlaceMarker, removeMarker, removeMarkerSelector, updateFormfields; App.Map.cleanInvestmentCoordinates(element); removeMarkerSelector = $(element).data("marker-remove-selector"); addMarkerInvestments = $(element).data("marker-investments-coordinates"); @@ -73,19 +72,6 @@ inputs.long.val(""); inputs.zoom.val(""); }; - openMarkerPopup = function(e) { - marker = e.target; - $.ajax("/investments/" + marker.options.id + "/json_data", { - type: "GET", - dataType: "json", - success: function(data) { - e.target.bindPopup(getPopupContent(data)).openPopup(); - } - }); - }; - getPopupContent = function(data) { - return "" + data.investment_title + ""; - }; centerData = App.Map.centerData(element); mapCenterLatLng = new L.LatLng(centerData.lat, centerData.long); @@ -111,7 +97,7 @@ if (App.Map.validCoordinates(coordinates)) { marker = createMarker(coordinates.lat, coordinates.long); marker.options.id = coordinates.investment_id; - marker.on("click", openMarkerPopup); + marker.on("click", App.Map.openMarkerPopup); } }); } @@ -196,6 +182,20 @@ map.attributionControl.setPrefix(App.Map.attributionPrefix()); L.tileLayer(mapTilesProvider, { attribution: mapAttribution }).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 + ""; + }, validZoom: function(zoom) { return App.Map.isNumeric(zoom); }, From 2e8bc11c2a9c66c5cdc0aebc6b3316a0c1932ef0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Mon, 24 Apr 2023 17:56:02 +0200 Subject: [PATCH 15/18] Extract functions to update map form fields --- app/assets/javascripts/map.js | 44 ++++++++++++++++++----------------- 1 file changed, 23 insertions(+), 21 deletions(-) diff --git a/app/assets/javascripts/map.js b/app/assets/javascripts/map.js index df9fd156b..d84d917c3 100644 --- a/app/assets/javascripts/map.js +++ b/app/assets/javascripts/map.js @@ -15,9 +15,9 @@ App.Map.maps = []; }, initializeMap: function(element) { - var addMarkerInvestments, centerData, clearFormfields, createMarker, - editable, markerData, map, mapCenterLatLng, marker, markerIcon, - moveOrPlaceMarker, removeMarker, removeMarkerSelector, updateFormfields; + var addMarkerInvestments, centerData, createMarker, editable, markerData, + map, mapCenterLatLng, marker, markerIcon, moveOrPlaceMarker, + removeMarker, removeMarkerSelector; App.Map.cleanInvestmentCoordinates(element); removeMarkerSelector = $(element).data("marker-remove-selector"); addMarkerInvestments = $(element).data("marker-investments-coordinates"); @@ -37,7 +37,9 @@ draggable: editable }); if (editable) { - marker.on("dragend", updateFormfields); + marker.on("dragend", function() { + App.Map.updateFormfields(map, marker); + }); } marker.addTo(map); return marker; @@ -48,7 +50,7 @@ map.removeLayer(marker); marker = null; } - clearFormfields(); + App.Map.clearFormfields(element); }; moveOrPlaceMarker = function(e) { if (marker) { @@ -56,21 +58,7 @@ } else { marker = createMarker(e.latlng.lat, e.latlng.lng); } - updateFormfields(); - }; - updateFormfields = function() { - var inputs = App.Map.coordinatesInputs(element); - - inputs.lat.val(marker.getLatLng().lat); - inputs.long.val(marker.getLatLng().lng); - inputs.zoom.val(map.getZoom()); - }; - clearFormfields = function() { - var inputs = App.Map.coordinatesInputs(element); - - inputs.lat.val(""); - inputs.long.val(""); - inputs.zoom.val(""); + App.Map.updateFormfields(map, marker); }; centerData = App.Map.centerData(element); @@ -87,7 +75,7 @@ $(removeMarkerSelector).on("click", removeMarker); map.on("zoomend", function() { if (marker) { - updateFormfields(); + App.Map.updateFormfields(map, marker); } }); map.on("click", moveOrPlaceMarker); @@ -164,6 +152,20 @@ zoom: $($(element).data("zoom-input-selector")) }; }, + updateFormfields: function(map, marker) { + var inputs = App.Map.coordinatesInputs(map._container); + + inputs.lat.val(marker.getLatLng().lat); + inputs.long.val(marker.getLatLng().lng); + inputs.zoom.val(map.getZoom()); + }, + clearFormfields: function(element) { + var inputs = App.Map.coordinatesInputs(element); + + inputs.lat.val(""); + inputs.long.val(""); + inputs.zoom.val(""); + }, cleanInvestmentCoordinates: function(element) { var clean_markers, markers; markers = $(element).attr("data-marker-investments-coordinates"); From 74d165ae7a6655debf44e744709b98ad4e9d436b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Mon, 24 Apr 2023 18:05:22 +0200 Subject: [PATCH 16/18] Extract function to create a map --- app/assets/javascripts/map.js | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/app/assets/javascripts/map.js b/app/assets/javascripts/map.js index d84d917c3..06ddd619a 100644 --- a/app/assets/javascripts/map.js +++ b/app/assets/javascripts/map.js @@ -15,9 +15,8 @@ App.Map.maps = []; }, initializeMap: function(element) { - var addMarkerInvestments, centerData, createMarker, editable, markerData, - map, mapCenterLatLng, marker, markerIcon, moveOrPlaceMarker, - removeMarker, removeMarkerSelector; + var addMarkerInvestments, createMarker, editable, markerData, map, marker, + markerIcon, moveOrPlaceMarker, removeMarker, removeMarkerSelector; App.Map.cleanInvestmentCoordinates(element); removeMarkerSelector = $(element).data("marker-remove-selector"); addMarkerInvestments = $(element).data("marker-investments-coordinates"); @@ -61,9 +60,7 @@ App.Map.updateFormfields(map, marker); }; - centerData = App.Map.centerData(element); - mapCenterLatLng = new L.LatLng(centerData.lat, centerData.long); - map = L.map(element.id, { scrollWheelZoom: false }).setView(mapCenterLatLng, centerData.zoom); + map = App.Map.leafletMap(element); App.Map.maps.push(map); App.Map.addAttribution(map); @@ -90,6 +87,14 @@ }); } }, + leafletMap: function(element) { + var centerData, mapCenterLatLng; + + centerData = App.Map.centerData(element); + mapCenterLatLng = new L.LatLng(centerData.lat, centerData.long); + + return L.map(element.id, { scrollWheelZoom: false }).setView(mapCenterLatLng, centerData.zoom); + }, attributionPrefix: function() { return 'Leaflet'; }, From 21ce7689c2d815d189ec6b4b9567a1f5670b19dd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Mon, 24 Apr 2023 17:43:44 +0200 Subject: [PATCH 17/18] Don't overwrite marker when creating investment markers The `marker` variable is like a global variable inside the `initializeMap` function, so assigning it inside the `createMarker` function was changing its value in other places. So we're using different variable names like `newMarker` in order to make the code easier to follow. Now we "only" change the `marker` variable in functions that modify the marker. --- app/assets/javascripts/map.js | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/app/assets/javascripts/map.js b/app/assets/javascripts/map.js index 06ddd619a..3356338f8 100644 --- a/app/assets/javascripts/map.js +++ b/app/assets/javascripts/map.js @@ -29,19 +29,19 @@ html: '
' }); createMarker = function(latitude, longitude) { - var markerLatLng; + var newMarker, markerLatLng; markerLatLng = new L.LatLng(latitude, longitude); - marker = L.marker(markerLatLng, { + newMarker = L.marker(markerLatLng, { icon: markerIcon, draggable: editable }); if (editable) { - marker.on("dragend", function() { - App.Map.updateFormfields(map, marker); + newMarker.on("dragend", function() { + App.Map.updateFormfields(map, newMarker); }); } - marker.addTo(map); - return marker; + newMarker.addTo(map); + return newMarker; }; removeMarker = function(e) { e.preventDefault(); @@ -79,10 +79,12 @@ } if (addMarkerInvestments) { addMarkerInvestments.forEach(function(coordinates) { + var investmentMarker; + if (App.Map.validCoordinates(coordinates)) { - marker = createMarker(coordinates.lat, coordinates.long); - marker.options.id = coordinates.investment_id; - marker.on("click", App.Map.openMarkerPopup); + investmentMarker = createMarker(coordinates.lat, coordinates.long); + investmentMarker.options.id = coordinates.investment_id; + investmentMarker.on("click", App.Map.openMarkerPopup); } }); } From b0680628bafaa3992784dbaddc55c296884c02df Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Mon, 24 Apr 2023 17:44:39 +0200 Subject: [PATCH 18/18] Extract function to add investment markers --- app/assets/javascripts/map.js | 30 +++++++++++++++++------------- 1 file changed, 17 insertions(+), 13 deletions(-) diff --git a/app/assets/javascripts/map.js b/app/assets/javascripts/map.js index 3356338f8..2058a0791 100644 --- a/app/assets/javascripts/map.js +++ b/app/assets/javascripts/map.js @@ -15,11 +15,11 @@ App.Map.maps = []; }, initializeMap: function(element) { - var addMarkerInvestments, createMarker, editable, markerData, map, marker, + var createMarker, editable, investmentsMarkers, markerData, map, marker, markerIcon, moveOrPlaceMarker, removeMarker, removeMarkerSelector; App.Map.cleanInvestmentCoordinates(element); removeMarkerSelector = $(element).data("marker-remove-selector"); - addMarkerInvestments = $(element).data("marker-investments-coordinates"); + investmentsMarkers = $(element).data("marker-investments-coordinates"); editable = $(element).data("marker-editable"); marker = null; markerIcon = L.divIcon({ @@ -65,7 +65,7 @@ App.Map.addAttribution(map); markerData = App.Map.markerData(element); - if (markerData.lat && markerData.long && !addMarkerInvestments) { + if (markerData.lat && markerData.long && !investmentsMarkers) { marker = createMarker(markerData.lat, markerData.long); } if (editable) { @@ -77,17 +77,8 @@ }); map.on("click", moveOrPlaceMarker); } - if (addMarkerInvestments) { - addMarkerInvestments.forEach(function(coordinates) { - var investmentMarker; - if (App.Map.validCoordinates(coordinates)) { - investmentMarker = createMarker(coordinates.lat, coordinates.long); - investmentMarker.options.id = coordinates.investment_id; - investmentMarker.on("click", App.Map.openMarkerPopup); - } - }); - } + App.Map.addInvestmentsMarkers(investmentsMarkers, createMarker); }, leafletMap: function(element) { var centerData, mapCenterLatLng; @@ -173,6 +164,19 @@ inputs.long.val(""); inputs.zoom.val(""); }, + addInvestmentsMarkers: function(markers, createMarker) { + if (markers) { + markers.forEach(function(coordinates) { + var marker; + + if (App.Map.validCoordinates(coordinates)) { + marker = createMarker(coordinates.lat, coordinates.long); + marker.options.id = coordinates.investment_id; + marker.on("click", App.Map.openMarkerPopup); + } + }); + } + }, cleanInvestmentCoordinates: function(element) { var clean_markers, markers; markers = $(element).attr("data-marker-investments-coordinates");