From 804274af47bb01e5bf91f3e1ca5a0550495740e1 Mon Sep 17 00:00:00 2001 From: Andrei Prigorshnev Date: Thu, 16 Sep 2021 22:24:27 +0400 Subject: [PATCH] FEATURE: improve blank page syndrome on the activity/topics, activity/read and group messages pages (#14313) --- .../app/controllers/user-topics-list.js | 6 ++-- .../app/routes/build-group-messages-route.js | 16 ++++++++++- .../routes/build-private-messages-route.js | 12 ++++++-- .../app/routes/user-activity-read.js | 28 +++++++++++++++++-- .../app/routes/user-activity-topics.js | 25 ++++++++++++++--- .../app/templates/user-topics-list.hbs | 6 ++-- .../discourse/tests/acceptance/group-test.js | 7 +++-- .../acceptance/user-activity-read-test.js | 24 ++++++++++++++++ .../acceptance/user-activity-topic-test.js | 24 ++++++++++++++++ config/locales/client.en.yml | 7 +++++ 10 files changed, 136 insertions(+), 19 deletions(-) create mode 100644 app/assets/javascripts/discourse/tests/acceptance/user-activity-read-test.js create mode 100644 app/assets/javascripts/discourse/tests/acceptance/user-activity-topic-test.js diff --git a/app/assets/javascripts/discourse/app/controllers/user-topics-list.js b/app/assets/javascripts/discourse/app/controllers/user-topics-list.js index 55a9f771981..c884703e733 100644 --- a/app/assets/javascripts/discourse/app/controllers/user-topics-list.js +++ b/app/assets/javascripts/discourse/app/controllers/user-topics-list.js @@ -20,9 +20,9 @@ export default Controller.extend(BulkTopicSelection, { tagsForUser: null, incomingCount: reads("pmTopicTrackingState.newIncoming.length"), - @discourseComputed("emptyState", "model.topics.length", "incomingCount") - showEmptyStatePlaceholder(emptyState, topicsLength, incomingCount) { - return emptyState && topicsLength === 0 && incomingCount === 0; + @discourseComputed("model.topics.length", "incomingCount") + noContent(topicsLength, incomingCount) { + return topicsLength === 0 && incomingCount === 0; }, saveScrollPosition() { diff --git a/app/assets/javascripts/discourse/app/routes/build-group-messages-route.js b/app/assets/javascripts/discourse/app/routes/build-group-messages-route.js index cf4266910d3..c366a4eb830 100644 --- a/app/assets/javascripts/discourse/app/routes/build-group-messages-route.js +++ b/app/assets/javascripts/discourse/app/routes/build-group-messages-route.js @@ -14,7 +14,14 @@ export default (type) => { if (this._isArchive()) { filter = `${filter}/archive`; } - return this.store.findFiltered("topicList", { filter }); + return this.store.findFiltered("topicList", { filter }).then((model) => { + // andrei: we agreed that this is an anti pattern, + // it's better to avoid mutating a rest model like this + // this place we'll be refactored later + // see https://github.com/discourse/discourse/pull/14313#discussion_r708784704 + model.set("emptyState", this.emptyState()); + return model; + }); }, setupController() { @@ -40,6 +47,13 @@ export default (type) => { }); }, + emptyState() { + return { + title: I18n.t("no_group_messages_title"), + body: "", + }; + }, + _isArchive() { return type === "archive"; }, diff --git a/app/assets/javascripts/discourse/app/routes/build-private-messages-route.js b/app/assets/javascripts/discourse/app/routes/build-private-messages-route.js index 65a3cddd621..e88c5a42522 100644 --- a/app/assets/javascripts/discourse/app/routes/build-private-messages-route.js +++ b/app/assets/javascripts/discourse/app/routes/build-private-messages-route.js @@ -40,7 +40,16 @@ export default (inboxType, path, filter) => { return lastTopicList ? lastTopicList - : this.store.findFiltered("topicList", { filter: topicListFilter }); + : this.store + .findFiltered("topicList", { filter: topicListFilter }) + .then((model) => { + // andrei: we agreed that this is an anti pattern, + // it's better to avoid mutating a rest model like this + // this place we'll be refactored later + // see https://github.com/discourse/discourse/pull/14313#discussion_r708784704 + model.set("emptyState", this.emptyState()); + return model; + }); }, setupController() { @@ -61,7 +70,6 @@ export default (inboxType, path, filter) => { filter: filter, group: null, inbox: inboxType, - emptyState: this.emptyState(), }); userTopicsListController.subscribe(); diff --git a/app/assets/javascripts/discourse/app/routes/user-activity-read.js b/app/assets/javascripts/discourse/app/routes/user-activity-read.js index 7622ea03ce6..cceb733f7d0 100644 --- a/app/assets/javascripts/discourse/app/routes/user-activity-read.js +++ b/app/assets/javascripts/discourse/app/routes/user-activity-read.js @@ -1,14 +1,36 @@ import UserAction from "discourse/models/user-action"; import UserTopicListRoute from "discourse/routes/user-topic-list"; import { action } from "@ember/object"; +import { iconHTML } from "discourse-common/lib/icon-library"; +import getURL from "discourse-common/lib/get-url"; +import I18n from "I18n"; export default UserTopicListRoute.extend({ userActionType: UserAction.TYPES.topics, model() { - return this.store.findFiltered("topicList", { - filter: "read", - }); + return this.store + .findFiltered("topicList", { + filter: "read", + }) + .then((model) => { + // andrei: we agreed that this is an anti pattern, + // it's better to avoid mutating a rest model like this + // this place we'll be refactored later + // see https://github.com/discourse/discourse/pull/14313#discussion_r708784704 + model.set("emptyState", this.emptyState()); + return model; + }); + }, + + emptyState() { + const title = I18n.t("user_activity.no_read_topics_title"); + const body = I18n.t("user_activity.no_read_topics_body", { + topUrl: getURL("/top"), + categoriesUrl: getURL("/categories"), + searchIcon: iconHTML("search"), + }).htmlSafe(); + return { title, body }; }, @action diff --git a/app/assets/javascripts/discourse/app/routes/user-activity-topics.js b/app/assets/javascripts/discourse/app/routes/user-activity-topics.js index 7a08b365cf0..bea03cd389f 100644 --- a/app/assets/javascripts/discourse/app/routes/user-activity-topics.js +++ b/app/assets/javascripts/discourse/app/routes/user-activity-topics.js @@ -1,15 +1,32 @@ import UserAction from "discourse/models/user-action"; import UserTopicListRoute from "discourse/routes/user-topic-list"; import { action } from "@ember/object"; +import I18n from "I18n"; export default UserTopicListRoute.extend({ userActionType: UserAction.TYPES.topics, model: function () { - return this.store.findFiltered("topicList", { - filter: - "topics/created-by/" + this.modelFor("user").get("username_lower"), - }); + return this.store + .findFiltered("topicList", { + filter: + "topics/created-by/" + this.modelFor("user").get("username_lower"), + }) + .then((model) => { + // andrei: we agreed that this is an anti pattern, + // it's better to avoid mutating a rest model like this + // this place we'll be refactored later + // see https://github.com/discourse/discourse/pull/14313#discussion_r708784704 + model.set("emptyState", this.emptyState()); + return model; + }); + }, + + emptyState() { + return { + title: I18n.t("user_activity.no_topics_title"), + body: "", + }; }, @action diff --git a/app/assets/javascripts/discourse/app/templates/user-topics-list.hbs b/app/assets/javascripts/discourse/app/templates/user-topics-list.hbs index 7096fec48c6..b507cb4ecb1 100644 --- a/app/assets/javascripts/discourse/app/templates/user-topics-list.hbs +++ b/app/assets/javascripts/discourse/app/templates/user-topics-list.hbs @@ -1,8 +1,8 @@ -{{#if showEmptyStatePlaceholder}} +{{#if noContent}}
- {{emptyState.title}} + {{model.emptyState.title}}
-

{{emptyState.body}}

+

{{model.emptyState.body}}

{{else}} diff --git a/app/assets/javascripts/discourse/tests/acceptance/group-test.js b/app/assets/javascripts/discourse/tests/acceptance/group-test.js index 17d6d9de4ba..7c4c3ebb603 100644 --- a/app/assets/javascripts/discourse/tests/acceptance/group-test.js +++ b/app/assets/javascripts/discourse/tests/acceptance/group-test.js @@ -2,6 +2,7 @@ import { acceptance, count, exists, + query, queryAll, } from "discourse/tests/helpers/qunit-helpers"; import { click, visit } from "@ember/test-helpers"; @@ -227,9 +228,9 @@ acceptance("Group - Authenticated", function (needs) { await click(".nav-pills li a[title='Messages']"); assert.equal( - queryAll(".alert").text().trim(), - I18n.t("choose_topic.none_found"), - "it should display the right alert" + query("span.empty-state-title").innerText.trim(), + I18n.t("no_group_messages_title"), + "it should display the right text" ); }); diff --git a/app/assets/javascripts/discourse/tests/acceptance/user-activity-read-test.js b/app/assets/javascripts/discourse/tests/acceptance/user-activity-read-test.js new file mode 100644 index 00000000000..7c85564268e --- /dev/null +++ b/app/assets/javascripts/discourse/tests/acceptance/user-activity-read-test.js @@ -0,0 +1,24 @@ +import { acceptance, exists } from "../helpers/qunit-helpers"; +import { test } from "qunit"; +import { visit } from "@ember/test-helpers"; + +acceptance("User Activity / Read - empty state", function (needs) { + needs.user(); + + needs.pretender((server, helper) => { + const emptyResponse = { + topic_list: { + topics: [], + }, + }; + + server.get("/read.json", () => { + return helper.response(emptyResponse); + }); + }); + + test("It renders the empty state panel", async function (assert) { + await visit("/u/charlie/activity/read"); + assert.ok(exists("div.empty-state")); + }); +}); diff --git a/app/assets/javascripts/discourse/tests/acceptance/user-activity-topic-test.js b/app/assets/javascripts/discourse/tests/acceptance/user-activity-topic-test.js new file mode 100644 index 00000000000..da16e751480 --- /dev/null +++ b/app/assets/javascripts/discourse/tests/acceptance/user-activity-topic-test.js @@ -0,0 +1,24 @@ +import { acceptance, exists } from "../helpers/qunit-helpers"; +import { test } from "qunit"; +import { visit } from "@ember/test-helpers"; + +acceptance("User Activity / Topics - empty state", function (needs) { + needs.user(); + + needs.pretender((server, helper) => { + const emptyResponse = { + topic_list: { + topics: [], + }, + }; + + server.get("/topics/created-by/:username.json", () => { + return helper.response(emptyResponse); + }); + }); + + test("It renders the empty state panel", async function (assert) { + await visit("/u/charlie/activity/topics"); + assert.ok(exists("div.empty-state")); + }); +}); diff --git a/config/locales/client.en.yml b/config/locales/client.en.yml index b42db90b672..f038289c6cb 100644 --- a/config/locales/client.en.yml +++ b/config/locales/client.en.yml @@ -3866,6 +3866,13 @@ en: no_likes_title: "You haven’t liked any topics yet" no_likes_body: "A great way to jump in and start contributing is to start reading conversations that have already taken place, and select the %{heartIcon} on posts that you like!" no_likes_others: "No liked posts." + no_topics_title: "You have not started any topics yet" + no_read_topics_title: "You haven’t read any topics yet" + no_read_topics_body: "Once you start reading discussions, you’ll see a list here. To start reading, look for topics that interest you in Top or Categories or search by keyword %{searchIcon}" + + no_group_messages_title: "No group messages found" + + # This section is exported to the javascript for i18n in the admin section admin_js: