From 7c4515d6ce6607428e82a987e9b117266345beab Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Tue, 2 Feb 2021 01:08:10 +0100 Subject: [PATCH 1/7] Simplify code to generate "See more" link We can skip the `link_to(*options) if options` part if we accept anchor tags in the `link_list` helper. --- app/components/concerns/sdg/tag_list.rb | 9 ++++----- .../sdg/goals/plain_tag_list_component.rb | 8 +------- .../sdg/targets/plain_tag_list_component.rb | 8 +------- app/helpers/link_list_helper.rb | 10 +++++++--- spec/helpers/link_list_helper_spec.rb | 19 ++++++++++++++++++- 5 files changed, 31 insertions(+), 23 deletions(-) diff --git a/app/components/concerns/sdg/tag_list.rb b/app/components/concerns/sdg/tag_list.rb index a27225370..355005408 100644 --- a/app/components/concerns/sdg/tag_list.rb +++ b/app/components/concerns/sdg/tag_list.rb @@ -16,11 +16,10 @@ module SDG::TagList count = count_out_of_limit(collection) if count > 0 - [ - "#{count}+", - polymorphic_path(record), - class: "more-#{i18n_namespace}", title: t("sdg.#{i18n_namespace}.filter.more", count: count) - ] + link_to "#{count}+", + polymorphic_path(record), + class: "more-#{i18n_namespace}", + title: t("sdg.#{i18n_namespace}.filter.more", count: count) end end diff --git a/app/components/sdg/goals/plain_tag_list_component.rb b/app/components/sdg/goals/plain_tag_list_component.rb index 8b60b692b..bb1bb6d0a 100644 --- a/app/components/sdg/goals/plain_tag_list_component.rb +++ b/app/components/sdg/goals/plain_tag_list_component.rb @@ -8,13 +8,7 @@ class SDG::Goals::PlainTagListComponent < ApplicationComponent end def tags - [*goal_tags, see_more_link].compact - end - - def see_more_link - options = super(goals) - - link_to(*options) if options.present? + [*goal_tags, see_more_link(goals)].compact end def goal_tags diff --git a/app/components/sdg/targets/plain_tag_list_component.rb b/app/components/sdg/targets/plain_tag_list_component.rb index b24792cb2..73d73ed2a 100644 --- a/app/components/sdg/targets/plain_tag_list_component.rb +++ b/app/components/sdg/targets/plain_tag_list_component.rb @@ -8,13 +8,7 @@ class SDG::Targets::PlainTagListComponent < ApplicationComponent end def tags - [*target_tags, see_more_link].compact - end - - def see_more_link - options = super(targets) - - link_to(*options) if options.present? + [*target_tags, see_more_link(targets)].compact end def target_tags diff --git a/app/helpers/link_list_helper.rb b/app/helpers/link_list_helper.rb index f3e00d1bb..5af374fae 100644 --- a/app/helpers/link_list_helper.rb +++ b/app/helpers/link_list_helper.rb @@ -1,11 +1,15 @@ module LinkListHelper def link_list(*links, **options) - return "" if links.compact.empty? + return "" if links.select(&:present?).empty? tag.ul(options) do - safe_join(links.compact.map do |text, url, current = false, **link_options| + safe_join(links.select(&:present?).map do |text, url, current = false, **link_options| tag.li(({ "aria-current": true } if current)) do - link_to text, url, link_options + if url + link_to text, url, link_options + else + text + end end end, "\n") end diff --git a/spec/helpers/link_list_helper_spec.rb b/spec/helpers/link_list_helper_spec.rb index 31ff73768..7626d0198 100644 --- a/spec/helpers/link_list_helper_spec.rb +++ b/spec/helpers/link_list_helper_spec.rb @@ -19,6 +19,15 @@ describe LinkListHelper do expect(list).to be_html_safe end + it "accepts anchor tags" do + list = helper.link_list(link_to("Home", "/"), ["Info", "/info"], class: "menu") + + expect(list).to eq '' + expect(list).to be_html_safe + end + it "accepts options for links" do render helper.link_list(["Home", "/", class: "root"], ["Info", "/info", id: "info"]) @@ -30,7 +39,15 @@ describe LinkListHelper do it "ignores nil entries" do render helper.link_list(["Home", "/", class: "root"], nil, ["Info", "/info", id: "info"]) - expect(page).to have_css "a", count: 2 + expect(page).to have_css "li", count: 2 + expect(page).to have_css "a.root", count: 1, exact_text: "Home" + expect(page).to have_css "a#info", count: 1, exact_text: "Info" + end + + it "ignores empty entries" do + render helper.link_list(["Home", "/", class: "root"], "", ["Info", "/info", id: "info"]) + + expect(page).to have_css "li", count: 2 expect(page).to have_css "a.root", count: 1, exact_text: "Home" expect(page).to have_css "a#info", count: 1, exact_text: "Info" end From 6112016773e2f7844690520b8f7ae9c2290a787b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Tue, 2 Feb 2021 01:36:06 +0100 Subject: [PATCH 2/7] Extract component to render a list of tags --- .../shared/tag_list_component.html.erb | 1 + app/components/shared/tag_list_component.rb | 40 +++++++++++++++++++ app/helpers/tags_helper.rb | 10 ----- app/views/shared/_tags.html.erb | 18 +-------- 4 files changed, 42 insertions(+), 27 deletions(-) create mode 100644 app/components/shared/tag_list_component.html.erb create mode 100644 app/components/shared/tag_list_component.rb delete mode 100644 app/helpers/tags_helper.rb diff --git a/app/components/shared/tag_list_component.html.erb b/app/components/shared/tag_list_component.html.erb new file mode 100644 index 000000000..217f9855c --- /dev/null +++ b/app/components/shared/tag_list_component.html.erb @@ -0,0 +1 @@ +<%= link_list(*links, class: "tags", id: "tags_#{dom_id(taggable)}") %> diff --git a/app/components/shared/tag_list_component.rb b/app/components/shared/tag_list_component.rb new file mode 100644 index 000000000..0c4bf4f91 --- /dev/null +++ b/app/components/shared/tag_list_component.rb @@ -0,0 +1,40 @@ +class Shared::TagListComponent < ApplicationComponent + attr_reader :taggable, :limit + delegate :link_list, to: :helpers + + def initialize(taggable, limit:) + @taggable = taggable + @limit = limit + end + + private + + def links + [*tag_links, see_more_link] + end + + def tag_links + taggable.tag_list_with_limit(limit).map do |tag| + [ + sanitize(tag.name), + taggables_path(taggable, tag.name) + ] + end + end + + def see_more_link + if taggable.tags_count_out_of_limit(limit) > 0 + link_to "#{taggable.tags_count_out_of_limit(limit)}+", + polymorphic_path(taggable) + end + end + + def taggables_path(taggable, tag_name) + case taggable.class.name + when "Legislation::Proposal" + legislation_process_proposals_path(taggable.process, search: tag_name) + else + polymorphic_path(taggable.class, search: tag_name) + end + end +end diff --git a/app/helpers/tags_helper.rb b/app/helpers/tags_helper.rb deleted file mode 100644 index 464a155d0..000000000 --- a/app/helpers/tags_helper.rb +++ /dev/null @@ -1,10 +0,0 @@ -module TagsHelper - def taggables_path(taggable, tag_name) - case taggable.class.name - when "Legislation::Proposal" - legislation_process_proposals_path(taggable.process, search: tag_name) - else - polymorphic_path(taggable.class, search: tag_name) - end - end -end diff --git a/app/views/shared/_tags.html.erb b/app/views/shared/_tags.html.erb index f4ac1773e..bb868393e 100644 --- a/app/views/shared/_tags.html.erb +++ b/app/views/shared/_tags.html.erb @@ -1,20 +1,4 @@ <%- limit ||= nil %> <%= render SDG::TagListComponent.new(taggable, limit: limit) %> - -<% if taggable.tags.any? %> - -<% end %> +<%= render Shared::TagListComponent.new(taggable, limit: limit) %> From ad56b01a9a91cea0c0c83fb2f261c4c020440296 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Tue, 2 Feb 2021 01:43:21 +0100 Subject: [PATCH 3/7] Extract component to render "see more" link --- app/components/concerns/sdg/tag_list.rb | 17 +-------- .../sdg/goals/plain_tag_list_component.rb | 2 +- .../sdg/goals/tag_list_component.rb | 2 +- .../sdg/targets/plain_tag_list_component.rb | 2 +- .../sdg/targets/tag_list_component.rb | 2 +- .../shared/see_more_link_component.html.erb | 3 ++ .../shared/see_more_link_component.rb | 37 +++++++++++++++++++ app/components/shared/tag_list_component.rb | 5 +-- app/models/concerns/taggable.rb | 7 ---- config/i18n-tasks.yml | 1 + config/locales/en/general.yml | 5 +++ config/locales/es/general.yml | 5 +++ 12 files changed, 58 insertions(+), 30 deletions(-) create mode 100644 app/components/shared/see_more_link_component.html.erb create mode 100644 app/components/shared/see_more_link_component.rb diff --git a/app/components/concerns/sdg/tag_list.rb b/app/components/concerns/sdg/tag_list.rb index 355005408..b936853d5 100644 --- a/app/components/concerns/sdg/tag_list.rb +++ b/app/components/concerns/sdg/tag_list.rb @@ -12,15 +12,8 @@ module SDG::TagList process.enabled? end - def see_more_link(collection) - count = count_out_of_limit(collection) - - if count > 0 - link_to "#{count}+", - polymorphic_path(record), - class: "more-#{i18n_namespace}", - title: t("sdg.#{i18n_namespace}.filter.more", count: count) - end + def see_more_link(association_name) + render Shared::SeeMoreLinkComponent.new(record, association_name, limit: limit) end def filter_text(goal_or_target) @@ -33,12 +26,6 @@ module SDG::TagList polymorphic_path(model, advanced_search: advanced_search) end - def count_out_of_limit(collection) - return 0 unless limit - - collection.size - limit - end - def process @process ||= SDG::ProcessEnabled.new(record_or_name) end diff --git a/app/components/sdg/goals/plain_tag_list_component.rb b/app/components/sdg/goals/plain_tag_list_component.rb index bb1bb6d0a..3d9a701e0 100644 --- a/app/components/sdg/goals/plain_tag_list_component.rb +++ b/app/components/sdg/goals/plain_tag_list_component.rb @@ -8,7 +8,7 @@ class SDG::Goals::PlainTagListComponent < ApplicationComponent end def tags - [*goal_tags, see_more_link(goals)].compact + [*goal_tags, see_more_link(:sdg_goals)].select(&:present?) end def goal_tags diff --git a/app/components/sdg/goals/tag_list_component.rb b/app/components/sdg/goals/tag_list_component.rb index 391e07f55..bc23ca456 100644 --- a/app/components/sdg/goals/tag_list_component.rb +++ b/app/components/sdg/goals/tag_list_component.rb @@ -8,7 +8,7 @@ class SDG::Goals::TagListComponent < ApplicationComponent end def links - [*goal_links, see_more_link(goals)] + [*goal_links, see_more_link(:sdg_goals)] end def goal_links diff --git a/app/components/sdg/targets/plain_tag_list_component.rb b/app/components/sdg/targets/plain_tag_list_component.rb index 73d73ed2a..2e1ba705b 100644 --- a/app/components/sdg/targets/plain_tag_list_component.rb +++ b/app/components/sdg/targets/plain_tag_list_component.rb @@ -8,7 +8,7 @@ class SDG::Targets::PlainTagListComponent < ApplicationComponent end def tags - [*target_tags, see_more_link(targets)].compact + [*target_tags, see_more_link(:sdg_targets)].select(&:present?) end def target_tags diff --git a/app/components/sdg/targets/tag_list_component.rb b/app/components/sdg/targets/tag_list_component.rb index cb79f80b6..7d340b2aa 100644 --- a/app/components/sdg/targets/tag_list_component.rb +++ b/app/components/sdg/targets/tag_list_component.rb @@ -8,7 +8,7 @@ class SDG::Targets::TagListComponent < ApplicationComponent end def links - [*target_links, see_more_link(targets)] + [*target_links, see_more_link(:sdg_targets)] end def target_links diff --git a/app/components/shared/see_more_link_component.html.erb b/app/components/shared/see_more_link_component.html.erb new file mode 100644 index 000000000..dd78b17a5 --- /dev/null +++ b/app/components/shared/see_more_link_component.html.erb @@ -0,0 +1,3 @@ +<% if count_out_of_limit > 0 %> + <%= link_to text, url, class: html_class, title: title %> +<% end %> diff --git a/app/components/shared/see_more_link_component.rb b/app/components/shared/see_more_link_component.rb new file mode 100644 index 000000000..339f66384 --- /dev/null +++ b/app/components/shared/see_more_link_component.rb @@ -0,0 +1,37 @@ +class Shared::SeeMoreLinkComponent < ApplicationComponent + attr_reader :record, :association_name, :limit + + def initialize(record, association_name, limit: nil) + @record = record + @association_name = association_name + @limit = limit + end + + private + + def text + "#{count_out_of_limit}+" + end + + def url + polymorphic_path(record) + end + + def title + t("#{i18n_namespace}.filter.more", count: count_out_of_limit) + end + + def count_out_of_limit + return 0 unless limit + + record.send(association_name).size - limit + end + + def i18n_namespace + association_name.to_s.tr("_", ".") + end + + def html_class + "more-#{i18n_namespace.split(".").last}" + end +end diff --git a/app/components/shared/tag_list_component.rb b/app/components/shared/tag_list_component.rb index 0c4bf4f91..79510718e 100644 --- a/app/components/shared/tag_list_component.rb +++ b/app/components/shared/tag_list_component.rb @@ -23,10 +23,7 @@ class Shared::TagListComponent < ApplicationComponent end def see_more_link - if taggable.tags_count_out_of_limit(limit) > 0 - link_to "#{taggable.tags_count_out_of_limit(limit)}+", - polymorphic_path(taggable) - end + render Shared::SeeMoreLinkComponent.new(taggable, :tags, limit: limit) end def taggables_path(taggable, tag_name) diff --git a/app/models/concerns/taggable.rb b/app/models/concerns/taggable.rb index c840dd97e..73889ae81 100644 --- a/app/models/concerns/taggable.rb +++ b/app/models/concerns/taggable.rb @@ -12,13 +12,6 @@ module Taggable tags.sort { |a, b| b.taggings_count <=> a.taggings_count }[0, limit] end - def tags_count_out_of_limit(limit = nil) - return 0 unless limit - - count = tags.size - limit - count < 0 ? 0 : count - end - def max_number_of_tags errors.add(:tag_list, :less_than_or_equal_to, count: 6) if tag_list.count > 6 end diff --git a/config/i18n-tasks.yml b/config/i18n-tasks.yml index 37a8abda1..dd76cacec 100644 --- a/config/i18n-tasks.yml +++ b/config/i18n-tasks.yml @@ -221,6 +221,7 @@ ignore_unused: - "sdg.goals.goal_*" - "sdg.*.filter.more.*" - "sdg_management.relations.index.filter*" + - "tags.filter.more.*" #### ## Exclude these keys from the `i18n-tasks eq-base" report: # ignore_eq_base: diff --git a/config/locales/en/general.yml b/config/locales/en/general.yml index a150efd5a..3985effb9 100644 --- a/config/locales/en/general.yml +++ b/config/locales/en/general.yml @@ -973,3 +973,8 @@ en: create: enqueue_remote_translation: Translations have been correctly requested. button: Translate page + tags: + filter: + more: + one: "One more tag" + other: "%{count} more tags" diff --git a/config/locales/es/general.yml b/config/locales/es/general.yml index c9e011657..9b0e15134 100644 --- a/config/locales/es/general.yml +++ b/config/locales/es/general.yml @@ -973,3 +973,8 @@ es: create: enqueue_remote_translation: Se han solicitado correctamente las traducciones. button: Traducir página + tags: + filter: + more: + one: "Una etiqueta más" + other: "%{count} etiquetas más" From 999d1a2cfd08bc0a0fdaca0bd3d1a3ae1f171395 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Tue, 2 Feb 2021 17:49:51 +0100 Subject: [PATCH 4/7] Rename goals FilterLinks component to TagCloud This is more consistent with the "tag_cloud" partial name. --- .../sdg/goals/{filter_links.scss => tag_cloud.scss} | 2 +- ...inks_component.html.erb => tag_cloud_component.html.erb} | 2 +- .../{filter_links_component.rb => tag_cloud_component.rb} | 2 +- app/views/shared/_tag_cloud.html.erb | 2 +- ..._links_component_spec.rb => tag_cloud_component_spec.rb} | 6 +++--- 5 files changed, 7 insertions(+), 7 deletions(-) rename app/assets/stylesheets/sdg/goals/{filter_links.scss => tag_cloud.scss} (69%) rename app/components/sdg/goals/{filter_links_component.html.erb => tag_cloud_component.html.erb} (81%) rename app/components/sdg/goals/{filter_links_component.rb => tag_cloud_component.rb} (79%) rename spec/components/sdg/goals/{filter_links_component_spec.rb => tag_cloud_component_spec.rb} (67%) diff --git a/app/assets/stylesheets/sdg/goals/filter_links.scss b/app/assets/stylesheets/sdg/goals/tag_cloud.scss similarity index 69% rename from app/assets/stylesheets/sdg/goals/filter_links.scss rename to app/assets/stylesheets/sdg/goals/tag_cloud.scss index 914c4e7b0..8008cfb64 100644 --- a/app/assets/stylesheets/sdg/goals/filter_links.scss +++ b/app/assets/stylesheets/sdg/goals/tag_cloud.scss @@ -1,4 +1,4 @@ -.sdg-goal-filter-links { +.sdg-goal-tag-cloud { .sdg-goal-tag-list { @extend %sdg-goal-list; diff --git a/app/components/sdg/goals/filter_links_component.html.erb b/app/components/sdg/goals/tag_cloud_component.html.erb similarity index 81% rename from app/components/sdg/goals/filter_links_component.html.erb rename to app/components/sdg/goals/tag_cloud_component.html.erb index dc4b3c122..ebd572a6f 100644 --- a/app/components/sdg/goals/filter_links_component.html.erb +++ b/app/components/sdg/goals/tag_cloud_component.html.erb @@ -1,4 +1,4 @@ -