From 7a79bd7da3e0a59454a3c03f3be31e457d0b9fcc Mon Sep 17 00:00:00 2001 From: Martin Brennan Date: Wed, 26 May 2021 09:38:46 +1000 Subject: [PATCH] FEATURE: Allow selective dismissal of new and unread topics (#12976) This PR improves the UI of bulk select so that its context is applied to the Dismiss Unread and Dismiss New buttons. Regular users (not just staff) are now able to use topic bulk selection on the /new and /unread routes to perform these dismiss actions more selectively. For Dismiss Unread, there is a new count in the text of the button and in the modal when one or more topic is selected with the bulk select checkboxes. For Dismiss New, there is a count in the button text, and we have added functionality to the server side to accept an array of topic ids to dismiss new for, instead of always having to dismiss all new, the same as the bulk dismiss unread functionality. To clean things up, the `DismissTopics` service has been rolled into the `TopicsBulkAction` service. We now also show the top Dismiss/Dismiss New button based on whether the bottom one is in the viewport, not just based on the topic count. --- .../app/components/bulk-select-button.js | 3 + .../app/components/topic-dismiss-buttons.js | 105 ++++++++++++++++++ .../discourse/app/components/watch-read.js | 2 +- .../app/controllers/discovery/topics.js | 52 ++++----- .../discourse/app/controllers/tag-show.js | 38 ++----- .../discourse/app/controllers/topic.js | 2 +- .../app/lib/is-element-in-viewport.js | 4 +- .../discourse/app/lib/show-modal.js | 2 + .../app/mixins/bulk-topic-selection.js | 13 ++- .../javascripts/discourse/app/models/topic.js | 12 +- .../components/bulk-select-button.hbs | 10 +- .../components/topic-dismiss-buttons.hbs | 21 ++++ .../app/templates/discovery/topics.hbs | 40 +------ .../app/templates/mobile/discovery/topics.hbs | 40 +------ .../discourse/app/templates/tags/show.hbs | 46 +------- app/controllers/topics_controller.rb | 22 +++- app/services/dismiss_topics.rb | 45 -------- config/locales/client.en.yml | 3 + lib/topics_bulk_action.rb | 36 +++++- spec/components/topics_bulk_action_spec.rb | 64 +++++++++++ spec/requests/topics_controller_spec.rb | 76 ++++++++++++- spec/services/dismiss_topics_spec.rb | 55 --------- 22 files changed, 399 insertions(+), 292 deletions(-) create mode 100644 app/assets/javascripts/discourse/app/components/topic-dismiss-buttons.js create mode 100644 app/assets/javascripts/discourse/app/templates/components/topic-dismiss-buttons.hbs delete mode 100644 app/services/dismiss_topics.rb delete mode 100644 spec/services/dismiss_topics_spec.rb diff --git a/app/assets/javascripts/discourse/app/components/bulk-select-button.js b/app/assets/javascripts/discourse/app/components/bulk-select-button.js index 9f46112d23d..e3359e1f738 100644 --- a/app/assets/javascripts/discourse/app/components/bulk-select-button.js +++ b/app/assets/javascripts/discourse/app/components/bulk-select-button.js @@ -1,5 +1,6 @@ import Component from "@ember/component"; import { schedule } from "@ember/runloop"; +import { reads } from "@ember/object/computed"; import showModal from "discourse/lib/show-modal"; export default Component.extend({ @@ -17,6 +18,8 @@ export default Component.extend({ }); }, + canDoBulkActions: reads("currentUser.staff"), + actions: { showBulkActions() { const controller = showModal("topic-bulk-actions", { diff --git a/app/assets/javascripts/discourse/app/components/topic-dismiss-buttons.js b/app/assets/javascripts/discourse/app/components/topic-dismiss-buttons.js new file mode 100644 index 00000000000..5092bfe2df8 --- /dev/null +++ b/app/assets/javascripts/discourse/app/components/topic-dismiss-buttons.js @@ -0,0 +1,105 @@ +import { action } from "@ember/object"; +import showModal from "discourse/lib/show-modal"; +import { later } from "@ember/runloop"; +import isElementInViewport from "discourse/lib/is-element-in-viewport"; +import discourseComputed, { on } from "discourse-common/utils/decorators"; +import I18n from "I18n"; +import Component from "@ember/component"; + +export default Component.extend({ + tagName: "", + classNames: ["topic-dismiss-buttons"], + + position: null, + selectedTopics: null, + model: null, + + @discourseComputed("position") + containerClass(position) { + return `dismiss-container-${position}`; + }, + + @discourseComputed("position") + dismissReadId(position) { + return `dismiss-topics-${position}`; + }, + + @discourseComputed("position") + dismissNewId(position) { + return `dismiss-new-${position}`; + }, + + @discourseComputed( + "position", + "isOtherDismissUnreadButtonVisible", + "isOtherDismissNewButtonVisible" + ) + showBasedOnPosition( + position, + isOtherDismissUnreadButtonVisible, + isOtherDismissNewButtonVisible + ) { + if (position !== "top") { + return true; + } + + return !( + isOtherDismissUnreadButtonVisible || isOtherDismissNewButtonVisible + ); + }, + + @discourseComputed("selectedTopics.length") + dismissLabel(selectedTopicCount) { + if (selectedTopicCount === 0) { + return I18n.t("topics.bulk.dismiss_button"); + } + return I18n.t("topics.bulk.dismiss_button_with_selected", { + count: selectedTopicCount, + }); + }, + + @discourseComputed("selectedTopics.length") + dismissNewLabel(selectedTopicCount) { + if (selectedTopicCount === 0) { + return I18n.t("topics.bulk.dismiss_new"); + } + return I18n.t("topics.bulk.dismiss_new_with_selected", { + count: selectedTopicCount, + }); + }, + + // we want to only render the Dismiss... button at the top of the + // page if the user cannot see the bottom Dismiss... button based on their + // viewport, or if too many topics fill the page + @on("didInsertElement") + _determineOtherDismissVisibility() { + later(() => { + if (this.position === "top") { + this.set( + "isOtherDismissUnreadButtonVisible", + isElementInViewport(document.getElementById("dismiss-topics-bottom")) + ); + this.set( + "isOtherDismissNewButtonVisible", + isElementInViewport(document.getElementById("dismiss-new-bottom")) + ); + } else { + this.set("isOtherDismissUnreadButtonVisible", true); + this.set("isOtherDismissNewButtonVisible", true); + } + }); + }, + + @action + dismissReadPosts() { + let dismissTitle = "topics.bulk.dismiss_read"; + if (this.selectedTopics.length > 0) { + dismissTitle = "topics.bulk.dismiss_read_with_selected"; + } + showModal("dismiss-read", { + titleTranslated: I18n.t(dismissTitle, { + count: this.selectedTopics.length, + }), + }); + }, +}); diff --git a/app/assets/javascripts/discourse/app/components/watch-read.js b/app/assets/javascripts/discourse/app/components/watch-read.js index 66030689805..e375f4e183e 100644 --- a/app/assets/javascripts/discourse/app/components/watch-read.js +++ b/app/assets/javascripts/discourse/app/components/watch-read.js @@ -13,7 +13,7 @@ export default Component.extend({ if (path === "faq" || path === "guidelines") { $(window).on("load.faq resize.faq scroll.faq", () => { const faqUnread = !currentUser.get("read_faq"); - if (faqUnread && isElementInViewport($(".contents p").last())) { + if (faqUnread && isElementInViewport($(".contents p").last()[0])) { this.action(); } }); diff --git a/app/assets/javascripts/discourse/app/controllers/discovery/topics.js b/app/assets/javascripts/discourse/app/controllers/discovery/topics.js index 6e33eb20fce..99afc1dedab 100644 --- a/app/assets/javascripts/discourse/app/controllers/discovery/topics.js +++ b/app/assets/javascripts/discourse/app/controllers/discovery/topics.js @@ -18,7 +18,6 @@ import discourseComputed from "discourse-common/utils/decorators"; import { endWith } from "discourse/lib/computed"; import { routeAction } from "discourse/helpers/route-action"; import { inject as service } from "@ember/service"; -import showModal from "discourse/lib/show-modal"; import { userPath } from "discourse/lib/url"; const controllerOpts = { @@ -39,6 +38,18 @@ const controllerOpts = { order: readOnly("model.params.order"), ascending: readOnly("model.params.ascending"), + selected: null, + + @discourseComputed("model.filter", "model.topics.length") + showDismissRead(filter, topicsLength) { + return this._isFilterPage(filter, "unread") && topicsLength > 0; + }, + + @discourseComputed("model.filter", "model.topics.length") + showResetNew(filter, topicsLength) { + return this._isFilterPage(filter, "new") && topicsLength > 0; + }, + actions: { changeSort() { deprecated( @@ -98,17 +109,20 @@ const controllerOpts = { (this.router.currentRoute.queryParams["f"] || this.router.currentRoute.queryParams["filter"]) === "tracked"; - Topic.resetNew(this.category, !this.noSubcategories, tracked).then(() => + let topicIds = this.selected + ? this.selected.map((topic) => topic.id) + : null; + + Topic.resetNew(this.category, !this.noSubcategories, { + tracked, + topicIds, + }).then(() => this.send( "refresh", tracked ? { skipResettingParams: ["filter", "f"] } : {} ) ); }, - - dismissReadPosts() { - showModal("dismiss-read", { title: "topics.bulk.dismiss_read" }); - }, }, afterRefresh(filter, list, listModel = list) { @@ -122,32 +136,6 @@ const controllerOpts = { this.send("loadingComplete"); }, - isFilterPage: function (filter, filterType) { - if (!filter) { - return false; - } - return filter.match(new RegExp(filterType + "$", "gi")) ? true : false; - }, - - @discourseComputed("model.filter", "model.topics.length") - showDismissRead(filter, topicsLength) { - return this.isFilterPage(filter, "unread") && topicsLength > 0; - }, - - @discourseComputed("model.filter", "model.topics.length") - showResetNew(filter, topicsLength) { - return this.isFilterPage(filter, "new") && topicsLength > 0; - }, - - @discourseComputed("model.filter", "model.topics.length") - showDismissAtTop(filter, topicsLength) { - return ( - (this.isFilterPage(filter, "new") || - this.isFilterPage(filter, "unread")) && - topicsLength >= 15 - ); - }, - hasTopics: gt("model.topics.length", 0), allLoaded: empty("model.more_topics_url"), latest: endWith("model.filter", "latest"), diff --git a/app/assets/javascripts/discourse/app/controllers/tag-show.js b/app/assets/javascripts/discourse/app/controllers/tag-show.js index 0bf4cec6a96..76b7d070cb5 100644 --- a/app/assets/javascripts/discourse/app/controllers/tag-show.js +++ b/app/assets/javascripts/discourse/app/controllers/tag-show.js @@ -8,7 +8,6 @@ import Topic from "discourse/models/topic"; import { alias } from "@ember/object/computed"; import bootbox from "bootbox"; import { queryParams } from "discourse/controllers/discovery-sortable"; -import showModal from "discourse/lib/show-modal"; export default Controller.extend(BulkTopicSelection, FilterModeMixin, { application: controller(), @@ -93,48 +92,31 @@ export default Controller.extend(BulkTopicSelection, FilterModeMixin, { } }, - isFilterPage: function (filter, filterType) { - if (!filter) { - return false; - } - return filter.match(new RegExp(filterType + "$", "gi")) ? true : false; - }, - @discourseComputed("list.filter", "list.topics.length") showDismissRead(filter, topicsLength) { - return this.isFilterPage(filter, "unread") && topicsLength > 0; + return this._isFilterPage(filter, "unread") && topicsLength > 0; }, @discourseComputed("list.filter", "list.topics.length") showResetNew(filter, topicsLength) { - return this.isFilterPage(filter, "new") && topicsLength > 0; - }, - - @discourseComputed("list.filter", "list.topics.length") - showDismissAtTop(filter, topicsLength) { - return ( - (this.isFilterPage(filter, "new") || - this.isFilterPage(filter, "unread")) && - topicsLength >= 15 - ); + return this._isFilterPage(filter, "new") && topicsLength > 0; }, actions: { - dismissReadPosts() { - showModal("dismiss-read", { title: "topics.bulk.dismiss_read" }); - }, - resetNew() { const tracked = (this.router.currentRoute.queryParams["f"] || this.router.currentRoute.queryParams["filter"]) === "tracked"; - Topic.resetNew( - this.category, - !this.noSubcategories, + let topicIds = this.selected + ? this.selected.map((topic) => topic.id) + : null; + + Topic.resetNew(this.category, !this.noSubcategories, { tracked, - this.tag - ).then(() => + tag: this.tag, + topicIds, + }).then(() => this.send( "refresh", tracked ? { skipResettingParams: ["filter", "f"] } : {} diff --git a/app/assets/javascripts/discourse/app/controllers/topic.js b/app/assets/javascripts/discourse/app/controllers/topic.js index 6699af2c537..c6a5289b2b6 100644 --- a/app/assets/javascripts/discourse/app/controllers/topic.js +++ b/app/assets/javascripts/discourse/app/controllers/topic.js @@ -1639,7 +1639,7 @@ export default Controller.extend(bufferedProperty("model"), { function () { const $post = $(`.topic-post article#post_${postNumber}`); - if ($post.length === 0 || isElementInViewport($post)) { + if ($post.length === 0 || isElementInViewport($post[0])) { return; } diff --git a/app/assets/javascripts/discourse/app/lib/is-element-in-viewport.js b/app/assets/javascripts/discourse/app/lib/is-element-in-viewport.js index 5bac8fafc83..21d725fd32b 100644 --- a/app/assets/javascripts/discourse/app/lib/is-element-in-viewport.js +++ b/app/assets/javascripts/discourse/app/lib/is-element-in-viewport.js @@ -1,6 +1,6 @@ export default function (element) { - if (element instanceof jQuery) { - element = element[0]; + if (!element) { + return; } const $window = $(window), diff --git a/app/assets/javascripts/discourse/app/lib/show-modal.js b/app/assets/javascripts/discourse/app/lib/show-modal.js index f2602e01969..db3067fbcf5 100644 --- a/app/assets/javascripts/discourse/app/lib/show-modal.js +++ b/app/assets/javascripts/discourse/app/lib/show-modal.js @@ -41,6 +41,8 @@ export default function (name, opts) { route.render(fullName, renderArgs); if (opts.title) { modalController.set("title", I18n.t(opts.title)); + } else if (opts.titleTranslated) { + modalController.set("title", opts.titleTranslated); } else { modalController.set("title", null); } diff --git a/app/assets/javascripts/discourse/app/mixins/bulk-topic-selection.js b/app/assets/javascripts/discourse/app/mixins/bulk-topic-selection.js index 7ba734c65da..f2a11771ee2 100644 --- a/app/assets/javascripts/discourse/app/mixins/bulk-topic-selection.js +++ b/app/assets/javascripts/discourse/app/mixins/bulk-topic-selection.js @@ -1,8 +1,8 @@ import Mixin from "@ember/object/mixin"; +import { or } from "@ember/object/computed"; +import { on } from "discourse-common/utils/decorators"; import { NotificationLevels } from "discourse/lib/notification-levels"; import Topic from "discourse/models/topic"; -import { alias } from "@ember/object/computed"; -import { on } from "discourse-common/utils/decorators"; import { inject as service } from "@ember/service"; export default Mixin.create({ @@ -12,13 +12,20 @@ export default Mixin.create({ autoAddTopicsToBulkSelect: false, selected: null, - canBulkSelect: alias("currentUser.staff"), + canBulkSelect: or("currentUser.staff", "showDismissRead", "showResetNew"), @on("init") resetSelected() { this.set("selected", []); }, + _isFilterPage(filter, filterType) { + if (!filter) { + return false; + } + return new RegExp(filterType + "$", "gi").test(filter); + }, + actions: { toggleBulkSelect() { this.toggleProperty("bulkSelectEnabled"); diff --git a/app/assets/javascripts/discourse/app/models/topic.js b/app/assets/javascripts/discourse/app/models/topic.js index 37be80796ce..822caf52d98 100644 --- a/app/assets/javascripts/discourse/app/models/topic.js +++ b/app/assets/javascripts/discourse/app/models/topic.js @@ -756,7 +756,14 @@ Topic.reopenClass({ }); }, - resetNew(category, include_subcategories, tracked = false, tag = false) { + resetNew(category, include_subcategories, opts = {}) { + let { tracked, tag, topicIds } = { + tracked: false, + tag: null, + topicIds: null, + ...opts, + }; + const data = { tracked }; if (category) { data.category_id = category.id; @@ -765,6 +772,9 @@ Topic.reopenClass({ if (tag) { data.tag_id = tag.id; } + if (topicIds) { + data.topic_ids = topicIds; + } return ajax("/topics/reset-new", { type: "PUT", data }); }, diff --git a/app/assets/javascripts/discourse/app/templates/components/bulk-select-button.hbs b/app/assets/javascripts/discourse/app/templates/components/bulk-select-button.hbs index bdc803ae479..77306f93e11 100644 --- a/app/assets/javascripts/discourse/app/templates/components/bulk-select-button.hbs +++ b/app/assets/javascripts/discourse/app/templates/components/bulk-select-button.hbs @@ -1,5 +1,7 @@ -{{#if selected}} -
- {{d-button class="btn-default bulk-select-btn" action=(action "showBulkActions") icon="wrench"}} -
+{{#if canDoBulkActions}} + {{#if selected}} +
+ {{d-button class="btn-default bulk-select-btn" action=(action "showBulkActions") icon="wrench"}} +
+ {{/if}} {{/if}} diff --git a/app/assets/javascripts/discourse/app/templates/components/topic-dismiss-buttons.hbs b/app/assets/javascripts/discourse/app/templates/components/topic-dismiss-buttons.hbs new file mode 100644 index 00000000000..32d9e25d5d4 --- /dev/null +++ b/app/assets/javascripts/discourse/app/templates/components/topic-dismiss-buttons.hbs @@ -0,0 +1,21 @@ +{{#if showBasedOnPosition}} +
+ {{#if showDismissRead}} + {{d-button + class="btn-default dismiss-read" + id=dismissReadId + action=(action "dismissReadPosts") + translatedLabel=dismissLabel + title="topics.bulk.dismiss_tooltip"}} + {{/if}} + {{#if showResetNew}} + {{d-button + class="btn-default dismiss-read" + id=dismissNewId + action=resetNew + icon="check" + translatedLabel=dismissNewLabel}} + {{/if}} +
+{{/if}} + diff --git a/app/assets/javascripts/discourse/app/templates/discovery/topics.hbs b/app/assets/javascripts/discourse/app/templates/discovery/topics.hbs index ca4e8cbe82c..3e9e8d38757 100644 --- a/app/assets/javascripts/discourse/app/templates/discovery/topics.hbs +++ b/app/assets/javascripts/discourse/app/templates/discovery/topics.hbs @@ -2,26 +2,8 @@
{{redirectedReason}}
{{/if}} -{{#if showDismissAtTop}} -
- {{#if showDismissRead}} - {{d-button - class="btn-default dismiss-read" - id="dismiss-topics-top" - action=(action "dismissReadPosts") - title="topics.bulk.dismiss_tooltip" - label="topics.bulk.dismiss_button"}} - {{/if}} - {{#if showResetNew}} - {{d-button - class="btn-default dismiss-read" - id="dismiss-new-top" - action=(action "resetNew") - icon="check" - label="topics.bulk.dismiss_new"}} - {{/if}} -
-{{/if}} +{{topic-dismiss-buttons position="top" selectedTopics=selected +model=model showResetNew=showResetNew showDismissRead=showDismissRead resetNew=(action "resetNew")}} {{#if model.sharedDrafts}} {{topic-list @@ -89,22 +71,8 @@