Merge pull request #5283 from consuldemocracy/label_links

Allow links in forms to open in new tabs
2023-10-24 16:30:59 +02:00
parent bc1f303551 c2710de5fc
commit 2e217a647a
28 changed files with 95 additions and 58 deletions
--- a/app/components/admin/site_customization/pages/index_component.html.erb
+++ b/app/components/admin/site_customization/pages/index_component.html.erb
@@ -34,7 +34,7 @@
              <%= actions.action(:show,
                                 text: t("admin.site_customization.pages.index.see_page"),
                                 path: page.url,
-                                 options: { target: "_blank" }) %>
+                                 target: "_blank") %>
            <% end %>
          <% end %>
        </td>
--- a/app/components/budgets/investments/form_component.html.erb
+++ b/app/components/budgets/investments/form_component.html.erb
@@ -89,11 +89,7 @@
  <div class="actions">
    <% unless current_user.manager? || investment.persisted? %>
      <div>
-        <%= f.check_box :terms_of_service,
-                        title: t("form.accept_terms_title"),
-                        label: t("form.accept_terms",
-                                 policy: link_to(t("form.policy"), "/privacy", target: "blank"),
-                                 conditions: link_to(t("form.conditions"), "/conditions", target: "blank")) %>
+        <%= render Shared::AgreeWithTermsOfServiceFieldComponent.new(f) %>
      </div>
    <% end %>

--- a/app/components/debates/form_component.html.erb
+++ b/app/components/debates/form_component.html.erb
@@ -41,11 +41,7 @@
  <div class="actions">
    <% if debate.new_record? %>
      <div>
-        <%= f.check_box :terms_of_service,
-                        title: t("form.accept_terms_title"),
-                        label: t("form.accept_terms",
-                                 policy: link_to(t("form.policy"), "/privacy", target: "blank"),
-                                 conditions: link_to(t("form.conditions"), "/conditions", target: "blank")) %>
+        <%= render Shared::AgreeWithTermsOfServiceFieldComponent.new(f) %>
      </div>
    <% end %>

--- a/app/components/debates/new_component.html.erb
+++ b/app/components/debates/new_component.html.erb
@@ -2,9 +2,7 @@
  <%= back_link_to debates_path, t("debates.index.section_header.title") %>

  <%= header do %>
-    <%= link_to help_path(anchor: "debates"), title: t("shared.target_blank"), target: "_blank" do %>
-      <%= t("debates.new.more_info") %>
-    <% end %>
+    <%= link_to t("debates.new.more_info"), help_path(anchor: "debates") %>
  <% end %>

  <aside>
--- a/app/components/debates/new_component.rb
+++ b/app/components/debates/new_component.rb
@@ -1,6 +1,7 @@
 class Debates::NewComponent < ApplicationComponent
  include Header
  attr_reader :debate
+  delegate :new_window_link_to, to: :helpers

  def initialize(debate)
    @debate = debate
--- a/app/components/layout/footer_component.html.erb
+++ b/app/components/layout/footer_component.html.erb
@@ -7,8 +7,8 @@

      <p class="info">
        <%= sanitize(t("layouts.footer.description",
-                       open_source: link_to(t("layouts.footer.open_source"), t("layouts.footer.open_source_url"), target: "blank", rel: "nofollow"),
-                       consul: link_to(t("layouts.footer.consul"), t("layouts.footer.consul_url"), target: "blank", rel: "nofollow"))) %>
+                       open_source: link_to(t("layouts.footer.open_source"), t("layouts.footer.open_source_url"), target: "_blank", rel: "nofollow"),
+                       consul: link_to(t("layouts.footer.consul"), t("layouts.footer.consul_url"), target: "_blank", rel: "nofollow"))) %>
      </p>
    </div>

--- a/app/components/proposals/form_component.html.erb
+++ b/app/components/proposals/form_component.html.erb
@@ -96,11 +96,7 @@
  <div class="actions">
    <% if proposal.new_record? %>
      <div>
-        <%= f.check_box :terms_of_service,
-                        title: t("form.accept_terms_title"),
-                        label: t("form.accept_terms",
-                                 policy: link_to(t("form.policy"), "/privacy", target: "blank"),
-                                 conditions: link_to(t("form.conditions"), "/conditions", target: "blank")) %>
+        <%= render Shared::AgreeWithTermsOfServiceFieldComponent.new(f) %>
      </div>
    <% end %>

