From 1f637867a402ed4a9cd354e5e2b9276508e8e56a Mon Sep 17 00:00:00 2001 From: evazion Date: Sat, 30 Jan 2021 14:01:07 -0600 Subject: [PATCH] tags: refactor tag lists to use ViewComponent. --- app/components/tag_list_component.rb | 51 ++++++++++ .../tag_list_component.html+categorized.erb | 30 ++++++ .../tag_list_component.html+inline.erb | 7 ++ .../tag_list_component.html+search.erb | 22 +++++ .../tag_list_component.html+simple.erb | 5 + .../tag_list_component.html.scss | 25 +++++ app/helpers/components_helper.rb | 28 ++++++ .../src/styles/common/tags.scss.erb | 15 --- .../src/styles/specific/mod_queue.scss.erb | 2 +- app/logical/post_sets/post.rb | 8 -- app/presenters/post_presenter.rb | 2 +- app/presenters/tag_set_presenter.rb | 94 ------------------- .../comments/partials/index/_header.html.erb | 2 +- app/views/modqueue/_post.html.erb | 2 +- app/views/post_versions/_listing.html.erb | 5 +- app/views/post_votes/index.html.erb | 2 +- app/views/posts/index.html.erb | 2 +- app/views/posts/show.html+tooltip.erb | 2 +- app/views/posts/show.html.erb | 2 +- app/views/related_tags/_source_tags.html.erb | 4 +- app/views/related_tags/_tag_column.html.erb | 2 +- app/views/reports/upload_tags.html.erb | 2 +- app/views/uploads/index.html.erb | 2 +- test/components/tag_list_component_test.rb | 63 +++++++++++++ test/factories/tag.rb | 8 ++ 25 files changed, 256 insertions(+), 131 deletions(-) create mode 100644 app/components/tag_list_component.rb create mode 100644 app/components/tag_list_component/tag_list_component.html+categorized.erb create mode 100644 app/components/tag_list_component/tag_list_component.html+inline.erb create mode 100644 app/components/tag_list_component/tag_list_component.html+search.erb create mode 100644 app/components/tag_list_component/tag_list_component.html+simple.erb create mode 100644 app/components/tag_list_component/tag_list_component.html.scss create mode 100644 test/components/tag_list_component_test.rb diff --git a/app/components/tag_list_component.rb b/app/components/tag_list_component.rb new file mode 100644 index 000000000..48379e8ba --- /dev/null +++ b/app/components/tag_list_component.rb @@ -0,0 +1,51 @@ +# frozen_string_literal: true + +class TagListComponent < ApplicationComponent + attr_reader :tags, :current_query, :show_extra_links + + def initialize(tags: [], current_query: nil, show_extra_links: false) + @tags = tags + @current_query = current_query + @show_extra_links = show_extra_links + end + + def self.tags_from_names(tag_names) + names_to_tags = Tag.where(name: tag_names).map { |tag| [tag.name, tag] }.to_h + + tag_names.map do |name| + names_to_tags.fetch(name) { Tag.new(name: name).freeze } + end + end + + def categorized_tags(categories, &block) + return to_enum(:categorized_tags, categories) unless block_given? + + categories.each do |category| + tags = tags_for_category(category) + yield category, tags if tags.present? + end + end + + def tags_for_category(category_name) + category = TagCategory.mapping[category_name.downcase] + tags_by_category[category] || [] + end + + def tags_by_category + @tags_by_category ||= tags.sort_by(&:name).group_by(&:category) + end + + def is_underused_tag?(tag) + tag.post_count <= 1 && tag.general? + end + + def humanized_post_count(tag) + if tag.post_count >= 10_000 + "#{tag.post_count / 1_000}k" + elsif tag.post_count >= 1_000 + "%.1fk" % (tag.post_count / 1_000.0) + else + tag.post_count.to_s + end + end +end diff --git a/app/components/tag_list_component/tag_list_component.html+categorized.erb b/app/components/tag_list_component/tag_list_component.html+categorized.erb new file mode 100644 index 000000000..5a63bfa6f --- /dev/null +++ b/app/components/tag_list_component/tag_list_component.html+categorized.erb @@ -0,0 +1,30 @@ +
+ <% categorized_tags(TagCategory.split_header_list).each do |category_name, tags| %> +

+ <%= category_name.capitalize.pluralize(tags) %> +

