diff --git a/app/assets/javascripts/discourse/app/components/bookmark-menu.gjs b/app/assets/javascripts/discourse/app/components/bookmark-menu.gjs index a5f58e4276e..ed16c77d838 100644 --- a/app/assets/javascripts/discourse/app/components/bookmark-menu.gjs +++ b/app/assets/javascripts/discourse/app/components/bookmark-menu.gjs @@ -6,7 +6,6 @@ import didInsert from "@ember/render-modifiers/modifiers/did-insert"; import { inject as service } from "@ember/service"; import DButton from "discourse/components/d-button"; import BookmarkModal from "discourse/components/modal/bookmark"; -import concatClass from "discourse/helpers/concat-class"; import { popupAjaxError } from "discourse/lib/ajax-error"; import { TIME_SHORTCUT_TYPES, @@ -59,18 +58,61 @@ export default class BookmarkMenu extends Component { return I18n.t("bookmarks.not_bookmarked"); } else { if (this.existingBookmark.reminderAt) { - return I18n.t("bookmarks.created_with_reminder", { + return I18n.t("bookmarks.created_with_reminder_generic", { date: this.existingBookmark.formattedReminder(this.timezone), name: this.existingBookmark.name || "", }); } else { - return I18n.t("bookmarks.created", { + return I18n.t("bookmarks.created_generic", { name: this.existingBookmark.name || "", }); } } } + get buttonClasses() { + let cssClasses = ["bookmark widget-button bookmark-menu__trigger"]; + + if (!this.args.showLabel) { + cssClasses.push("btn-icon no-text"); + } else { + cssClasses.push("btn-icon-text"); + } + + if (this.args.buttonClasses) { + cssClasses.push(this.args.buttonClasses); + } + + if (this.existingBookmark) { + cssClasses.push("bookmarked"); + if (this.existingBookmark.reminderAt) { + cssClasses.push("with-reminder"); + } + } + + return cssClasses.join(" "); + } + + get buttonIcon() { + if (this.existingBookmark?.reminderAt) { + return "discourse-bookmark-clock"; + } else { + return "bookmark"; + } + } + + get buttonLabel() { + if (!this.args.showLabel) { + return; + } + + if (this.existingBookmark) { + return I18n.t("bookmarked.edit_bookmark"); + } else { + return I18n.t("bookmarked.title"); + } + } + @action reminderShortcutTimeTitle(option) { if (!option.time) { @@ -196,25 +238,16 @@ export default class BookmarkMenu extends Component { {{didInsert this.setReminderShortcuts}} @identifier="bookmark-menu" @triggers={{array "click"}} - class={{concatClass - "bookmark widget-button btn-flat no-text btn-icon bookmark-menu__trigger" - (if this.existingBookmark "bookmarked") - (if this.existingBookmark.reminderAt "with-reminder") - }} + class={{this.buttonClasses}} @title={{this.buttonTitle}} + @label={{this.buttonLabel}} + @icon={{this.buttonIcon}} @onClose={{this.onCloseMenu}} @onShow={{this.onShowMenu}} @onRegisterApi={{this.onRegisterApi}} @modalForMobile={{true}} @arrow={{false}} > - <:trigger> - {{#if this.existingBookmark.reminderAt}} - {{icon "discourse-bookmark-clock"}} - {{else}} - {{icon "bookmark"}} - {{/if}} - <:content>
diff --git a/app/assets/javascripts/discourse/app/components/topic-footer-buttons.hbs b/app/assets/javascripts/discourse/app/components/topic-footer-buttons.hbs index f601dd84a5b..dcec0512030 100644 --- a/app/assets/javascripts/discourse/app/components/topic-footer-buttons.hbs +++ b/app/assets/javascripts/discourse/app/components/topic-footer-buttons.hbs @@ -27,20 +27,27 @@ {{#each this.inlineActionables as |actionable|}} {{#if (eq actionable.type "inline-button")}} - + {{#if (eq actionable.id "bookmark")}} + + {{else}} + + {{/if}} {{else}} + bookmark.bookmarkable_id === this.model.id && + bookmark.bookmarkable_type === this.type + ) || this.bookmarkApi.buildNewBookmark(this.type, this.model.id); + this.trackedBookmark = new BookmarkFormData(this.bookmarkModel); + } + + create() { + return this.bookmarkApi + .create(this.trackedBookmark) + .then((updatedBookmark) => { + this.trackedBookmark = updatedBookmark; + }); + } + + delete() { + return this.bookmarkApi.delete(this.trackedBookmark.id); + } + + save() { + return this.bookmarkApi.update(this.trackedBookmark); + } + + // noop for topics + afterModalClose() { + return; + } + + afterSave(bookmarkFormData) { + this.trackedBookmark = bookmarkFormData; + this._syncBookmarks(bookmarkFormData.saveData); + this.topicController.model.set("bookmarking", false); + this.topicController.model.set("bookmarked", true); + this.topicController.model.incrementProperty("bookmarksWereChanged"); + this.topicController.model.appEvents.trigger( + "bookmarks:changed", + bookmarkFormData.saveData, + this.bookmarkModel.attachedTo() + ); + return [this.model.id]; + } + + afterDelete(deleteResponse, bookmarkId) { + this.topicController.model.removeBookmark(bookmarkId); + this.bookmarkModel = this.bookmarkApi.buildNewBookmark( + this.type, + this.model.id + ); + this.trackedBookmark = new BookmarkFormData(this.bookmarkModel); + } + + _syncBookmarks(data) { + if (!this.topicController.bookmarks) { + this.topicController.set("bookmarks", []); + } + + const bookmark = this.topicController.bookmarks.findBy("id", data.id); + if (!bookmark) { + this.topicController.bookmarks.pushObject(Bookmark.create(data)); + } else { + bookmark.reminder_at = data.reminder_at; + bookmark.name = data.name; + bookmark.auto_delete_preference = data.auto_delete_preference; + } + } +} diff --git a/app/assets/javascripts/discourse/app/widgets/bookmark-menu.js b/app/assets/javascripts/discourse/app/widgets/bookmark-menu.js index edff13f595a..49e2b3c97c1 100644 --- a/app/assets/javascripts/discourse/app/widgets/bookmark-menu.js +++ b/app/assets/javascripts/discourse/app/widgets/bookmark-menu.js @@ -4,5 +4,5 @@ import { registerWidgetShim } from "discourse/widgets/render-glimmer"; registerWidgetShim( "bookmark-menu-shim", "div.bookmark-menu-shim", - hbs`` + hbs`` ); diff --git a/spec/system/bookmarks_spec.rb b/spec/system/bookmarks_spec.rb index c71f8abbc13..5c3405e6332 100644 --- a/spec/system/bookmarks_spec.rb +++ b/spec/system/bookmarks_spec.rb @@ -45,7 +45,7 @@ describe "Bookmarking posts and topics", type: :system do bookmark_menu.click_menu_option("tomorrow") expect(topic_page).to have_post_bookmarked(post, with_reminder: true) - expect(page).to have_no_css(".bookmark-menu-content") + expect(page).to have_no_css(".bookmark-menu-content.-expanded") expect(Bookmark.find_by(bookmarkable: post, user: current_user).reminder_at).not_to be_blank end @@ -97,34 +97,24 @@ describe "Bookmarking posts and topics", type: :system do describe "topic level bookmarks" do it "allows the topic to be bookmarked" do topic_page.visit_topic(topic) - topic_page.click_topic_footer_button(:bookmark) - - bookmark_modal.fill_name("something important") - bookmark_modal.save - - expect(topic_page).to have_topic_bookmarked + topic_page.click_topic_bookmark_button + expect(topic_page).to have_topic_bookmarked(topic) expect(Bookmark.exists?(bookmarkable: topic, user: current_user)).to be_truthy end - it "opens the edit bookmark modal from the topic bookmark button if one post is bookmarked" do - bookmark = Fabricate(:bookmark, bookmarkable: post_2, user: current_user) + it "opens the edit bookmark modal from the topic bookmark button and saves edits" do + bookmark = Fabricate(:bookmark, bookmarkable: topic, user: current_user) topic_page.visit_topic(topic) - topic_page.click_topic_footer_button(:bookmark) + topic_page.click_topic_bookmark_button + bookmark_menu.click_menu_option("edit") expect(bookmark_modal).to be_open expect(bookmark_modal).to be_editing_id(bookmark.id) - end + bookmark_modal.fill_name("something important") + bookmark_modal.click_primary_button - it "clears all topic bookmarks from the topic bookmark button if more than one post is bookmarked" do - Fabricate(:bookmark, bookmarkable: post, user: current_user) - Fabricate(:bookmark, bookmarkable: post_2, user: current_user) - topic_page.visit_topic(topic) - topic_page.click_topic_footer_button(:bookmark) - dialog = PageObjects::Components::Dialog.new - expect(dialog).to have_content(I18n.t("js.bookmarks.confirm_clear")) - dialog.click_yes - expect(dialog).to be_closed - expect(topic_page).to have_no_bookmarks - expect(Bookmark.where(user: current_user).count).to eq(0) + try_until_success(frequency: 0.5) do + expect(bookmark.reload.name).to eq("something important") + end end end diff --git a/spec/system/page_objects/pages/topic.rb b/spec/system/page_objects/pages/topic.rb index 603bdd67df9..4c4415154c5 100644 --- a/spec/system/page_objects/pages/topic.rb +++ b/spec/system/page_objects/pages/topic.rb @@ -63,9 +63,7 @@ module PageObjects end def has_post_more_actions?(post) - within post_by_number(post) do - has_css?(".show-more-actions") - end + within_post(post) { has_css?(".show-more-actions") } end def has_post_bookmarked?(post, with_reminder: false) @@ -83,20 +81,20 @@ module PageObjects def click_post_action_button(post, button) case button when :bookmark - post_by_number(post).find(".bookmark").click + within_post(post) { find(".bookmark").click } when :reply - post_by_number(post).find(".post-controls .reply").click + within_post(post) { find(".post-controls .reply").click } when :flag - post_by_number(post).find(".post-controls .create-flag").click + within_post(post) { find(".post-controls .create-flag").click } when :copy_link - post_by_number(post).find(".post-controls .post-action-menu__copy-link").click + within_post(post) { find(".post-controls .post-action-menu__copy-link").click } when :edit - post_by_number(post).find(".post-controls .edit").click + within_post(post) { find(".post-controls .edit").click } end end def expand_post_admin_actions(post) - post_by_number(post).find(".show-post-admin-menu").click + within_post(post) { find(".show-post-admin-menu").click } end def click_post_admin_action_button(post, button) @@ -111,24 +109,22 @@ module PageObjects find(element_klass).click end - def click_topic_footer_button(button) - find_topic_footer_button(button).click + def click_topic_bookmark_button + within_topic_footer_buttons { find(".bookmark-menu-trigger").click } end - def has_topic_bookmarked? - has_css?("#{topic_footer_button_id("bookmark")}.bookmarked", text: "Edit Bookmark") + def has_topic_bookmarked?(topic) + within_topic_footer_buttons do + has_css?(".bookmark-menu-trigger.bookmarked", text: "Edit Bookmark") + end end - def has_no_bookmarks? - has_no_css?("#{topic_footer_button_id("bookmark")}.bookmarked") - end - - def find_topic_footer_button(button) - find(topic_footer_button_id(button)) + def has_no_bookmarks?(topic) + within_topic_footer_buttons { has_no_css?(".bookmark-menu-trigger.bookmarked") } end def click_reply_button - find(".topic-footer-main-buttons > .create").click + within_topic_footer_buttons { find(".create").click } has_expanded_composer? end @@ -186,9 +182,7 @@ module PageObjects end def click_mention(post, mention) - within post_by_number(post) do - find("a.mention-group", text: mention).click - end + within_post(post) { find("a.mention-group", text: mention).click } end def click_footer_reply @@ -197,7 +191,7 @@ module PageObjects end def click_like_reaction_for(post) - post_by_number(post).find(".post-controls .actions .like").click + within_post(post) { find(".post-controls .actions .like").click } end def has_topic_map? @@ -217,7 +211,7 @@ module PageObjects end def click_admin_menu_button - find("#topic-footer-buttons .topic-admin-menu-button").click + within_topic_footer_buttons { find(".topic-admin-menu-button").click } end def watch_topic @@ -236,12 +230,16 @@ module PageObjects private - def topic_footer_button_id(button) - "#topic-footer-button-#{button}" + def within_post(post) + within(post_by_number(post)) { yield } + end + + def within_topic_footer_buttons + within("#topic-footer-buttons") { yield } end def is_post_bookmarked(post, bookmarked:, with_reminder: false) - within post_by_number(post) do + within_post(post) do css_class = ".bookmark.bookmarked#{with_reminder ? ".with-reminder" : ""}" page.public_send(bookmarked ? :has_css? : :has_no_css?, css_class) end