--- a/app/components/proposals/new_component.html.erb
+++ b/app/components/proposals/new_component.html.erb
@@ -2,9 +2,7 @@
  <%= back_link_to proposals_path, t("proposals.index.section_header.title") %>

  <%= header do %>
-    <%= link_to help_path(anchor: "proposals"), title: t("shared.target_blank"), target: "_blank" do %>
-      <%= t("proposals.new.more_info") %>
-    <% end %>
+    <%= new_window_link_to t("proposals.new.more_info"), help_path(anchor: "proposals") %>
  <% end %>

  <aside>
--- a/app/components/proposals/new_component.rb
+++ b/app/components/proposals/new_component.rb
@@ -1,6 +1,7 @@
 class Proposals::NewComponent < ApplicationComponent
  include Header
  attr_reader :proposal
+  delegate :new_window_link_to, to: :helpers

  def initialize(proposal)
    @proposal = proposal
--- a/app/components/sdg/related_list_selector_component.html.erb
+++ b/app/components/sdg/related_list_selector_component.html.erb
@@ -14,10 +14,8 @@
    <%= f.text_field :related_sdg_list,
                     class: "input",
                     hint: sanitize(t("sdg.related_list_selector.hint",
-                                      link: link_to(t("sdg.related_list_selector.help.text"),
-                                                    sdg_help_path,
-                                                    title: t("shared.target_blank"),
-                                                    target: "_blank")),
+                                      link: new_window_link_to(t("sdg.related_list_selector.help.text"),
+                                                               sdg_help_path)),
                                    attributes: %w[href title target]),
                     data: { "suggestions-list": sdg_related_suggestions,
                             "remove-tag-text": t("sdg.related_list_selector.remove_tag") } %>
--- a/app/components/sdg/related_list_selector_component.rb
+++ b/app/components/sdg/related_list_selector_component.rb
@@ -1,5 +1,6 @@
 class SDG::RelatedListSelectorComponent < ApplicationComponent
  attr_reader :f
+  delegate :new_window_link_to, to: :helpers

  def initialize(form)
    @f = form
--- a/app/components/shared/agree_with_terms_of_service_field_component.html.erb
+++ b/app/components/shared/agree_with_terms_of_service_field_component.html.erb
@@ -0,0 +1 @@
+<%= form.check_box :terms_of_service, label: label %>
--- a/app/components/shared/agree_with_terms_of_service_field_component.rb
+++ b/app/components/shared/agree_with_terms_of_service_field_component.rb
@@ -0,0 +1,16 @@
+class Shared::AgreeWithTermsOfServiceFieldComponent < ApplicationComponent
+  attr_reader :form
+  delegate :new_window_link_to, to: :helpers
+
+  def initialize(form)
+    @form = form
+  end
+
+  private
+
+    def label
+      t("form.accept_terms",
+        policy: new_window_link_to(t("form.policy"), "/privacy"),
+        conditions: new_window_link_to(t("form.conditions"), "/conditions"))
+    end
+end
--- a/app/helpers/application_helper.rb
+++ b/app/helpers/application_helper.rb
@@ -30,6 +30,10 @@ module ApplicationHelper
    end
  end

+  def new_window_link_to(text, path, **options)
+    link_to text, path, { target: "_blank", title: t("shared.target_blank") }.merge(options)
+  end
+
  def image_path_for(filename)
    image = SiteCustomization::Image.image_for(filename)

--- a/app/views/legislation/proposals/_form.html.erb
+++ b/app/views/legislation/proposals/_form.html.erb
@@ -62,11 +62,7 @@

    <div class="small-12 column">
      <% if @proposal.new_record? %>
-        <%= f.check_box :terms_of_service,
-                        title: t("form.accept_terms_title"),
-                        label: t("form.accept_terms",
-                                 policy: link_to(t("form.policy"), "/privacy", target: "blank"),
-                                 conditions: link_to(t("form.conditions"), "/conditions", target: "blank")) %>
+        <%= render Shared::AgreeWithTermsOfServiceFieldComponent.new(f) %>
      <% end %>
    </div>

--- a/app/views/organizations/registrations/new.html.erb
+++ b/app/views/organizations/registrations/new.html.erb
@@ -25,12 +25,9 @@
                                                   label: t("devise_views.organizations.registrations.new.password_confirmation_label") %>

      <%= f.check_box :terms_of_service,
