From cd0f9121593d8f0aada2eec42e8754aabce90124 Mon Sep 17 00:00:00 2001 From: Vinoth Kannan Date: Wed, 1 Jun 2022 13:43:15 +0530 Subject: [PATCH] FIX: Make f query param sticky when navigating between nav items (#16714) Also, hides categories navigation link when f query param is present. --- .../discourse/app/components/d-navigation.js | 7 ++- .../app/components/navigation-item.js | 10 +-- .../app/controllers/navigation/default.js | 9 +++ .../app/lib/topic-list-tracked-filter.js | 7 ++- .../discourse/app/models/nav-item.js | 18 ++++-- .../app/templates/navigation/default.hbs | 3 +- .../topic-discovery-tracked-test.js | 62 +++++++++++++++++++ 7 files changed, 104 insertions(+), 12 deletions(-) diff --git a/app/assets/javascripts/discourse/app/components/d-navigation.js b/app/assets/javascripts/discourse/app/components/d-navigation.js index 1bbb7f4d0f..266bbf7c4d 100644 --- a/app/assets/javascripts/discourse/app/components/d-navigation.js +++ b/app/assets/javascripts/discourse/app/components/d-navigation.js @@ -107,14 +107,16 @@ export default Component.extend(FilterModeMixin, { "category", "noSubcategories", "tag.id", - "router.currentRoute.queryParams" + "router.currentRoute.queryParams", + "skipCategoriesNavItem" ) navItems( filterType, category, noSubcategories, tagId, - currentRouteQueryParams + currentRouteQueryParams, + skipCategoriesNavItem ) { return NavItem.buildList(category, { filterType, @@ -122,6 +124,7 @@ export default Component.extend(FilterModeMixin, { currentRouteQueryParams, tagId, siteSettings: this.siteSettings, + skipCategoriesNavItem, }); }, diff --git a/app/assets/javascripts/discourse/app/components/navigation-item.js b/app/assets/javascripts/discourse/app/components/navigation-item.js index 539494b225..e50001bba5 100644 --- a/app/assets/javascripts/discourse/app/components/navigation-item.js +++ b/app/assets/javascripts/discourse/app/components/navigation-item.js @@ -56,10 +56,12 @@ export default Component.extend(FilterModeMixin, { // If no query param is present, add an empty one to ensure a ? is // appended to the URL. if (content.currentRouteQueryParams) { - if (content.currentRouteQueryParams.filter) { - if (queryParams.length === 0) { - queryParams.push(""); - } + if (content.currentRouteQueryParams.filter && queryParams.length === 0) { + queryParams.push(""); + } + + if (content.currentRouteQueryParams.f) { + queryParams.push(`f=${content.currentRouteQueryParams.f}`); } } diff --git a/app/assets/javascripts/discourse/app/controllers/navigation/default.js b/app/assets/javascripts/discourse/app/controllers/navigation/default.js index 2e6fd002a2..b9af541cdf 100644 --- a/app/assets/javascripts/discourse/app/controllers/navigation/default.js +++ b/app/assets/javascripts/discourse/app/controllers/navigation/default.js @@ -1,6 +1,15 @@ import Controller, { inject as controller } from "@ember/controller"; import FilterModeMixin from "discourse/mixins/filter-mode"; +import discourseComputed from "discourse-common/utils/decorators"; +import { inject as service } from "@ember/service"; +import { TRACKED_QUERY_PARAM_VALUE } from "discourse/lib/topic-list-tracked-filter"; export default Controller.extend(FilterModeMixin, { discovery: controller(), + router: service(), + + @discourseComputed("router.currentRoute.queryParams.f") + skipCategoriesNavItem(filterParamValue) { + return !filterParamValue || filterParamValue !== TRACKED_QUERY_PARAM_VALUE; + }, }); diff --git a/app/assets/javascripts/discourse/app/lib/topic-list-tracked-filter.js b/app/assets/javascripts/discourse/app/lib/topic-list-tracked-filter.js index 446fbabf0e..51f7476311 100644 --- a/app/assets/javascripts/discourse/app/lib/topic-list-tracked-filter.js +++ b/app/assets/javascripts/discourse/app/lib/topic-list-tracked-filter.js @@ -1,12 +1,17 @@ import Site from "discourse/models/site"; import User from "discourse/models/user"; +export const TRACKED_QUERY_PARAM_VALUE = "tracked"; + export function hasTrackedFilter(queryParams) { if (!queryParams) { return false; } - return queryParams.f === "tracked" || queryParams.filter === "tracked"; + return ( + queryParams.f === TRACKED_QUERY_PARAM_VALUE || + queryParams.filter === TRACKED_QUERY_PARAM_VALUE + ); } /** diff --git a/app/assets/javascripts/discourse/app/models/nav-item.js b/app/assets/javascripts/discourse/app/models/nav-item.js index c61cd7be07..71f61e11d4 100644 --- a/app/assets/javascripts/discourse/app/models/nav-item.js +++ b/app/assets/javascripts/discourse/app/models/nav-item.js @@ -236,10 +236,20 @@ NavItem.reopenClass({ items = items .map((i) => NavItem.fromText(i, args)) - .filter( - (i) => - i !== null && !(category && i.get("name").indexOf("categor") === 0) - ); + .filter((i) => { + if (i === null) { + return false; + } + + if ( + (category || !args.skipCategoriesNavItem) && + i.name.indexOf("categor") === 0 + ) { + return false; + } + + return true; + }); const context = { category: args.category, diff --git a/app/assets/javascripts/discourse/app/templates/navigation/default.hbs b/app/assets/javascripts/discourse/app/templates/navigation/default.hbs index df1f1fa5e7..27fb2beb5a 100644 --- a/app/assets/javascripts/discourse/app/templates/navigation/default.hbs +++ b/app/assets/javascripts/discourse/app/templates/navigation/default.hbs @@ -3,5 +3,6 @@ filterMode=filterMode canCreateTopic=canCreateTopic hasDraft=currentUser.has_topic_draft - createTopic=(route-action "createTopic")}} + createTopic=(route-action "createTopic") + skipCategoriesNavItem=skipCategoriesNavItem}} {{/d-section}} diff --git a/app/assets/javascripts/discourse/tests/acceptance/topic-discovery-tracked-test.js b/app/assets/javascripts/discourse/tests/acceptance/topic-discovery-tracked-test.js index 8f91d708f7..357a5dd7c3 100644 --- a/app/assets/javascripts/discourse/tests/acceptance/topic-discovery-tracked-test.js +++ b/app/assets/javascripts/discourse/tests/acceptance/topic-discovery-tracked-test.js @@ -4,6 +4,7 @@ import { test } from "qunit"; import { acceptance, + exists, publishToMessageBus, query, } from "discourse/tests/helpers/qunit-helpers"; @@ -38,6 +39,67 @@ acceptance("Topic Discovery Tracked", function (needs) { }); }); + test("navigation items with tracked filter", async function (assert) { + this.container.lookup("topic-tracking-state:main").loadStates([ + { + topic_id: 1, + highest_post_number: 1, + last_read_post_number: null, + created_at: "2022-05-11T03:09:31.959Z", + category_id: 1, + notification_level: null, + created_in_new_period: true, + unread_not_too_old: true, + treat_as_new_topic_start_date: "2022-05-09T03:17:34.286Z", + }, + { + topic_id: 2, + highest_post_number: 12, + last_read_post_number: 11, + created_at: "2020-02-09T09:40:02.672Z", + category_id: 2, + notification_level: NotificationLevels.TRACKING, + created_in_new_period: false, + unread_not_too_old: true, + treat_as_new_topic_start_date: "2022-05-09T03:17:34.286Z", + }, + ]); + + await visit("/"); + + assert.ok( + exists("#navigation-bar li.categories"), + "the categories nav item is displayed when tracked filter is not present" + ); + + await visit("/?f=tracked"); + + assert.ok( + !exists("#navigation-bar li.categories"), + "the categories nav item is not displayed when tracked filter is present" + ); + + assert.ok( + query("#navigation-bar li.unread a").href.endsWith("/unread?f=tracked"), + "unread link has tracked filter" + ); + + assert.ok( + query("#navigation-bar li.new a").href.endsWith("/new?f=tracked"), + "new link has tracked filter" + ); + + assert.ok( + query("#navigation-bar li.top a").href.endsWith("/top?f=tracked"), + "top link has tracked filter" + ); + + assert.ok( + query("#navigation-bar li.latest a").href.endsWith("/latest?f=tracked"), + "latest link has tracked filter" + ); + }); + test("visit discovery pages with tracked filter", async function (assert) { const categories = Site.current().categories;