From 3ae62d08eb16d203ce7e3d87316b79ff2dbb7602 Mon Sep 17 00:00:00 2001 From: evazion Date: Fri, 19 Nov 2021 20:54:02 -0600 Subject: [PATCH] favorites: show favlist when hovering over favcount. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Changes: * Make it so you can click or hover over a post's favorite count to see the list of public favorites. * Remove the "Show »" button next to the favorite count. * Make the favorites list visible to all users. Before favorites were only visible to Gold users. * Make the /favorites page show the list of all public favorites, instead of redirecting to the current user's favorites. * Add /posts/:id/favorites endpoint. * Add /users/:id/favorites endpoint. This is for several reasons: * To make viewing favorites work the same way as viewing upvotes. * To make posts load faster for Gold users. Before, we loaded all the favorites when viewing a post, even when the user didn't look at them. This made pageloads slower for posts that had hundreds or thousands of favorites. Now we only load the favlist if the user hovers over the favcount. * To make the favorite list visible to all users. Before, it wasn't visible to non-Gold users, because of the performance issue listed above. * To make it more obvious that favorites are public by default. Before, since regular users could only see the favcount, they may have mistakenly believed other users couldn't see their favorites. --- app/components/favorites_tooltip_component.rb | 24 +++++++ .../favorites_tooltip_component.html.erb | 9 +++ .../favorites_tooltip_component.js | 65 +++++++++++++++++++ .../favorites_tooltip_component.scss | 13 ++++ app/controllers/favorites_controller.rb | 23 +++---- app/helpers/components_helper.rb | 4 ++ app/javascript/packs/application.js | 2 + app/javascript/src/javascripts/posts.js | 8 --- app/javascript/src/styles/specific/posts.scss | 4 -- app/models/favorite.rb | 10 ++- app/models/post.rb | 8 --- app/policies/favorite_policy.rb | 4 ++ app/policies/post_policy.rb | 4 -- app/views/favorites/_search.html.erb | 6 ++ app/views/favorites/_update.js.erb | 13 +--- app/views/favorites/index.html+tooltip.erb | 3 + app/views/favorites/index.html.erb | 44 +++++++++++++ app/views/layouts/default.html.erb | 1 + .../partials/show/_favorite_list.html.erb | 2 - .../posts/partials/show/_information.html.erb | 14 ++-- config/routes.rb | 2 + test/functional/favorites_controller_test.rb | 40 +++++++----- .../moderator/post/posts_controller_test.rb | 4 +- 23 files changed, 229 insertions(+), 78 deletions(-) create mode 100644 app/components/favorites_tooltip_component.rb create mode 100644 app/components/favorites_tooltip_component/favorites_tooltip_component.html.erb create mode 100644 app/components/favorites_tooltip_component/favorites_tooltip_component.js create mode 100644 app/components/favorites_tooltip_component/favorites_tooltip_component.scss create mode 100644 app/views/favorites/_search.html.erb create mode 100644 app/views/favorites/index.html+tooltip.erb create mode 100644 app/views/favorites/index.html.erb delete mode 100644 app/views/posts/partials/show/_favorite_list.html.erb diff --git a/app/components/favorites_tooltip_component.rb b/app/components/favorites_tooltip_component.rb new file mode 100644 index 000000000..bd69821b8 --- /dev/null +++ b/app/components/favorites_tooltip_component.rb @@ -0,0 +1,24 @@ +# frozen_string_literal: true + +# This component represents the tooltip that displays when you hover over a post's favorite count. +class FavoritesTooltipComponent < ApplicationComponent + attr_reader :post, :current_user + + def initialize(post:, current_user:) + super + @post = post + @current_user = current_user + end + + def favorites + post.favorites.includes(:user).order(id: :desc) + end + + def favoriter_name(favorite) + if policy(favorite).can_see_favoriter? + link_to_user(favorite.user) + else + tag.i("hidden") + end + end +end diff --git a/app/components/favorites_tooltip_component/favorites_tooltip_component.html.erb b/app/components/favorites_tooltip_component/favorites_tooltip_component.html.erb new file mode 100644 index 000000000..13c1f5b32 --- /dev/null +++ b/app/components/favorites_tooltip_component/favorites_tooltip_component.html.erb @@ -0,0 +1,9 @@ +
+
+ <% favorites.each do |favorite| %> +
+ <%= favoriter_name(favorite) %> +
+ <% end %> +
+
diff --git a/app/components/favorites_tooltip_component/favorites_tooltip_component.js b/app/components/favorites_tooltip_component/favorites_tooltip_component.js new file mode 100644 index 000000000..918f61786 --- /dev/null +++ b/app/components/favorites_tooltip_component/favorites_tooltip_component.js @@ -0,0 +1,65 @@ +import Utility from "../../javascript/src/javascripts/utility.js"; +import { delegate, hideAll } from 'tippy.js'; +import 'tippy.js/dist/tippy.css'; + +class FavoritesTooltipComponent { + // Trigger on the post favcount link. + static TARGET_SELECTOR = "span.post-favcount a"; + static SHOW_DELAY = 125; + static HIDE_DELAY = 125; + static DURATION = 250; + static instance = null; + + static initialize() { + if ($(FavoritesTooltipComponent.TARGET_SELECTOR).length === 0) { + return; + } + + FavoritesTooltipComponent.instance = delegate("body", { + allowHTML: true, + appendTo: document.querySelector("#post-favorites-tooltips"), + delay: [FavoritesTooltipComponent.SHOW_DELAY, FavoritesTooltipComponent.HIDE_DELAY], + duration: FavoritesTooltipComponent.DURATION, + interactive: true, + maxWidth: "none", + target: FavoritesTooltipComponent.TARGET_SELECTOR, + theme: "common-tooltip", + touch: false, + + onShow: FavoritesTooltipComponent.onShow, + onHide: FavoritesTooltipComponent.onHide, + }); + } + + static async onShow(instance) { + let $target = $(instance.reference); + let $tooltip = $(instance.popper); + let postId = $target.parents("[data-id]").data("id"); + + hideAll({ exclude: instance }); + + try { + $tooltip.addClass("tooltip-loading"); + + instance._request = $.get(`/posts/${postId}/favorites?variant=tooltip`); + let html = await instance._request; + instance.setContent(html); + + $tooltip.removeClass("tooltip-loading"); + } catch (error) { + if (error.status !== 0 && error.statusText !== "abort") { + Utility.error(`Error displaying favorites for post #${postId} (error: ${error.status} ${error.statusText})`); + } + } + } + + static async onHide(instance) { + if (instance._request?.state() === "pending") { + instance._request.abort(); + } + } +} + +$(document).ready(FavoritesTooltipComponent.initialize); + +export default FavoritesTooltipComponent; diff --git a/app/components/favorites_tooltip_component/favorites_tooltip_component.scss b/app/components/favorites_tooltip_component/favorites_tooltip_component.scss new file mode 100644 index 000000000..4876c8cdd --- /dev/null +++ b/app/components/favorites_tooltip_component/favorites_tooltip_component.scss @@ -0,0 +1,13 @@ +.favorites-tooltip { + font-size: var(--text-xs); + max-height: 240px; + + .post-favoriter { + max-width: 160px; + } +} + +span.post-favcount a { + color: var(--text-color); + &:hover { text-decoration: underline; } +} diff --git a/app/controllers/favorites_controller.rb b/app/controllers/favorites_controller.rb index cd740ca72..40378df79 100644 --- a/app/controllers/favorites_controller.rb +++ b/app/controllers/favorites_controller.rb @@ -1,19 +1,16 @@ class FavoritesController < ApplicationController - respond_to :html, :xml, :json, :js + respond_to :js, :json, :html, :xml def index - authorize Favorite - if !request.format.html? - @favorites = Favorite.visible(CurrentUser.user).paginated_search(params) - respond_with(@favorites) - elsif params[:user_id].present? - user = User.find(params[:user_id]) - redirect_to posts_path(tags: "ordfav:#{user.name}", format: request.format.symbol) - elsif !CurrentUser.is_anonymous? - redirect_to posts_path(tags: "ordfav:#{CurrentUser.user.name}", format: request.format.symbol) - else - redirect_to posts_path(format: request.format.symbol) - end + post_id = params[:post_id] || params[:search][:post_id] + user_id = params[:user_id] || params[:search][:user_id] + user_name = params[:search][:user_name] + @post = Post.find(post_id) if post_id + @user = User.find(user_id) if user_id + @user = User.find_by_name(user_name) if user_name + + @favorites = authorize Favorite.visible(CurrentUser.user).paginated_search(params, defaults: { post_id: @post&.id, user_id: @user&.id }) + respond_with(@favorites) end def create diff --git a/app/helpers/components_helper.rb b/app/helpers/components_helper.rb index db731eb71..ade8022d7 100644 --- a/app/helpers/components_helper.rb +++ b/app/helpers/components_helper.rb @@ -24,6 +24,10 @@ module ComponentsHelper render PostVotesTooltipComponent.new(post: post, **options) end + def render_favorites_tooltip(post, **options) + render FavoritesTooltipComponent.new(post: post, **options) + end + def render_post_navbar(post, **options) render PostNavbarComponent.new(post: post, **options) end diff --git a/app/javascript/packs/application.js b/app/javascript/packs/application.js index 094c96578..acfb2096a 100644 --- a/app/javascript/packs/application.js +++ b/app/javascript/packs/application.js @@ -37,6 +37,7 @@ import Blacklist from "../src/javascripts/blacklists.js"; import CommentComponent from "../../components/comment_component/comment_component.js"; import CurrentUser from "../src/javascripts/current_user.js"; import Dtext from "../src/javascripts/dtext.js"; +import FavoritesTooltipComponent from "../../components/favorites_tooltip_component/favorites_tooltip_component.js"; import IqdbQuery from "../src/javascripts/iqdb_queries.js"; import Note from "../src/javascripts/notes.js"; import PopupMenuComponent from "../../components/popup_menu_component/popup_menu_component.js"; @@ -59,6 +60,7 @@ Danbooru.Blacklist = Blacklist; Danbooru.CommentComponent = CommentComponent; Danbooru.CurrentUser = CurrentUser; Danbooru.Dtext = Dtext; +Danbooru.FavoritesTooltipComponent = FavoritesTooltipComponent; Danbooru.IqdbQuery = IqdbQuery; Danbooru.Note = Note; Danbooru.PopupMenuComponent = PopupMenuComponent; diff --git a/app/javascript/src/javascripts/posts.js b/app/javascript/src/javascripts/posts.js index dc658c2a2..85fc8f070 100644 --- a/app/javascript/src/javascripts/posts.js +++ b/app/javascript/src/javascripts/posts.js @@ -30,7 +30,6 @@ Post.initialize_all = function() { if ($("#c-posts").length && $("#a-show").length) { this.initialize_links(); this.initialize_post_relationship_previews(); - this.initialize_favlist(); this.initialize_post_sections(); this.initialize_post_image_resize_links(); this.initialize_recommended(); @@ -242,13 +241,6 @@ Post.toggle_relationship_preview = function(preview, preview_link) { } } -Post.initialize_favlist = function() { - $("#show-favlist-link, #hide-favlist-link").on("click.danbooru", function(e) { - $("#favlist, #show-favlist-link, #hide-favlist-link").toggle(); - e.preventDefault(); - }); -} - Post.view_original = function(e = null) { if (Utility.test_max_width(660)) { // Do the default behavior (navigate to image) diff --git a/app/javascript/src/styles/specific/posts.scss b/app/javascript/src/styles/specific/posts.scss index 169ff14df..460f1de71 100644 --- a/app/javascript/src/styles/specific/posts.scss +++ b/app/javascript/src/styles/specific/posts.scss @@ -170,10 +170,6 @@ div#c-posts { } } - #favlist { - word-wrap: break-word; - } - #recommended.loading-recommended-posts { pointer-events: none; opacity: 0.5; diff --git a/app/models/favorite.rb b/app/models/favorite.rb index aa1c63aa0..6f8d472c2 100644 --- a/app/models/favorite.rb +++ b/app/models/favorite.rb @@ -6,10 +6,16 @@ class Favorite < ApplicationRecord after_create :upvote_post_on_create after_destroy :unvote_post_on_destroy - scope :public_favorites, -> { where(user: User.bit_prefs_match(:enable_private_favorites, false)) } + scope :public_favorites, -> { where(user: User.has_public_favorites) } def self.visible(user) - user.is_admin? ? all : where(user: user).or(public_favorites) + if user.is_admin? + all + elsif user.is_anonymous? + public_favorites + else + where(user: user).or(public_favorites) + end end def self.search(params) diff --git a/app/models/post.rb b/app/models/post.rb index 8e8e8beee..f0e6a0e7a 100644 --- a/app/models/post.rb +++ b/app/models/post.rb @@ -50,7 +50,6 @@ class Post < ApplicationRecord has_many :approvals, :class_name => "PostApproval", :dependent => :destroy has_many :disapprovals, :class_name => "PostDisapproval", :dependent => :destroy has_many :favorites, dependent: :destroy - has_many :favorited_users, through: :favorites, source: :user has_many :replacements, class_name: "PostReplacement", :dependent => :destroy attr_accessor :old_tag_string, :old_parent_id, :old_source, :old_rating, :has_constraints, :disable_versioning @@ -667,13 +666,6 @@ class Post < ApplicationRecord Favorite.exists?(post: self, user: user) end - # Users who publicly favorited this post, ordered by time of favorite. - def visible_favorited_users(viewer) - favorited_users.order("favorites.id DESC").select do |fav_user| - Pundit.policy!(viewer, fav_user).can_see_favorites? - end - end - def favorite_groups FavoriteGroup.for_post(id) end diff --git a/app/policies/favorite_policy.rb b/app/policies/favorite_policy.rb index 29784ceb7..4c3a5aef2 100644 --- a/app/policies/favorite_policy.rb +++ b/app/policies/favorite_policy.rb @@ -6,4 +6,8 @@ class FavoritePolicy < ApplicationPolicy def destroy? record.user_id == user.id end + + def can_see_favoriter? + user.is_admin? || record.user == user || !record.user.enable_private_favorites? + end end diff --git a/app/policies/post_policy.rb b/app/policies/post_policy.rb index 5612e1ce5..ae2592e42 100644 --- a/app/policies/post_policy.rb +++ b/app/policies/post_policy.rb @@ -59,10 +59,6 @@ class PostPolicy < ApplicationPolicy user.is_gold? end - def can_view_favlist? - user.is_gold? - end - # whether to show the + - links in the tag list. def show_extra_links? user.is_gold? diff --git a/app/views/favorites/_search.html.erb b/app/views/favorites/_search.html.erb new file mode 100644 index 000000000..1519c13ec --- /dev/null +++ b/app/views/favorites/_search.html.erb @@ -0,0 +1,6 @@ +<%= search_form_for(favorites_path) do |f| %> + <%= f.input :user_name, label: "Favoriter", input_html: { value: @user&.name, "data-autocomplete": "user" } %> + <%= f.input :post_id, label: "Post", input_html: { value: @post&.id } %> + <%= f.input :post_tags_match, label: "Tags", input_html: { value: params[:search][:post_tags_match], "data-autocomplete": "tag-query" } %> + <%= f.submit "Search" %> +<% end %> diff --git a/app/views/favorites/_update.js.erb b/app/views/favorites/_update.js.erb index b87eabc06..afd469fb8 100644 --- a/app/views/favorites/_update.js.erb +++ b/app/views/favorites/_update.js.erb @@ -4,19 +4,8 @@ $("#add-to-favorites, #add-fav-button, #remove-from-favorites, #remove-fav-button").toggle(); $("#remove-fav-button").addClass("animate"); $("span.post-votes[data-id=<%= @post.id %>]").replaceWith("<%= j render_post_votes @post, current_user: CurrentUser.user %>"); - $("#favcount-for-post-<%= @post.id %>").text(<%= @post.fav_count %>); + $("span.post-favcount[data-id=<%= @post.id %>]").html("<%= j link_to @post.fav_count, favorites_path(post_id: @post.id, variant: :compact) %>"); $(".fav-buttons").toggleClass("fav-buttons-false").toggleClass("fav-buttons-true"); - <% if policy(@post).can_view_favlist? %> - var fav_count = <%= @post.fav_count %>; - $("#favlist").html("<%= j render "posts/partials/show/favorite_list", post: @post %>"); - - if (fav_count === 0) { - $("#show-favlist-link, #hide-favlist-link, #favlist").hide(); - } else if (!$("#favlist").is(":visible")) { - $("#show-favlist-link").show(); - } - <% end %> - Danbooru.Utility.notice("<%= j flash[:notice] %>"); <% end %> diff --git a/app/views/favorites/index.html+tooltip.erb b/app/views/favorites/index.html+tooltip.erb new file mode 100644 index 000000000..288e797d7 --- /dev/null +++ b/app/views/favorites/index.html+tooltip.erb @@ -0,0 +1,3 @@ +<% if @post.present? %> + <%= render_favorites_tooltip(@post, current_user: CurrentUser.user) %> +<% end %> diff --git a/app/views/favorites/index.html.erb b/app/views/favorites/index.html.erb new file mode 100644 index 000000000..d25e903b4 --- /dev/null +++ b/app/views/favorites/index.html.erb @@ -0,0 +1,44 @@ +
+
+ <% if @post %> +

<%= link_to "Favorites", favorites_path %>/<%= link_to @post.dtext_shortlink, @post %>

+ <% elsif @user %> +

<%= link_to "Favorites", favorites_path %>/<%= link_to_user @user %>

+ <% else %> +

<%= link_to "Favorites", favorites_path %>

+ <% end %> + + <%= render "search" %> + + <%= table_for @favorites.includes(:user, post: [:uploader, :media_asset]), class: "striped autofit" do |t| %> + <% if @post.nil? %> + <% t.column "Post" do |favorite| %> + <%= post_preview(favorite.post, show_deleted: true) %> + <% end %> + + <% t.column "Tags", td: {class: "col-expand"} do |favorite| %> + <%= render_inline_tag_list(favorite.post) %> + <% end %> + + <% t.column "Uploader" do |favorite| %> + <%= link_to_user favorite.post.uploader %> + <%= link_to "»", favorites_path(search: { post_tags_match: "user:#{favorite.post.uploader.name}" }) %> +
<%= time_ago_in_words_tagged(favorite.post.created_at) %>
+ <% end %> + <% end %> + + <% if @user.nil? %> + <% t.column "Favoriter" do |favorite| %> + <% if policy(favorite).can_see_favoriter? %> + <%= link_to_user favorite.user %> + <%= link_to "»", favorites_path(search: { user_name: favorite.user.name }) %> + <% else %> + hidden + <% end %> + <% end %> + <% end %> + <% end %> + + <%= numbered_paginator(@favorites) %> +
+
diff --git a/app/views/layouts/default.html.erb b/app/views/layouts/default.html.erb index e927f4ab8..3dee6ac89 100644 --- a/app/views/layouts/default.html.erb +++ b/app/views/layouts/default.html.erb @@ -102,6 +102,7 @@
+
diff --git a/app/views/posts/partials/show/_favorite_list.html.erb b/app/views/posts/partials/show/_favorite_list.html.erb deleted file mode 100644 index 863a24a7a..000000000 --- a/app/views/posts/partials/show/_favorite_list.html.erb +++ /dev/null @@ -1,2 +0,0 @@ -<%# post %> -<%= safe_join(post.visible_favorited_users(CurrentUser.user).map { |user| link_to_user(user) }, ", ") %> diff --git a/app/views/posts/partials/show/_information.html.erb b/app/views/posts/partials/show/_information.html.erb index a389d4be2..91f1c4e60 100644 --- a/app/views/posts/partials/show/_information.html.erb +++ b/app/views/posts/partials/show/_information.html.erb @@ -23,14 +23,12 @@
  • Score: <%= render_post_votes post, current_user: CurrentUser.user %>
  • -
  • Favorites: <%= post.fav_count %> - <% if policy(post).can_view_favlist? %> - <%= link_to "Show »", "#", id: "show-favlist-link", style: ("display: none;" if post.fav_count == 0) %> - <%= link_to "« Hide", "#", id: "hide-favlist-link", style: "display: none;" %> - - <% end %>
  • +
  • + Favorites: + <%= tag.span class: "post-favcount", "data-id": post.id do %> + <%= link_to post.fav_count, post_favorites_path(post) %> + <% end %> +
  • Status: <% if post.is_pending? %> diff --git a/config/routes.rb b/config/routes.rb index b7e2287df..7d104b16e 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -194,6 +194,7 @@ Rails.application.routes.draw do # XXX Use `only: []` to avoid redefining post routes defined at top of file. resources :posts, only: [] do resources :events, :only => [:index], :controller => "post_events" + resources :favorites, only: [:index, :create, :destroy] resources :replacements, :only => [:index, :new, :create], :controller => "post_replacements" resource :artist_commentary, only: [:show] do collection { put :create_or_update } @@ -252,6 +253,7 @@ Rails.application.routes.draw do end end resources :users do + resources :favorites, only: [:index, :create, :destroy] resources :favorite_groups, controller: "favorite_groups", only: [:index], as: "favorite_groups" resource :email, only: [:show, :edit, :update] do get :verify diff --git a/test/functional/favorites_controller_test.rb b/test/functional/favorites_controller_test.rb index f714e8393..936e69e48 100644 --- a/test/functional/favorites_controller_test.rb +++ b/test/functional/favorites_controller_test.rb @@ -10,25 +10,35 @@ class FavoritesControllerTest < ActionDispatch::IntegrationTest end context "index action" do - should "redirect the user_id param to an ordfav: search" do - get favorites_path(user_id: @user.id) - assert_redirected_to posts_path(tags: "ordfav:#{@user.name}", format: "html") - end - - should "redirect members to an ordfav: search" do - get_auth favorites_path, @user - assert_redirected_to posts_path(tags: "ordfav:#{@user.name}", format: "html") - end - - should "redirect anonymous users to the posts index" do - get favorites_path - assert_redirected_to posts_path(format: "html") - end - should "render for json" do get favorites_path, as: :json assert_response :success end + + should "render for html" do + get favorites_path + assert_response :success + end + + should "render for /favorites?variant=tooltip" do + get post_favorites_path(@post, variant: "tooltip") + assert_response :success + end + + should "render for /users/:id/favorites" do + get user_favorites_path(@user) + assert_response :success + end + + should "render for /posts/:id/favorites" do + get post_favorites_path(@faved_post) + assert_response :success + end + + should "render for /favorites?search[user_name]=" do + get favorites_path(search: { user_name: @user.name }) + assert_response :success + end end context "create action" do diff --git a/test/functional/moderator/post/posts_controller_test.rb b/test/functional/moderator/post/posts_controller_test.rb index 7b13dc163..6aff77f78 100644 --- a/test/functional/moderator/post/posts_controller_test.rb +++ b/test/functional/moderator/post/posts_controller_test.rb @@ -43,8 +43,8 @@ module Moderator @parent.reload @child.reload as(@admin) do - assert_equal(users.map(&:id).sort, @parent.favorited_users.map(&:id).sort) - assert_equal([], @child.favorited_users.map(&:id)) + assert_equal(users.map(&:id).sort, @parent.favorites.map(&:user_id).sort) + assert_equal([], @child.favorites.map(&:user_id)) end end end