-                      title: t("devise_views.users.registrations.new.terms_title"),
                      label: t("devise_views.users.registrations.new.terms",
-                               terms: link_to(t("devise_views.users.registrations.new.terms_link"),
-                                              "/conditions",
-                                              title: t("shared.target_blank"),
-                                              target: "_blank")) %>
+                               terms: new_window_link_to(t("devise_views.users.registrations.new.terms_link"),
+                                                         "/conditions")) %>

      <div class="small-12 medium-6 small-centered">
        <%= f.submit t("devise_views.organizations.registrations.new.submit"), class: "button expanded" %>
--- a/app/views/users/registrations/new.html.erb
+++ b/app/views/users/registrations/new.html.erb
@@ -35,12 +35,9 @@
      <% end %>

      <%= f.check_box :terms_of_service,
-                      title: t("devise_views.users.registrations.new.terms_title"),
                      label: t("devise_views.users.registrations.new.terms",
-                               terms: link_to(t("devise_views.users.registrations.new.terms_link"),
-                                              "/conditions",
-                                              title: t("shared.target_blank"),
-                                              target: "_blank")) %>
+                               terms: new_window_link_to(t("devise_views.users.registrations.new.terms_link"),
+                                                         "/conditions")) %>

      <div class="small-12 medium-6 small-centered">
        <%= f.submit t("devise_views.users.registrations.new.submit"), class: "button expanded" %>
--- a/app/views/verification/residence/new.html.erb
+++ b/app/views/verification/residence/new.html.erb
@@ -69,12 +69,9 @@

      <div class="small-12">
        <%= f.check_box :terms_of_service,
-                        title: t("verification.residence.new.accept_terms_text_title"),
                        label: t("verification.residence.new.accept_terms_text",
-                                 terms_url: link_to(t("verification.residence.new.terms"),
-                                                    page_path("census_terms"),
-                                                    title: t("shared.target_blank"),
-                                                    target: "_blank")) %>
+                                 terms_url: new_window_link_to(t("verification.residence.new.terms"),
+                                                               page_path("census_terms"))) %>
      </div>

      <div class="small-12 medium-3 clear">
--- a/config/locales/en/devise_views.yml
+++ b/config/locales/en/devise_views.yml
@@ -103,7 +103,6 @@ en:
          submit: Register
          terms: By registering you accept the %{terms}
          terms_link: terms and conditions of use
-          terms_title: By registering you accept the terms and conditions of use
          title: Register
          username_is_available: Username available
          username_is_not_available: Username already in use
--- a/config/locales/en/general.yml
+++ b/config/locales/en/general.yml
@@ -143,7 +143,6 @@ en:
        poll_ended: "Cannot be changed if voting has already ended"
  form:
    accept_terms: I agree to the %{policy} and the %{conditions}
-    accept_terms_title: I agree to the Privacy Policy and the Terms and conditions of use
    conditions: Terms and conditions of use
    debate: Debate
    direct_message: private message
--- a/config/locales/en/verification.yml
+++ b/config/locales/en/verification.yml
@@ -46,7 +46,6 @@ en:
          success: Residence verified
      new:
        accept_terms_text: I accept %{terms_url} of the Census
-        accept_terms_text_title: I accept the terms and conditions of access of the Census
        document_number: Document number
        document_number_help_title: Help
        document_number_help_text: "<strong>DNI</strong>: 12345678A<br> <strong>Passport</strong>: AAA000001<br> <strong>Residence card</strong>: X1234567P"
--- a/config/locales/es/devise_views.yml
+++ b/config/locales/es/devise_views.yml
@@ -103,7 +103,6 @@ es:
          submit: Registrarse
          terms: Al registrarte aceptas las %{terms}
          terms_link: condiciones de uso
-          terms_title: Al registrarte aceptas las condiciones de uso
          title: Registrarse
          username_is_available: Nombre de usuario disponible
          username_is_not_available: Nombre de usuario ya existente
--- a/config/locales/es/general.yml
+++ b/config/locales/es/general.yml
@@ -143,7 +143,6 @@ es:
        poll_ended: "No puede ser cambiada si la votación ha acabado"
  form:
    accept_terms: Acepto la %{policy} y las %{conditions}
-    accept_terms_title: Acepto la Política de privacidad y las Condiciones de uso
    conditions: Condiciones de uso
    debate: el debate
    direct_message: el mensaje privado