+ + + <% end %> +
diff --git a/app/components/tag_list_component/tag_list_component.html+inline.erb b/app/components/tag_list_component/tag_list_component.html+inline.erb new file mode 100644 index 000000000..d8cc0b8cc --- /dev/null +++ b/app/components/tag_list_component/tag_list_component.html+inline.erb @@ -0,0 +1,7 @@ +
+ <% categorized_tags(TagCategory.categorized_list).each do |category_name, tags| %> + <% tags.each do |t| %> + <%= link_to t.name, posts_path(tags: t.name), class: "search-tag tag-type-#{t.category}", "data-tag-name": t.name %> + <% end %> + <% end %> +
diff --git a/app/components/tag_list_component/tag_list_component.html+search.erb b/app/components/tag_list_component/tag_list_component.html+search.erb new file mode 100644 index 000000000..09e5fccc1 --- /dev/null +++ b/app/components/tag_list_component/tag_list_component.html+search.erb @@ -0,0 +1,22 @@ + diff --git a/app/components/tag_list_component/tag_list_component.html+simple.erb b/app/components/tag_list_component/tag_list_component.html+simple.erb new file mode 100644 index 000000000..c26e2f70a --- /dev/null +++ b/app/components/tag_list_component/tag_list_component.html+simple.erb @@ -0,0 +1,5 @@ +
+ <% tags.each do |t| %> + <%= link_to t.pretty_name, posts_path(tags: t.name), class: "search-tag tag-type-#{t.category}", "data-tag-name": t.name %> + <% end %> +
diff --git a/app/components/tag_list_component/tag_list_component.html.scss b/app/components/tag_list_component/tag_list_component.html.scss new file mode 100644 index 000000000..a9e74226a --- /dev/null +++ b/app/components/tag_list_component/tag_list_component.html.scss @@ -0,0 +1,25 @@ +.tag-list { + a.search-tag { + overflow-wrap: normal; + } + + &.inline-tag-list { + display: inline; + + a { + margin-right: 0.5em; + } + } + + &.categorized-tag-list { + ul { + margin-bottom: 1em; + } + } + + &.simple-tag-list { + a { + display: block; + } + } +} diff --git a/app/helpers/components_helper.rb b/app/helpers/components_helper.rb index b12be2897..41f733d05 100644 --- a/app/helpers/components_helper.rb +++ b/app/helpers/components_helper.rb @@ -22,4 +22,32 @@ module ComponentsHelper def render_post_navbar(post, **options) render PostNavbarComponent.new(post: post, **options) end + + # A simple vertical tag list with no post counts. Used in related tags. + def render_simple_tag_list(tag_names, **options) + tags = TagListComponent.tags_from_names(tag_names) + render TagListComponent.new(tags: tags, **options).with_variant(:simple) + end + + # A horizontal tag list, with tags grouped by category. Used in post + # tooltips, on the comments index, and in the modqueue. + def render_inline_tag_list(post, **options) + render TagListComponent.new(tags: post.tags, **options).with_variant(:inline) + end + + def render_inline_tag_list_from_names(tag_names, **options) + tags = TagListComponent.tags_from_names(tag_names) + render TagListComponent.new(tags: tags, **options).with_variant(:inline) + end + + # A vertical tag list, with tags split into categories. Used on post show pages. + def render_categorized_tag_list(post, **options) + render TagListComponent.new(tags: post.tags, **options).with_variant(:categorized) + end + + # A vertical tag list, used in the post index sidebar. + def render_search_tag_list(tag_names, **options) + tags = TagListComponent.tags_from_names(tag_names) + render TagListComponent.new(tags: tags, **options).with_variant(:search) + end end diff --git a/app/javascript/src/styles/common/tags.scss.erb b/app/javascript/src/styles/common/tags.scss.erb index 6d5e2ca59..31dc5147d 100644 --- a/app/javascript/src/styles/common/tags.scss.erb +++ b/app/javascript/src/styles/common/tags.scss.erb @@ -34,18 +34,3 @@ } } <% end %> - -a.search-tag { - word-break: break-word; -} - -.inline-tag-list { - ul { - display: inline; - - li { - display: inline; - margin-right: 0.5em; - } - } -} diff --git a/app/javascript/src/styles/specific/mod_queue.scss.erb b/app/javascript/src/styles/specific/mod_queue.scss.erb index 9874a248c..16c5ab575 100644 --- a/app/javascript/src/styles/specific/mod_queue.scss.erb +++ b/app/javascript/src/styles/specific/mod_queue.scss.erb @@ -27,7 +27,7 @@ div#c-modqueue { } <% Danbooru.config.modqueue_warning_tags.each do |tag| %> - li[data-tag-name="<%= tag %>"] { + a[data-tag-name="<%= tag %>"] { background-color: var(--modqueue-tag-warning-color); } <% end %> diff --git a/app/logical/post_sets/post.rb b/app/logical/post_sets/post.rb index fff5581ac..9c377581c 100644 --- a/app/logical/post_sets/post.rb +++ b/app/logical/post_sets/post.rb @@ -201,14 +201,6 @@ module PostSets searches = ["search:all"] + SavedSearch.labels_for(CurrentUser.user.id).map {|x| "search:#{x}"} searches.take(MAX_SIDEBAR_TAGS) end - - def tag_set_presenter - @tag_set_presenter ||= TagSetPresenter.new(related_tags) - end - - def tag_list_html(**options) - tag_set_presenter.tag_list_html(name_only: query.is_metatag?(:search), **options) - end end end end diff --git a/app/presenters/post_presenter.rb b/app/presenters/post_presenter.rb index 5b308d150..a0de157b5 100644 --- a/app/presenters/post_presenter.rb +++ b/app/presenters/post_presenter.rb @@ -1,6 +1,6 @@ class PostPresenter attr_reader :pool, :next_post_in_pool - delegate :tag_list_html, :split_tag_list_html, :split_tag_list_text, :inline_tag_list_html, to: :tag_set_presenter + delegate :split_tag_list_text, to: :tag_set_presenter def initialize(post) @post = post diff --git a/app/presenters/tag_set_presenter.rb b/app/presenters/tag_set_presenter.rb index d167c3cd1..f95f836c5 100644 --- a/app/presenters/tag_set_presenter.rb +++ b/app/presenters/tag_set_presenter.rb @@ -14,48 +14,6 @@ class TagSetPresenter @tag_names = tag_names end - def tag_list_html(current_query: "", show_extra_links: false, name_only: false) - html = "" - - if ordered_tags.present? - html << '" - end - - html.html_safe - end - - def split_tag_list_html(headers: true, category_list: TagCategory.split_header_list, current_query: "", show_extra_links: false, name_only: false, humanize_tags: true) - html = "" - - category_list.each do |category| - typetags = tags_for_category(category) - - if typetags.any? - if headers - html << %{

