UX: Move top dismiss button from topics to d-navigation (#26032)

discourse · Mar 5, 2024 · 5c1147a · 5c1147a · discoursebot · Mar 6, 2024
1 parent 0c82798
commit 5c1147a
Show file tree

Hide file tree

Showing 12 changed files with 100 additions and 91 deletions.
diff --git a/app/assets/javascripts/discourse/app/components/d-navigation.hbs b/app/assets/javascripts/discourse/app/components/d-navigation.hbs
@@ -21,6 +21,16 @@
     <BulkSelectToggle @bulkSelectHelper={{@bulkSelectHelper}} />
   {{/if}}
 
+  <TopicDismissButtons
+    @position="top"
+    @selectedTopics={{@bulkSelectHelper.selected}}
+    @model={{@model}}
+    @showResetNew={{@showResetNew}}
+    @showDismissRead={{@showDismissRead}}
+    @resetNew={{@resetNew}}
+    @dismissRead={{@dismissRead}}
+  />
+
   {{#if this.showCategoryAdmin}}
     {{#if this.fixedCategoryPositions}}
       <CategoriesAdminDropdown

diff --git a/app/assets/javascripts/discourse/app/components/discovery/navigation.hbs b/app/assets/javascripts/discourse/app/components/discovery/navigation.hbs
@@ -55,6 +55,11 @@
     @skipCategoriesNavItem={{this.skipCategoriesNavItem}}
     @toggleInfo={{@toggleTagInfo}}
     @tagNotification={{@tagNotification}}
+    @model={{@model}}
+    @showDismissRead={{@showDismissRead}}
+    @showResetNew={{@showResetNew}}
+    @dismissRead={{@dismissRead}}
+    @resetNew={{@resetNew}}
   />
 
   {{#if @category}}

diff --git a/app/assets/javascripts/discourse/app/components/discovery/topics.hbs b/app/assets/javascripts/discourse/app/components/discovery/topics.hbs
@@ -6,16 +6,6 @@
   <div class="alert alert-info">{{this.redirectedReason}}</div>
 {{/if}}
 
-<TopicDismissButtons
-  @position="top"
-  @selectedTopics={{@bulkSelectHelper.selected}}
-  @model={{@model}}
-  @showResetNew={{@showResetNew}}
-  @showDismissRead={{@showDismissRead}}
-  @resetNew={{this.resetNew}}
-  @dismissRead={{this.dismissRead}}
-/>
-
 {{#if @model.sharedDrafts}}
   <TopicList
     @listTitle="shared_drafts.title"
@@ -137,8 +127,8 @@
         @model={{@model}}
         @showResetNew={{@showResetNew}}
         @showDismissRead={{@showDismissRead}}
-        @resetNew={{this.resetNew}}
-        @dismissRead={{this.dismissRead}}
+        @resetNew={{@resetNew}}
+        @dismissRead={{@dismissRead}}
       />
 
       <FooterMessage

diff --git a/app/assets/javascripts/discourse/app/components/discovery/topics.js b/app/assets/javascripts/discourse/app/components/discovery/topics.js
@@ -2,11 +2,9 @@ import Component from "@glimmer/component";
 import { tracked } from "@glimmer/tracking";
 import { action } from "@ember/object";
 import { inject as service } from "@ember/service";
-import DismissNew from "discourse/components/modal/dismiss-new";
 import { popupAjaxError } from "discourse/lib/ajax-error";
 import { filterTypeForMode } from "discourse/lib/filter-mode";
 import { userPath } from "discourse/lib/url";
-import Topic from "discourse/models/topic";
 import I18n from "discourse-i18n";
 
 export default class DiscoveryTopics extends Component {
@@ -55,52 +53,6 @@ export default class DiscoveryTopics extends Component {
     return filterTypeForMode(this.args.model.filter) === "new";
   }
 
-  async callResetNew(
-    dismissPosts = false,
-    dismissTopics = false,
-    untrack = false
-  ) {
-    const isTracked =
-      (this.router.currentRoute.queryParams["f"] ||
-        this.router.currentRoute.queryParams["filter"]) === "tracked";
-
-    let topicIds = this.args.bulkSelectHelper.selected.map((topic) => 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
@@ -1,22 +1,22 @@
-{{#if this.showBasedOnPosition}}
+{{~#if this.showBasedOnPosition}}
   <div class="row {{this.containerClass}}">
-    {{#if this.showDismissRead}}
+    {{~#if this.showDismissRead}}
       <DButton
         @action={{this.dismissReadPosts}}
         @translatedLabel={{this.dismissLabel}}
         @title="topics.bulk.dismiss_tooltip"
         id={{this.dismissReadId}}
         class="btn-default dismiss-read"
       />
-    {{/if}}
-    {{#if this.showResetNew}}
+    {{/if~}}
+    {{~#if this.showResetNew}}
       <DButton
         @action={{this.resetNew}}
         @icon="check"
         @translatedLabel={{this.dismissNewLabel}}
         id={{this.dismissNewId}}
         class="btn-default dismiss-read"
       />
-    {{/if}}
+    {{/if~}}
   </div>
-{{/if}}
+{{/if~}}
diff --git 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
@@ -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
@@ -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}}
     />
   </:navigation>
 
@@ -44,6 +49,8 @@
       @tag={{this.model.tag}}
       @changeSort={{this.changeSort}}
       @changeNewListSubset={{this.changeNewListSubset}}
+      @dismissRead={{this.dismissRead}}
+      @resetNew={{this.resetNew}}
     />
   </:list>
 </Discovery::Layout>
diff --git 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
@@ -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
@@ -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
@@ -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