From bcc9ad6f57f72faa1367063117cf992ce1031a08 Mon Sep 17 00:00:00 2001 From: Martin Brennan Date: Fri, 1 May 2020 16:14:20 +1000 Subject: [PATCH] FIX: Bookmark UI tweaks (#9604) * When hovering over the bookmark icon for a post, show the name of the bookmark at the end of the tooltip _if_ it has been set. * Order bookmarks by `updated_at DESC` in the user list and show that instead of created at. --- .../discourse/app/lib/transform-post.js | 1 + .../app/templates/user/bookmarks.hbs | 4 +-- .../discourse/app/widgets/post-menu.js | 10 ++++--- app/serializers/user_bookmark_serializer.rb | 1 + config/locales/client.en.yml | 7 ++--- lib/bookmark_query.rb | 3 +-- spec/lib/bookmark_query_spec.rb | 27 ++++++++++++++----- 7 files changed, 35 insertions(+), 18 deletions(-) diff --git a/app/assets/javascripts/discourse/app/lib/transform-post.js b/app/assets/javascripts/discourse/app/lib/transform-post.js index 60823f04fb8..c48f0421602 100644 --- a/app/assets/javascripts/discourse/app/lib/transform-post.js +++ b/app/assets/javascripts/discourse/app/lib/transform-post.js @@ -36,6 +36,7 @@ export function transformBasicPost(post) { avatar_template: post.avatar_template, bookmarked: post.bookmarked, bookmarkReminderAt: post.bookmark_reminder_at, + bookmarkName: post.bookmark_name, bookmarkReminderType: post.bookmark_reminder_type, yours: post.yours, shareUrl: post.get("shareUrl"), diff --git a/app/assets/javascripts/discourse/app/templates/user/bookmarks.hbs b/app/assets/javascripts/discourse/app/templates/user/bookmarks.hbs index 77ecf912317..aa40b814b5d 100644 --- a/app/assets/javascripts/discourse/app/templates/user/bookmarks.hbs +++ b/app/assets/javascripts/discourse/app/templates/user/bookmarks.hbs @@ -6,7 +6,7 @@ - + @@ -39,7 +39,7 @@ {{discourse-tags bookmark mode="list" tagsForUser=tagsForUser}} - + {{raw "list/activity-column" topic=bookmark class="num" tagName="td"}}
{{i18n "topic.title"}}{{i18n "post.bookmarks.created"}}{{i18n "post.bookmarks.updated"}} {{i18n "activity"}}  
{{format-date bookmark.created_at format="tiny"}}{{format-date bookmark.updated_at format="tiny"}} {{bookmark-actions-dropdown diff --git a/app/assets/javascripts/discourse/app/widgets/post-menu.js b/app/assets/javascripts/discourse/app/widgets/post-menu.js index 73a786e0e3c..62489dc94d9 100644 --- a/app/assets/javascripts/discourse/app/widgets/post-menu.js +++ b/app/assets/javascripts/discourse/app/widgets/post-menu.js @@ -289,7 +289,7 @@ registerButton("bookmark", attrs => { let classNames = ["bookmark", "with-reminder"]; let title = "bookmarks.not_bookmarked"; - let titleOptions = {}; + let titleOptions = { name: "" }; if (attrs.bookmarked) { classNames.push("bookmarked"); @@ -300,14 +300,16 @@ registerButton("bookmark", attrs => { Discourse.currentUser.resolvedTimezone() ); title = "bookmarks.created_with_reminder"; - titleOptions = { - date: formattedReminder - }; + titleOptions.date = formattedReminder; } else if (attrs.bookmarkReminderType === "at_desktop") { title = "bookmarks.created_with_at_desktop_reminder"; } else { title = "bookmarks.created"; } + + if (attrs.bookmarkName) { + titleOptions.name = `. ${attrs.bookmarkName}`; + } } return { diff --git a/app/serializers/user_bookmark_serializer.rb b/app/serializers/user_bookmark_serializer.rb index b87b3a9d000..d4fcc4a72de 100644 --- a/app/serializers/user_bookmark_serializer.rb +++ b/app/serializers/user_bookmark_serializer.rb @@ -8,6 +8,7 @@ class UserBookmarkSerializer < ApplicationSerializer attributes :id, :created_at, + :updated_at, :topic_id, :linked_post_number, :post_id, diff --git a/config/locales/client.en.yml b/config/locales/client.en.yml index 9c5d262701a..d7ffe903a8d 100644 --- a/config/locales/client.en.yml +++ b/config/locales/client.en.yml @@ -307,10 +307,10 @@ en: unbookmark_with_reminder: "Click to remove all bookmarks and reminders in this topic. You have a reminder set %{reminder_at} for this topic." bookmarks: - created: "you've bookmarked this post" + created: "you've bookmarked this post%{name}" not_bookmarked: "bookmark this post" - created_with_reminder: "you've bookmarked this post with a reminder %{date}" - created_with_at_desktop_reminder: "you've bookmarked this post and will be reminded next time you are at your desktop" + created_with_reminder: "you've bookmarked this post with a reminder %{date}%{name}" + created_with_at_desktop_reminder: "you've bookmarked this post and will be reminded next time you are at your desktop%{name}" remove: "Remove Bookmark" delete: "Delete Bookmark" confirm_delete: "Are you sure you want to delete this bookmark? The reminder will also be deleted." @@ -2724,6 +2724,7 @@ en: create: "Create bookmark" edit: "Edit bookmark" created: "Created" + updated: "Updated" name: "Name" name_placeholder: "What is this bookmark for?" set_reminder: "Remind me" diff --git a/lib/bookmark_query.rb b/lib/bookmark_query.rb index 45442c211f4..b5351f108ad 100644 --- a/lib/bookmark_query.rb +++ b/lib/bookmark_query.rb @@ -27,8 +27,7 @@ class BookmarkQuery end def list_all - results = user_bookmarks - .order('bookmarks.created_at DESC') + results = user_bookmarks.order('bookmarks.updated_at DESC') topics = Topic.listable_topics.secured(@guardian) pms = Topic.private_messages_for_user(@user) diff --git a/spec/lib/bookmark_query_spec.rb b/spec/lib/bookmark_query_spec.rb index c627abfc85e..de717575382 100644 --- a/spec/lib/bookmark_query_spec.rb +++ b/spec/lib/bookmark_query_spec.rb @@ -4,20 +4,16 @@ require 'rails_helper' RSpec.describe BookmarkQuery do fab!(:user) { Fabricate(:user) } - fab!(:bookmark1) { Fabricate(:bookmark, user: user) } - fab!(:bookmark2) { Fabricate(:bookmark, user: user) } let(:params) { {} } def bookmark_query(user: nil, params: nil) BookmarkQuery.new(user: user || self.user, params: params || self.params) end - before do - TopicUser.change(user.id, bookmark1.topic_id, total_msecs_viewed: 1) - TopicUser.change(user.id, bookmark2.topic_id, total_msecs_viewed: 1) - end - describe "#list_all" do + fab!(:bookmark1) { Fabricate(:bookmark, user: user) } + fab!(:bookmark2) { Fabricate(:bookmark, user: user) } + it "returns all the bookmarks for a user" do expect(bookmark_query.list_all.count).to eq(2) end @@ -143,4 +139,21 @@ RSpec.describe BookmarkQuery do end end end + + describe "#list_all ordering" do + let!(:bookmark1) { Fabricate(:bookmark, user: user, updated_at: 1.day.ago) } + let!(:bookmark2) { Fabricate(:bookmark, user: user, updated_at: 2.days.ago) } + let!(:bookmark3) { Fabricate(:bookmark, user: user, updated_at: 6.days.ago) } + let!(:bookmark4) { Fabricate(:bookmark, user: user, updated_at: 4.days.ago) } + let!(:bookmark5) { Fabricate(:bookmark, user: user, updated_at: 3.days.ago) } + it "orders by updated_at" do + expect(bookmark_query.list_all.map(&:id)).to eq([ + bookmark1.id, + bookmark2.id, + bookmark5.id, + bookmark4.id, + bookmark3.id + ]) + end + end end