From e0155b6955e90c53b7d2b75d8efd31a1d204398e Mon Sep 17 00:00:00 2001 From: Alan Guo Xiang Tan Date: Mon, 27 Mar 2023 10:08:11 +0800 Subject: [PATCH] DEV: Introduce dedicated controller and route for `discovery.filter` (#20837) Instead of being tied to the old implementation and constraints, a dedicated route and controller for the `discovery.filter` app route will allow us to iterate on changes much faster. --- .../discourse/app/components/d-navigation.hbs | 44 ++++++--------- .../discourse/app/components/d-navigation.js | 9 ---- .../app/controllers/discovery-filter.js | 33 ++++++++++++ .../app/controllers/discovery-sortable.js | 27 ---------- .../app/controllers/navigation/default.js | 1 - .../app/controllers/navigation/filter.js | 12 +++++ .../dynamic-route-builders.js | 7 --- .../discourse/app/routes/discovery-filter.js | 53 +++++++++++++++++++ .../app/templates/navigation/default.hbs | 2 - .../app/templates/navigation/filter.hbs | 20 +++++++ 10 files changed, 133 insertions(+), 75 deletions(-) create mode 100644 app/assets/javascripts/discourse/app/controllers/discovery-filter.js create mode 100644 app/assets/javascripts/discourse/app/controllers/navigation/filter.js create mode 100644 app/assets/javascripts/discourse/app/routes/discovery-filter.js create mode 100644 app/assets/javascripts/discourse/app/templates/navigation/filter.hbs diff --git a/app/assets/javascripts/discourse/app/components/d-navigation.hbs b/app/assets/javascripts/discourse/app/components/d-navigation.hbs index 95098c83b71..e91c42db8ba 100644 --- a/app/assets/javascripts/discourse/app/components/d-navigation.hbs +++ b/app/assets/javascripts/discourse/app/components/d-navigation.hbs @@ -1,33 +1,19 @@ -{{#if this.isQueryFilterMode}} -
-
- {{d-icon "filter" class="topic-query-filter__icon"}} - -
-
-{{else}} - + - {{#unless this.additionalTags}} - {{! nav bar doesn't work with tag intersections }} - - {{/unless}} -{{/if}} +{{#unless this.additionalTags}} + {{! nav bar doesn't work with tag intersections }} + +{{/unless}}
{{#if (and this.notCategoriesRoute this.site.mobileView this.canBulk)}} diff --git a/app/assets/javascripts/discourse/app/components/d-navigation.js b/app/assets/javascripts/discourse/app/components/d-navigation.js index eaa0d47fbef..73e63a5d21e 100644 --- a/app/assets/javascripts/discourse/app/components/d-navigation.js +++ b/app/assets/javascripts/discourse/app/components/d-navigation.js @@ -6,18 +6,11 @@ import { NotificationLevels } from "discourse/lib/notification-levels"; import { getOwner } from "discourse-common/lib/get-owner"; import { htmlSafe } from "@ember/template"; import { inject as service } from "@ember/service"; -import { equal } from "@ember/object/computed"; export default Component.extend(FilterModeMixin, { router: service(), dialog: service(), tagName: "", - queryString: "", - - init() { - this._super(...arguments); - this.queryString = this.filterQueryString; - }, // Should be a `readOnly` instead but some themes/plugins still pass // the `categories` property into this component @@ -147,8 +140,6 @@ export default Component.extend(FilterModeMixin, { return controller.canBulkSelect; }, - isQueryFilterMode: equal("filterMode", "filter"), - actions: { changeCategoryNotificationLevel(notificationLevel) { this.category.setNotification(notificationLevel); diff --git a/app/assets/javascripts/discourse/app/controllers/discovery-filter.js b/app/assets/javascripts/discourse/app/controllers/discovery-filter.js new file mode 100644 index 00000000000..983fe838853 --- /dev/null +++ b/app/assets/javascripts/discourse/app/controllers/discovery-filter.js @@ -0,0 +1,33 @@ +import Controller from "@ember/controller"; +import { action } from "@ember/object"; +import { tracked } from "@glimmer/tracking"; + +export default class extends Controller { + @tracked status = ""; + + queryParams = ["status"]; + + get queryString() { + let paramStrings = []; + + this.queryParams.forEach((key) => { + if (this[key]) { + paramStrings.push(`${key}:${this[key]}`); + } + }); + + return paramStrings.join(" "); + } + + @action + updateTopicsListQueryParams(queryString) { + for (const match of queryString.matchAll(/(\w+):([^:\s]+)/g)) { + const key = match[1]; + const value = match[2]; + + if (this.queryParams.includes(key)) { + this.set(key, value); + } + } + } +} diff --git a/app/assets/javascripts/discourse/app/controllers/discovery-sortable.js b/app/assets/javascripts/discourse/app/controllers/discovery-sortable.js index 7885da4f34a..6730ed46ef7 100644 --- a/app/assets/javascripts/discourse/app/controllers/discovery-sortable.js +++ b/app/assets/javascripts/discourse/app/controllers/discovery-sortable.js @@ -1,6 +1,4 @@ import Controller, { inject as controller } from "@ember/controller"; -import discourseComputed from "discourse-common/utils/decorators"; -import { action } from "@ember/object"; // Just add query params here to have them automatically passed to topic list filters. export const queryParams = { @@ -29,31 +27,6 @@ export const queryParams = { const controllerOpts = { discoveryTopics: controller("discovery/topics"), queryParams: Object.keys(queryParams), - - @discourseComputed(...Object.keys(queryParams)) - queryString() { - let paramStrings = []; - - this.queryParams.forEach((key) => { - if (this[key]) { - paramStrings.push(`${key}:${this[key]}`); - } - }); - - return paramStrings.join(" "); - }, - - @action - updateTopicsListQueryParams(queryString) { - for (const match of queryString.matchAll(/(\w+):([^:\s]+)/g)) { - const key = match[1]; - const value = match[2]; - - if (controllerOpts.queryParams.includes(key)) { - this.set(key, value); - } - } - }, }; // Default to `undefined` diff --git a/app/assets/javascripts/discourse/app/controllers/navigation/default.js b/app/assets/javascripts/discourse/app/controllers/navigation/default.js index 18e62a93709..2e46e7abb17 100644 --- a/app/assets/javascripts/discourse/app/controllers/navigation/default.js +++ b/app/assets/javascripts/discourse/app/controllers/navigation/default.js @@ -6,7 +6,6 @@ import { TRACKED_QUERY_PARAM_VALUE } from "discourse/lib/topic-list-tracked-filt export default Controller.extend(FilterModeMixin, { discovery: controller(), - discoveryFilter: controller("discovery.filter"), router: service(), @discourseComputed("router.currentRoute.queryParams.f") diff --git a/app/assets/javascripts/discourse/app/controllers/navigation/filter.js b/app/assets/javascripts/discourse/app/controllers/navigation/filter.js new file mode 100644 index 00000000000..195d124ba57 --- /dev/null +++ b/app/assets/javascripts/discourse/app/controllers/navigation/filter.js @@ -0,0 +1,12 @@ +import Controller, { inject as controller } from "@ember/controller"; + +export default class extends Controller { + @controller("discovery/filter") discoveryFilter; + + queryString = ""; + + constructor() { + super(...arguments); + this.queryString = this.discoveryFilter.queryString; + } +} diff --git a/app/assets/javascripts/discourse/app/pre-initializers/dynamic-route-builders.js b/app/assets/javascripts/discourse/app/pre-initializers/dynamic-route-builders.js index 99ea7796dd9..c218efac68d 100644 --- a/app/assets/javascripts/discourse/app/pre-initializers/dynamic-route-builders.js +++ b/app/assets/javascripts/discourse/app/pre-initializers/dynamic-route-builders.js @@ -11,13 +11,6 @@ export default { name: "dynamic-route-builders", initialize(_container, app) { - app.register( - "controller:discovery.filter", - DiscoverySortableController.extend() - ); - - app.register("route:discovery.filter", buildTopicRoute("filter")); - app.register( "controller:discovery.category", DiscoverySortableController.extend() diff --git a/app/assets/javascripts/discourse/app/routes/discovery-filter.js b/app/assets/javascripts/discourse/app/routes/discovery-filter.js new file mode 100644 index 00000000000..144f9c8e39e --- /dev/null +++ b/app/assets/javascripts/discourse/app/routes/discovery-filter.js @@ -0,0 +1,53 @@ +import I18n from "I18n"; + +import DiscourseRoute from "discourse/routes/discourse"; +import { isEmpty } from "@ember/utils"; +import { action } from "@ember/object"; + +export default class extends DiscourseRoute { + queryParams = { + status: { replace: true, refreshModel: true }, + }; + + model(data) { + return this.store.findFiltered("topicList", { + filter: "filter", + params: this.#filterQueryParams(data), + }); + } + + titleToken() { + const filterText = I18n.t("filters.filter.title"); + return I18n.t("filters.with_topics", { filter: filterText }); + } + + setupController(_controller, model) { + this.controllerFor("discovery/topics").setProperties({ model }); + } + + renderTemplate() { + this.render("navigation/filter", { outlet: "navigation-bar" }); + + this.render("discovery/topics", { + controller: "discovery/topics", + outlet: "list-container", + }); + } + + // TODO(tgxworld): This action is required by the `discovery/topics` controller which is not necessary for this route. + // Figure out a way to remove this. + @action + changeSort() {} + + #filterQueryParams(data) { + const params = {}; + + Object.keys(this.queryParams).forEach((key) => { + if (!isEmpty(data[key])) { + params[key] = data[key]; + } + }); + + return params; + } +} diff --git a/app/assets/javascripts/discourse/app/templates/navigation/default.hbs b/app/assets/javascripts/discourse/app/templates/navigation/default.hbs index cfa4ee3536b..9848aadf5fd 100644 --- a/app/assets/javascripts/discourse/app/templates/navigation/default.hbs +++ b/app/assets/javascripts/discourse/app/templates/navigation/default.hbs @@ -9,7 +9,5 @@ @hasDraft={{this.currentUser.has_topic_draft}} @createTopic={{route-action "createTopic"}} @skipCategoriesNavItem={{this.skipCategoriesNavItem}} - @filterQueryString={{this.discoveryFilter.queryString}} - @updateTopicsListQueryParams={{this.discoveryFilter.updateTopicsListQueryParams}} /> \ No newline at end of file diff --git a/app/assets/javascripts/discourse/app/templates/navigation/filter.hbs b/app/assets/javascripts/discourse/app/templates/navigation/filter.hbs new file mode 100644 index 00000000000..6729f1a1785 --- /dev/null +++ b/app/assets/javascripts/discourse/app/templates/navigation/filter.hbs @@ -0,0 +1,20 @@ + +
+
+ {{d-icon "filter" class="topic-query-filter__icon"}} + +
+
+ \ No newline at end of file