From 0353d9fb0d1d039822c15195b5aedc484f7f2b35 Mon Sep 17 00:00:00 2001 From: Kelv Date: Wed, 13 Mar 2024 09:26:34 +0800 Subject: [PATCH] DEV: refactor topic map to glimmer component (#26119) * DEV: add toggle to switch to glimmer TopicMap and rename imported hbs-compiler * DEV: refactor topic-map tests to use assert.dom * DEV: add topic-map glimmer component * DEV: remove topic-map widget and switch summary-box to use explicitly passed-in actions --------- Co-authored-by: David Taylor --- .../discourse/app/components/summary-box.hbs | 10 +- .../discourse/app/components/summary-box.js | 20 ---- .../discourse/app/components/topic-map.gjs | 54 +++++++++ .../javascripts/discourse/app/widgets/post.js | 37 +++++- .../discourse/app/widgets/topic-map.js | 107 ------------------ .../tests/acceptance/topic-summary-test.js | 30 ++--- .../components/widgets/post-test.js | 10 +- 7 files changed, 111 insertions(+), 157 deletions(-) create mode 100644 app/assets/javascripts/discourse/app/components/topic-map.gjs delete mode 100644 app/assets/javascripts/discourse/app/widgets/topic-map.js diff --git a/app/assets/javascripts/discourse/app/components/summary-box.hbs b/app/assets/javascripts/discourse/app/components/summary-box.hbs index cc64ccc8856..4de34ca792f 100644 --- a/app/assets/javascripts/discourse/app/components/summary-box.hbs +++ b/app/assets/javascripts/discourse/app/components/summary-box.hbs @@ -6,7 +6,7 @@ {{#if @postAttrs.summarizable}} {{#if this.summary.showSummaryBox}} {{else}} +
+ +
+ {{#unless this.collapsed}} +
+ +
+ {{/unless}} + {{#if (or @postAttrs.hasTopRepliesSummary @postAttrs.summarizable)}} +
+ +
+ {{/if}} + {{#if @postAttrs.showPMMap}} +
+ +
+ {{/if}} + +} diff --git a/app/assets/javascripts/discourse/app/widgets/post.js b/app/assets/javascripts/discourse/app/widgets/post.js index 115f3f0c434..ea8870e56b4 100644 --- a/app/assets/javascripts/discourse/app/widgets/post.js +++ b/app/assets/javascripts/discourse/app/widgets/post.js @@ -1,4 +1,5 @@ import { getOwner } from "@ember/application"; +import { hbs } from "ember-cli-htmlbars"; import { Promise } from "rsvp"; import { h } from "virtual-dom"; import ShareTopicModal from "discourse/components/modal/share-topic"; @@ -15,10 +16,11 @@ import { transformBasicPost } from "discourse/lib/transform-post"; import DiscourseURL from "discourse/lib/url"; import { clipboardCopy, formatUsername } from "discourse/lib/utilities"; import DecoratorHelper from "discourse/widgets/decorator-helper"; -import hbs from "discourse/widgets/hbs-compiler"; +import widgetHbs from "discourse/widgets/hbs-compiler"; import PostCooked from "discourse/widgets/post-cooked"; import { postTransformCallbacks } from "discourse/widgets/post-stream"; import RawHtml from "discourse/widgets/raw-html"; +import RenderGlimmer from "discourse/widgets/render-glimmer"; import { applyDecorators, createWidget } from "discourse/widgets/widget"; import { isTesting } from "discourse-common/config/environment"; import { avatarUrl, translateSize } from "discourse-common/lib/avatar-utils"; @@ -263,7 +265,7 @@ createWidget("post-avatar", { createWidget("post-locked-indicator", { tagName: "div.post-info.post-locked", - template: hbs`{{d-icon "lock"}}`, + template: widgetHbs`{{d-icon "lock"}}`, title: () => I18n.t("post.locked"), }); @@ -741,11 +743,40 @@ createWidget("post-body", { result.push(this.attach("actions-summary", attrs)); result.push(this.attach("post-links", attrs)); if (attrs.showTopicMap) { - result.push(this.attach("topic-map", attrs)); + result.push(this.buildTopicMap(attrs)); } return result; }, + + buildTopicMap(attrs) { + return new RenderGlimmer( + this, + "div.topic-map", + hbs``, + { + postAttrs: attrs, + cancelFilter: () => this.sendWidgetAction("cancelFilter"), + showTopReplies: () => this.sendWidgetAction("showTopReplies"), + collapseSummary: () => this.sendWidgetAction("collapseSummary"), + showSummary: () => this.sendWidgetAction("showSummary"), + showInvite: () => this.sendWidgetAction("showInvite"), + removeAllowedGroup: (group) => + this.sendWidgetAction("removeAllowedGroup", group), + removeAllowedUser: (user) => + this.sendWidgetAction("removeAllowedUser", user), + } + ); + }, }); createWidget("post-article", { diff --git a/app/assets/javascripts/discourse/app/widgets/topic-map.js b/app/assets/javascripts/discourse/app/widgets/topic-map.js deleted file mode 100644 index a08cd8fbe14..00000000000 --- a/app/assets/javascripts/discourse/app/widgets/topic-map.js +++ /dev/null @@ -1,107 +0,0 @@ -import { hbs } from "ember-cli-htmlbars"; -import RenderGlimmer from "discourse/widgets/render-glimmer"; -import { createWidget } from "discourse/widgets/widget"; - -export default createWidget("topic-map", { - tagName: "div.topic-map", - buildKey: (attrs) => `topic-map-${attrs.id}`, - - defaultState(attrs) { - return { collapsed: !attrs.hasTopRepliesSummary }; - }, - - html(attrs, state) { - const contents = [this.buildTopicMapSummary(attrs, state)]; - - if (!state.collapsed) { - contents.push(this.buildTopicMapExpanded(attrs)); - } - - if (attrs.hasTopRepliesSummary || attrs.summarizable) { - contents.push(this.buildSummaryBox(attrs)); - } - - if (attrs.showPMMap) { - contents.push(this.buildPrivateMessageMap(attrs)); - } - return contents; - }, - - toggleMap() { - this.state.collapsed = !this.state.collapsed; - this.scheduleRerender(); - }, - - buildTopicMapSummary(attrs, state) { - const { collapsed } = state; - const wrapperClass = collapsed - ? "section.map.map-collapsed" - : "section.map"; - - return new RenderGlimmer( - this, - wrapperClass, - hbs``, - { - toggleMap: this.toggleMap.bind(this), - postAttrs: attrs, - collapsed, - } - ); - }, - - buildTopicMapExpanded(attrs) { - return new RenderGlimmer( - this, - "section.topic-map-expanded", - hbs``, - { - postAttrs: attrs, - } - ); - }, - - buildSummaryBox(attrs) { - return new RenderGlimmer( - this, - "section.information.toggle-summary", - hbs``, - { - postAttrs: attrs, - actionDispatchFunc: (actionName) => { - this.sendWidgetAction(actionName); - }, - } - ); - }, - - buildPrivateMessageMap(attrs) { - return new RenderGlimmer( - this, - "section.information.private-message-map", - hbs``, - { - postAttrs: attrs, - showInvite: () => this.sendWidgetAction("showInvite"), - removeAllowedGroup: (group) => - this.sendWidgetAction("removeAllowedGroup", group), - removeAllowedUser: (user) => - this.sendWidgetAction("removeAllowedUser", user), - } - ); - }, -}); diff --git a/app/assets/javascripts/discourse/tests/acceptance/topic-summary-test.js b/app/assets/javascripts/discourse/tests/acceptance/topic-summary-test.js index db041d51b22..2bd4b3edd21 100644 --- a/app/assets/javascripts/discourse/tests/acceptance/topic-summary-test.js +++ b/app/assets/javascripts/discourse/tests/acceptance/topic-summary-test.js @@ -3,9 +3,7 @@ import { test } from "qunit"; import topicFixtures from "discourse/tests/fixtures/topic"; import { acceptance, - exists, publishToMessageBus, - query, updateCurrentUser, } from "discourse/tests/helpers/qunit-helpers"; import { cloneJSON } from "discourse-common/lib/object"; @@ -43,11 +41,9 @@ acceptance("Topic - Summary", function (needs) { await click(".topic-strategy-summarization"); - assert.strictEqual( - query(".summary-box .generated-summary p").innerText, - partialSummary, - "Updates the summary with a partial result" - ); + assert + .dom(".summary-box .generated-summary p") + .hasText(partialSummary, "Updates the summary with a partial result"); const finalSummary = "This is a completed summary"; await publishToMessageBus("/summaries/topic/1", { @@ -62,13 +58,11 @@ acceptance("Topic - Summary", function (needs) { }, }); - assert.strictEqual( - query(".summary-box .generated-summary p").innerText, - finalSummary, - "Updates the summary with a partial result" - ); + assert + .dom(".summary-box .generated-summary p") + .hasText(finalSummary, "Updates the summary with a final result"); - assert.ok(exists(".summary-box .summarized-on"), "summary metadata exists"); + assert.dom(".summary-box .summarized-on").exists("summary metadata exists"); }); }); @@ -103,12 +97,10 @@ acceptance("Topic - Summary - Anon", function (needs) { await click(".topic-strategy-summarization"); - assert.strictEqual( - query(".summary-box .generated-summary p").innerText, - finalSummary, - "Updates the summary with the result" - ); + assert + .dom(".summary-box .generated-summary p") + .hasText(finalSummary, "Updates the summary with the result"); - assert.ok(exists(".summary-box .summarized-on"), "summary metadata exists"); + assert.dom(".summary-box .summarized-on").exists("summary metadata exists"); }); }); diff --git a/app/assets/javascripts/discourse/tests/integration/components/widgets/post-test.js b/app/assets/javascripts/discourse/tests/integration/components/widgets/post-test.js index 174dd9f57af..5faadad1167 100644 --- a/app/assets/javascripts/discourse/tests/integration/components/widgets/post-test.js +++ b/app/assets/javascripts/discourse/tests/integration/components/widgets/post-test.js @@ -776,7 +776,7 @@ module("Integration | Component | Widget | post", function (hooks) { await render(hbs``); - assert.ok(!exists(".topic-map")); + assert.dom(".topic-map").doesNotExist(); }); test("topic map - few posts", async function (assert) { @@ -849,7 +849,7 @@ module("Integration | Component | Widget | post", function (hooks) { await render(hbs``); - assert.ok(!exists(".toggle-summary")); + assert.dom(".toggle-summary").doesNotExist(); }); test("topic map - has top replies summary", async function (assert) { @@ -860,7 +860,7 @@ module("Integration | Component | Widget | post", function (hooks) { hbs`` ); - assert.strictEqual(count(".toggle-summary"), 1); + assert.dom(".toggle-summary").exists({ count: 1 }); await click(".toggle-summary button"); assert.ok(this.summaryToggled); @@ -876,8 +876,8 @@ module("Integration | Component | Widget | post", function (hooks) { await render(hbs``); - assert.strictEqual(count(".private-message-map"), 1); - assert.strictEqual(count(".private-message-map .user"), 1); + assert.dom(".private-message-map").exists({ count: 1 }); + assert.dom(".private-message-map .user").exists({ count: 1 }); }); test("post notice - with username", async function (assert) {