--- a/config/locales/es/verification.yml
+++ b/config/locales/es/verification.yml
@@ -46,7 +46,6 @@ es:
          success: Residencia verificada
      new:
        accept_terms_text: Acepto %{terms_url} al Padrón
-        accept_terms_text_title: Acepto los términos de acceso al Padrón
        document_number: Número de documento
        document_number_help_title: Ayuda
        document_number_help_text: "<strong>DNI</strong>: 12345678A<br> <strong>Pasaporte</strong>: AAA000001<br> <strong>Tarjeta de residencia</strong>: X1234567P"
--- a/lib/consul_form_builder.rb
+++ b/lib/consul_form_builder.rb
@@ -23,7 +23,7 @@ class ConsulFormBuilder < FoundationRailsHelper::FormBuilder
    if options[:label] == false
      super
    else
-      label = tag.span sanitize(label_text(attribute, options[:label])), class: "checkbox"
+      label = tag.span sanitized_label_text(attribute, options[:label]), class: "checkbox"

      super(attribute, options.merge(
        label: label,
@@ -51,7 +51,7 @@ class ConsulFormBuilder < FoundationRailsHelper::FormBuilder
      if text == false
        super
      else
-        super(attribute, sanitize(label_text(attribute, text)), options)
+        super(attribute, sanitized_label_text(attribute, text), options)
      end
    end

@@ -68,6 +68,14 @@ class ConsulFormBuilder < FoundationRailsHelper::FormBuilder
      end
    end

+    def sanitized_label_text(attribute, text)
+      sanitize(label_text(attribute, text), attributes: allowed_attributes)
+    end
+
+    def allowed_attributes
+      self.class.sanitized_allowed_attributes + ["target"]
+    end
+
    def label_options_for(options)
      label_options = options[:label_options] || {}

--- a/spec/components/shared/agree_with_terms_of_service_field_component_spec.rb
+++ b/spec/components/shared/agree_with_terms_of_service_field_component_spec.rb
@@ -0,0 +1,30 @@
+require "rails_helper"
+
+describe Shared::AgreeWithTermsOfServiceFieldComponent do
+  before do
+    dummy_model = Class.new do
+      include ActiveModel::Model
+      attr_accessor :terms_of_service
+    end
+
+    stub_const("DummyModel", dummy_model)
+  end
+
+  let(:form) { ConsulFormBuilder.new(:dummy, DummyModel.new, ApplicationController.new.view_context, {}) }
+  let(:component) { Shared::AgreeWithTermsOfServiceFieldComponent.new(form) }
+
+  it "contains links that open in a new window" do
+    render_inline component
+
+    expect(page).to have_css "a[target=_blank]", count: 2
+  end
+
+  it "contains links indicating they open in a new window" do
+    render_inline component
+
+    expect(page).to have_link count: 2
+    expect(page).to have_link "Privacy Policy"
+    expect(page).to have_link "Terms and conditions of use"
+    expect(page).to have_link " (link opens in new window)", count: 2
+  end
+end
--- a/spec/lib/consul_form_builder_spec.rb
+++ b/spec/lib/consul_form_builder_spec.rb
@@ -13,6 +13,17 @@ describe ConsulFormBuilder do

  let(:builder) { ConsulFormBuilder.new(:dummy, DummyModel.new, ApplicationController.new.view_context, {}) }

+  describe "label" do
+    it "accepts links that open in a new window in its content" do
+      render builder.text_field(:title, label: 'My <a href="/" target="_blank" title="New tab">title</a>')
+
+      expect(page).to have_link count: 1
+      expect(page).to have_link "title"
+      expect(page).to have_link "New tab"
+      expect(page).to have_css "a[target=_blank]"
+    end
+  end
+
  describe "hints" do
    it "does not generate hints by default" do
      render builder.text_field(:title)
--- a/spec/system/verification/residence_spec.rb
+++ b/spec/system/verification/residence_spec.rb
@@ -164,8 +164,9 @@ describe "Residence" do
    login_as(create(:user))

    visit new_residence_path
-    click_link "the terms and conditions of access"

-    expect(page).to have_content "Terms and conditions of access of the Census"
+    within_window(window_opened_by { click_link "the terms and conditions of access" }) do
+      expect(page).to have_content "Terms and conditions of access of the Census"
+    end
  end
 end
				`@@ -0,0 +1 @@`
				`<%= form.check_box :terms_of_service, label: label %>`