#{category.capitalize.pluralize(typetags.size)}

} - end - - html << %{" - end - end - - html.html_safe - end - - # compact (horizontal) list, as seen in the /comments index. - def inline_tag_list_html(humanize_tags: false) - html = split_tag_list_html(category_list: TagCategory.categorized_list, headers: false, show_extra_links: false, name_only: true, humanize_tags: humanize_tags) - %{#{html}}.html_safe - end - # the list of tags inside the tag box in the post edit form. def split_tag_list_text(category_list: TagCategory.categorized_list) category_list.map do |category| @@ -104,57 +62,5 @@ class TagSetPresenter end end - def build_list_item(tag, name_only: false, humanize_tags: true, show_extra_links: false, current_query: "") - name = tag.name - count = tag.post_count - category = tag.category - - html = %{
  • } - - unless name_only - if tag.artist? - html << %{? } - elsif name =~ /\A\d+\z/ - html << %{? } - else - html << %{? } - end - - if show_extra_links && current_query.present? - html << %{+ } - html << %{– } - end - end - - humanized_tag = humanize_tags ? name.tr("_", " ") : name - html << %{#{h(humanized_tag)} } - - unless name_only || tag.new_record? - if count >= 10_000 - post_count = "#{count / 1_000}k" - elsif count >= 1_000 - post_count = format("%.1fk", (count / 1_000.0)) - else - post_count = count - end - - is_underused_tag = count <= 1 && tag.general? - klass = "post-count#{is_underused_tag ? " low-post-count" : ""}" - - html << %{#{post_count}} - end - - html << "
    • " - html - end - - def h(s) - CGI.escapeHTML(s) - end - - def u(s) - CGI.escape(s) - end - memoize :tags, :tags_by_category, :ordered_tags, :humanized_essential_tag_string end diff --git a/app/views/comments/partials/index/_header.html.erb b/app/views/comments/partials/index/_header.html.erb index 53a1a1dc0..dd806d822 100644 --- a/app/views/comments/partials/index/_header.html.erb +++ b/app/views/comments/partials/index/_header.html.erb @@ -19,7 +19,7 @@
    Tags - <%= post.presenter.inline_tag_list_html %> + <%= render_inline_tag_list(post) %>
    diff --git a/app/views/modqueue/_post.html.erb b/app/views/modqueue/_post.html.erb index b2c29e1d3..e09951d85 100644 --- a/app/views/modqueue/_post.html.erb +++ b/app/views/modqueue/_post.html.erb @@ -51,7 +51,7 @@
    Tags - <%= post.presenter.inline_tag_list_html %> + <%= render_inline_tag_list(post) %>
    diff --git a/app/views/post_versions/_listing.html.erb b/app/views/post_versions/_listing.html.erb index 87b04a8a6..11477b4f6 100644 --- a/app/views/post_versions/_listing.html.erb +++ b/app/views/post_versions/_listing.html.erb @@ -22,7 +22,10 @@ <%= post_version_field(post_version, :rating) %> <%= post_version_field(post_version, :parent_id) %> -
    Tags: <%= TagSetPresenter.new(post_version.tag_array).inline_tag_list_html %>
    +
    + Tags: + <%= render_inline_tag_list_from_names(post_version.tag_array) %> +
    <%= post_source_tag(post_version.source) %>
    diff --git a/app/views/post_votes/index.html.erb b/app/views/post_votes/index.html.erb index 2b87427b8..609ef2f7b 100644 --- a/app/views/post_votes/index.html.erb +++ b/app/views/post_votes/index.html.erb @@ -13,7 +13,7 @@ <%= post_preview(vote.post, show_deleted: true) %> <% end %> <% t.column "Tags", td: {class: "col-expand"} do |vote| %> - <%= TagSetPresenter.new(vote.post.tag_array).inline_tag_list_html %> + <%= render_inline_tag_list(vote.post) %> <% end %> <% t.column "Score" do |vote| %> <%= link_to sprintf("%+d", vote.score), post_votes_path(search: { score: vote.score }) %> diff --git a/app/views/posts/index.html.erb b/app/views/posts/index.html.erb index 1b5330f92..2e2e8f9f6 100644 --- a/app/views/posts/index.html.erb +++ b/app/views/posts/index.html.erb @@ -7,7 +7,7 @@

    Tags

    - <%= @post_set.tag_list_html(current_query: params[:tags], show_extra_links: policy(Post).show_extra_links?) %> + <%= render_search_tag_list(@post_set.related_tags, current_query: params[:tags], show_extra_links: policy(Post).show_extra_links?) %>
    <%= render "posts/partials/index/options" %> diff --git a/app/views/posts/show.html+tooltip.erb b/app/views/posts/show.html+tooltip.erb index 4063090ca..1783a2746 100644 --- a/app/views/posts/show.html+tooltip.erb +++ b/app/views/posts/show.html+tooltip.erb @@ -52,6 +52,6 @@ <% end %> - <%= @post.presenter.inline_tag_list_html %> + <%= render_inline_tag_list(@post) %> diff --git a/app/views/posts/show.html.erb b/app/views/posts/show.html.erb index fb56206fb..679d0c189 100644 --- a/app/views/posts/show.html.erb +++ b/app/views/posts/show.html.erb @@ -11,7 +11,7 @@ <%= render "posts/partials/index/blacklist" %>
    - <%= @post.presenter.split_tag_list_html(current_query: params[:q], show_extra_links: policy(@post).show_extra_links?) %> + <%= render_categorized_tag_list(@post, current_query: params[:q], show_extra_links: policy(@post).show_extra_links?) %>
    diff --git a/app/views/related_tags/_source_tags.html.erb b/app/views/related_tags/_source_tags.html.erb index 2b1899c67..512135444 100644 --- a/app/views/related_tags/_source_tags.html.erb +++ b/app/views/related_tags/_source_tags.html.erb @@ -9,11 +9,11 @@ none <% else %> <% if source.artists.any?(&:is_banned?) %> - <%= TagSetPresenter.new(["banned_artist"]).tag_list_html(name_only: true) %> + <%= render_simple_tag_list(["banned_artist"]) %> <% end %> <% source.artists.each do |artist| %> - <%= TagSetPresenter.new([artist.name]).tag_list_html(name_only: true) %> + <%= render_simple_tag_list(["banned_artist"]) %>