From 5c1147adf3339c89a3ccb69839b4a4c5fd75a2ab Mon Sep 17 00:00:00 2001 From: Kris Date: Tue, 5 Mar 2024 15:43:07 -0500 Subject: [PATCH] UX: Move top dismiss button from `topics` to `d-navigation` (#26032) --- .../discourse/app/components/d-navigation.hbs | 10 ++++ .../app/components/discovery/navigation.hbs | 5 ++ .../app/components/discovery/topics.hbs | 14 +---- .../app/components/discovery/topics.js | 58 ------------------- .../app/components/topic-dismiss-buttons.hbs | 12 ++-- .../app/components/topic-dismiss-buttons.js | 5 +- .../app/controllers/discovery/list.js | 56 ++++++++++++++++++ .../app/templates/discovery/list.hbs | 7 +++ .../acceptance/keyboard-shortcuts-test.js | 12 ++-- .../stylesheets/common/base/_topic-list.scss | 4 ++ app/assets/stylesheets/mobile/topic-list.scss | 4 -- .../components/topic_list_controls.rb | 4 +- 12 files changed, 100 insertions(+), 91 deletions(-) diff --git a/app/assets/javascripts/discourse/app/components/d-navigation.hbs b/app/assets/javascripts/discourse/app/components/d-navigation.hbs index 79ea521488936..50d1f7afe9349 100644 --- a/app/assets/javascripts/discourse/app/components/d-navigation.hbs +++ b/app/assets/javascripts/discourse/app/components/d-navigation.hbs @@ -21,6 +21,16 @@ {{/if}} + + {{#if this.showCategoryAdmin}} {{#if this.fixedCategoryPositions}} {{#if @category}} diff --git a/app/assets/javascripts/discourse/app/components/discovery/topics.hbs b/app/assets/javascripts/discourse/app/components/discovery/topics.hbs index e48fb4e85ed21..9fd91eb2046ca 100644 --- a/app/assets/javascripts/discourse/app/components/discovery/topics.hbs +++ b/app/assets/javascripts/discourse/app/components/discovery/topics.hbs @@ -6,16 +6,6 @@
{{this.redirectedReason}}
{{/if}} - - {{#if @model.sharedDrafts}} topic.id); - const result = await Topic.resetNew( - this.args.category, - !this.args.noSubcategories, - { - tracked: isTracked, - tag: this.args.tag, - topicIds, - dismissPosts, - dismissTopics, - untrack, - } - ); - - if (result.topic_ids) { - this.topicTrackingState.removeTopics(result.topic_ids); - } - this.router.refresh(); - } - - @action - resetNew() { - if (!this.currentUser.new_new_view_enabled) { - return this.callResetNew(); - } - - this.modal.show(DismissNew, { - model: { - selectedTopics: this.args.bulkSelectHelper.selected, - subset: this.args.model.listParams?.subset, - dismissCallback: ({ dismissPosts, dismissTopics, untrack }) => { - this.callResetNew(dismissPosts, dismissTopics, untrack); - }, - }, - }); - } - // Show newly inserted topics @action async showInserted(event) { @@ -227,14 +179,4 @@ export default class DiscoveryTopics extends Component { get expandAllPinned() { return this.args.tag || this.args.category; } - - @action - dismissRead(dismissTopics) { - const operationType = dismissTopics ? "topics" : "posts"; - this.args.bulkSelectHelper.dismissRead(operationType, { - categoryId: this.args.category?.id, - tagName: this.args.tag?.id, - includeSubcategories: this.args.noSubcategories, - }); - } } diff --git a/app/assets/javascripts/discourse/app/components/topic-dismiss-buttons.hbs b/app/assets/javascripts/discourse/app/components/topic-dismiss-buttons.hbs index 75bff4dbdb0b5..a9aa1e91faa40 100644 --- a/app/assets/javascripts/discourse/app/components/topic-dismiss-buttons.hbs +++ b/app/assets/javascripts/discourse/app/components/topic-dismiss-buttons.hbs @@ -1,6 +1,6 @@ -{{#if this.showBasedOnPosition}} +{{~#if this.showBasedOnPosition}}
- {{#if this.showDismissRead}} + {{~#if this.showDismissRead}} - {{/if}} - {{#if this.showResetNew}} + {{/if~}} + {{~#if this.showResetNew}} - {{/if}} + {{/if~}}
-{{/if}} \ No newline at end of file +{{/if~}} \ No newline at end of file diff --git a/app/assets/javascripts/discourse/app/components/topic-dismiss-buttons.js b/app/assets/javascripts/discourse/app/components/topic-dismiss-buttons.js index 27c89aade41a9..5d82fd527ea8c 100644 --- a/app/assets/javascripts/discourse/app/components/topic-dismiss-buttons.js +++ b/app/assets/javascripts/discourse/app/components/topic-dismiss-buttons.js @@ -34,10 +34,9 @@ export default Component.extend({ @discourseComputed("position", "model.topics.length") showBasedOnPosition(position, topicCount) { if (position !== "top") { - return true; + return topicCount > 5; } - - return this.currentUser?.new_new_view_enabled || topicCount > 5; + return true; }, @discourseComputed("selectedTopics.length") diff --git a/app/assets/javascripts/discourse/app/controllers/discovery/list.js b/app/assets/javascripts/discourse/app/controllers/discovery/list.js index 6d9335b0f4c37..e6209f7d0c1a9 100644 --- a/app/assets/javascripts/discourse/app/controllers/discovery/list.js +++ b/app/assets/javascripts/discourse/app/controllers/discovery/list.js @@ -2,10 +2,12 @@ import { tracked } from "@glimmer/tracking"; import Controller from "@ember/controller"; import { action } from "@ember/object"; import { inject as service } from "@ember/service"; +import DismissNew from "discourse/components/modal/dismiss-new"; import BulkSelectHelper from "discourse/lib/bulk-select-helper"; import { filterTypeForMode } from "discourse/lib/filter-mode"; import { disableImplicitInjections } from "discourse/lib/implicit-injections"; import { defineTrackedProperty } from "discourse/lib/tracked-tools"; +import Topic from "discourse/models/topic"; // Just add query params here to have them automatically passed to topic list filters. export const queryParams = { @@ -49,6 +51,9 @@ export default class DiscoveryListController extends Controller { @service siteSettings; @service site; @service currentUser; + @service router; + @service topicTrackingState; + @service modal; @tracked model; @@ -111,6 +116,52 @@ export default class DiscoveryListController extends Controller { return this.order ?? this.model.list.get("params.order") ?? "activity"; } + async callResetNew( + dismissPosts = false, + dismissTopics = false, + untrack = false + ) { + const isTracked = + (this.router.currentRoute.queryParams["f"] || + this.router.currentRoute.queryParams["filter"]) === "tracked"; + + let topicIds = this.bulkSelectHelper.selected.map((topic) => topic.id); + const result = await Topic.resetNew( + this.model.category, + !this.model.noSubcategories, + { + tracked: isTracked, + tag: this.model.tag, + topicIds, + dismissPosts, + dismissTopics, + untrack, + } + ); + + if (result.topic_ids) { + this.topicTrackingState.removeTopics(result.topic_ids); + } + this.router.refresh(); + } + + @action + resetNew() { + if (!this.currentUser.new_new_view_enabled) { + return this.callResetNew(); + } + + this.modal.show(DismissNew, { + model: { + selectedTopics: this.bulkSelectHelper.selected, + subset: this.model.listParams?.subset, + dismissCallback: ({ dismissPosts, dismissTopics, untrack }) => { + this.callResetNew(dismissPosts, dismissTopics, untrack); + }, + }, + }); + } + @action createTopic() { this.composer.openNewTopic({ @@ -148,4 +199,9 @@ export default class DiscoveryListController extends Controller { toggleTagInfo() { this.toggleProperty("showTagInfo"); } + + @action + dismissRead(operationType, options) { + this.bulkSelectHelper.dismissRead(operationType, options); + } } diff --git a/app/assets/javascripts/discourse/app/templates/discovery/list.hbs b/app/assets/javascripts/discourse/app/templates/discovery/list.hbs index 25a981923314e..c0f9520c623d7 100644 --- a/app/assets/javascripts/discourse/app/templates/discovery/list.hbs +++ b/app/assets/javascripts/discourse/app/templates/discovery/list.hbs @@ -16,6 +16,11 @@ @canCreateTopicOnTag={{this.model.canCreateTopicOnTag}} @toggleTagInfo={{this.toggleTagInfo}} @tagNotification={{this.model.tagNotification}} + @model={{this.model.list}} + @showDismissRead={{this.showDismissRead}} + @showResetNew={{this.showResetNew}} + @dismissRead={{this.dismissRead}} + @resetNew={{this.resetNew}} /> @@ -44,6 +49,8 @@ @tag={{this.model.tag}} @changeSort={{this.changeSort}} @changeNewListSubset={{this.changeNewListSubset}} + @dismissRead={{this.dismissRead}} + @resetNew={{this.resetNew}} /> \ No newline at end of file diff --git a/app/assets/javascripts/discourse/tests/acceptance/keyboard-shortcuts-test.js b/app/assets/javascripts/discourse/tests/acceptance/keyboard-shortcuts-test.js index ea868db787d25..c4f5f9903dbb1 100644 --- a/app/assets/javascripts/discourse/tests/acceptance/keyboard-shortcuts-test.js +++ b/app/assets/javascripts/discourse/tests/acceptance/keyboard-shortcuts-test.js @@ -170,7 +170,7 @@ acceptance("Keyboard Shortcuts - Authenticated Users", function (needs) { "mark read has been called on the backend once" ); - // we get rid of all but one topic so the top dismiss button doesn't + // we get rid of all but one topic so the bottom dismiss button doesn't // show up, as it only appears if there are too many topics pushing // the bottom button out of the viewport let originalTopics = [...topicList.topic_list.topics]; @@ -180,8 +180,8 @@ acceptance("Keyboard Shortcuts - Authenticated Users", function (needs) { await visit("/"); await visit("/unread"); assert.notOk( - exists("#dismiss-topics-top"), - "dismiss unread top button is hidden" + exists("#dismiss-topics-bottom"), + "dismiss unread bottom button is hidden" ); await triggerKeyEvent(document, "keypress", "X"); await triggerKeyEvent(document, "keypress", "T"); @@ -216,7 +216,7 @@ acceptance("Keyboard Shortcuts - Authenticated Users", function (needs) { await triggerKeyEvent(document, "keypress", "R"); assert.strictEqual(resetNewCalled, 1); - // we get rid of all but one topic so the top dismiss button doesn't + // we get rid of all but one topic so the bottom dismiss button doesn't // show up, as it only appears if there are too many topics pushing // the bottom button out of the viewport let originalTopics = [...topicList.topic_list.topics]; @@ -226,8 +226,8 @@ acceptance("Keyboard Shortcuts - Authenticated Users", function (needs) { await visit("/"); await visit("/new"); assert.notOk( - exists("#dismiss-new-top"), - "dismiss new top button has been hidden" + exists("#dismiss-new-bottom"), + "dismiss new bottom button has been hidden" ); await triggerKeyEvent(document, "keypress", "X"); await triggerKeyEvent(document, "keypress", "R"); diff --git a/app/assets/stylesheets/common/base/_topic-list.scss b/app/assets/stylesheets/common/base/_topic-list.scss index ceb8212e059c3..505bb2aca147d 100644 --- a/app/assets/stylesheets/common/base/_topic-list.scss +++ b/app/assets/stylesheets/common/base/_topic-list.scss @@ -76,6 +76,10 @@ } } } + + .dismiss-container-top:empty { + display: none; + } } .category-heading { diff --git a/app/assets/stylesheets/mobile/topic-list.scss b/app/assets/stylesheets/mobile/topic-list.scss index 0e9a371a423b8..1ab687519f59b 100644 --- a/app/assets/stylesheets/mobile/topic-list.scss +++ b/app/assets/stylesheets/mobile/topic-list.scss @@ -449,10 +449,6 @@ tr.category-topic-link { margin: 1.5em 0 1em; } -button.dismiss-read { - margin-bottom: 10px; -} - // base defines extra padding for easier click/top of title field // this is a bit too much for mobile td .main-link { diff --git a/spec/system/page_objects/components/topic_list_controls.rb b/spec/system/page_objects/components/topic_list_controls.rb index b10440619b900..e764e2a9fe92f 100644 --- a/spec/system/page_objects/components/topic_list_controls.rb +++ b/spec/system/page_objects/components/topic_list_controls.rb @@ -26,14 +26,14 @@ def has_unread?(count:) end def dismiss_unread(untrack: false) - click_button("dismiss-topics-bottom") + click_button("dismiss-topics-top") find(".dismiss-read-modal__stop-tracking").click if untrack click_button("dismiss-read-confirm") self end def dismiss_new - click_button("dismiss-new-bottom") + click_button("dismiss-new-top") self end end