From 41df7051883dca8404c38e151294058ec8525b83 Mon Sep 17 00:00:00 2001 From: Joffrey JAFFEUX Date: Mon, 16 Dec 2024 19:59:18 +0100 Subject: [PATCH] DEV: replaces topic-notifications-options by DMenu (#30298) This commit introduces which is a wrapper component around which replaces the select-kit component . Each tracking case has its dedicated component: - topic -> `` - group -> `` - tag -> `` - category -> `` - chat thread -> `` --- .../category-notifications-tracking.gjs | 16 ++ .../discourse/app/components/d-modal.gjs | 6 +- .../discourse/app/components/d-navigation.hbs | 17 +- .../group-notifications-tracking.gjs | 14 ++ .../app/components/notifications-tracking.gjs | 160 ++++++++++++++++++ .../components/tag-notifications-tracking.gjs | 14 ++ .../topic-notifications-tracking.gjs | 24 +++ .../templates/user-private-messages-group.hbs | 4 +- .../discourse/tests/acceptance/tags-test.js | 2 +- .../topic-notifications-button-test.js | 25 +-- .../acceptance/user-private-messages-test.js | 2 +- .../notifications-tracking-assertions.js | 31 ++++ .../helpers/notifications-tracking-helper.js | 31 ++++ .../discourse/tests/helpers/qunit-helpers.js | 2 + .../notifications-tracking-test.gjs | 81 +++++++++ .../topic-notifications-button-test.gjs | 25 +-- .../topic-notifications-options-test.gjs | 100 ----------- .../float-kit/addon/components/d-menu.gjs | 1 + .../addon/modifiers/close-on-escape.js | 4 +- .../category-notifications-button.js | 16 -- .../components/topic-notifications-button.gjs | 18 +- .../stylesheets/common/components/_index.scss | 1 + .../components/notifications-tracking.scss | 43 +++++ .../stylesheets/common/float-kit/d-menu.scss | 4 + .../topic-notifications-button.scss | 17 +- app/assets/stylesheets/mobile/user.scss | 10 -- .../da-category-notification-level-field.gjs | 8 +- ...-category-notification-level-field-test.js | 6 +- .../chat-thread-tracking-dropdown.js | 19 --- .../chat/navbar/thread-tracking-dropdown.gjs | 6 +- .../thread-notifications-tracking.gjs | 17 ++ .../components/thread-notifications-button.js | 18 -- .../system/page_objects/chat/chat_thread.rb | 14 +- .../components/sections/atoms/dropdowns.hbs | 24 +-- .../components/notifications_tracking.rb | 52 ++++++ .../page_objects/components/topic_view.rb | 6 +- spec/system/page_objects/pages/topic.rb | 4 +- spec/system/tag_notification_level_spec.rb | 12 +- 38 files changed, 565 insertions(+), 289 deletions(-) create mode 100644 app/assets/javascripts/discourse/app/components/category-notifications-tracking.gjs create mode 100644 app/assets/javascripts/discourse/app/components/group-notifications-tracking.gjs create mode 100644 app/assets/javascripts/discourse/app/components/notifications-tracking.gjs create mode 100644 app/assets/javascripts/discourse/app/components/tag-notifications-tracking.gjs create mode 100644 app/assets/javascripts/discourse/app/components/topic-notifications-tracking.gjs create mode 100644 app/assets/javascripts/discourse/tests/helpers/notifications-tracking-assertions.js create mode 100644 app/assets/javascripts/discourse/tests/helpers/notifications-tracking-helper.js create mode 100644 app/assets/javascripts/discourse/tests/integration/components/notifications-tracking-test.gjs delete mode 100644 app/assets/javascripts/discourse/tests/integration/components/select-kit/topic-notifications-options-test.gjs delete mode 100644 app/assets/javascripts/select-kit/addon/components/category-notifications-button.js create mode 100644 app/assets/stylesheets/common/components/notifications-tracking.scss delete mode 100644 plugins/chat/assets/javascripts/discourse/components/chat-thread-tracking-dropdown.js create mode 100644 plugins/chat/assets/javascripts/discourse/components/thread-notifications-tracking.gjs delete mode 100644 plugins/chat/assets/javascripts/select-kit/addons/components/thread-notifications-button.js create mode 100644 spec/system/page_objects/components/notifications_tracking.rb diff --git a/app/assets/javascripts/discourse/app/components/category-notifications-tracking.gjs b/app/assets/javascripts/discourse/app/components/category-notifications-tracking.gjs new file mode 100644 index 00000000000..67ba8e07f80 --- /dev/null +++ b/app/assets/javascripts/discourse/app/components/category-notifications-tracking.gjs @@ -0,0 +1,16 @@ +import NotificationsTracking from "discourse/components/notifications-tracking"; +import { i18n } from "discourse-i18n"; + +const CategoryNotificationsTracking = ; + +export default CategoryNotificationsTracking; diff --git a/app/assets/javascripts/discourse/app/components/d-modal.gjs b/app/assets/javascripts/discourse/app/components/d-modal.gjs index 487828bdd60..dc482b8c2f2 100644 --- a/app/assets/javascripts/discourse/app/components/d-modal.gjs +++ b/app/assets/javascripts/discourse/app/components/d-modal.gjs @@ -108,6 +108,10 @@ export default class DModal extends Component { } } + get autofocus() { + return this.args.autofocus ?? true; + } + shouldTriggerClickOnEnter(event) { if (this.args.submitOnEnter === false) { return false; @@ -277,7 +281,7 @@ export default class DModal extends Component { ...attributes {{didInsert this.setupModal}} {{willDestroy this.cleanupModal}} - {{trapTab preventScroll=false}} + {{trapTab preventScroll=false autofocus=this.autofocus}} >
{{yield to="aboveHeader"}} diff --git a/app/assets/javascripts/discourse/app/components/d-navigation.hbs b/app/assets/javascripts/discourse/app/components/d-navigation.hbs index 939254736ab..90f93c48ba2 100644 --- a/app/assets/javascripts/discourse/app/components/d-navigation.hbs +++ b/app/assets/javascripts/discourse/app/components/d-navigation.hbs @@ -119,11 +119,14 @@ {{#unless this.tag}} {{! don't show category notification menu on tag pages }} {{#if this.showCategoryNotifications}} - + {{#unless this.category.deleted}} + + {{/unless}} {{/if}} {{/unless}} {{/if}} @@ -132,9 +135,9 @@ {{#unless this.category}} {{! don't show tag notification menu on category pages }} {{#if this.showTagNotifications}} - {{/if}} {{/unless}} diff --git a/app/assets/javascripts/discourse/app/components/group-notifications-tracking.gjs b/app/assets/javascripts/discourse/app/components/group-notifications-tracking.gjs new file mode 100644 index 00000000000..8223667e8ad --- /dev/null +++ b/app/assets/javascripts/discourse/app/components/group-notifications-tracking.gjs @@ -0,0 +1,14 @@ +import NotificationsTracking from "discourse/components/notifications-tracking"; + +const GroupNotificationsTracking = ; + +export default GroupNotificationsTracking; diff --git a/app/assets/javascripts/discourse/app/components/notifications-tracking.gjs b/app/assets/javascripts/discourse/app/components/notifications-tracking.gjs new file mode 100644 index 00000000000..ac5ff309a49 --- /dev/null +++ b/app/assets/javascripts/discourse/app/components/notifications-tracking.gjs @@ -0,0 +1,160 @@ +import Component from "@glimmer/component"; +import { fn, hash } from "@ember/helper"; +import { action } from "@ember/object"; +import DButton from "discourse/components/d-button"; +import DropdownMenu from "discourse/components/dropdown-menu"; +import PluginOutlet from "discourse/components/plugin-outlet"; +import concatClass from "discourse/helpers/concat-class"; +import { allLevels, buttonDetails } from "discourse/lib/notification-levels"; +import icon from "discourse-common/helpers/d-icon"; +import { i18n } from "discourse-i18n"; +import DMenu from "float-kit/components/d-menu"; + +function constructKey(prefix, level, suffix, key) { + let string = prefix + "." + level; + + if (suffix) { + string += suffix; + } + + return i18n(string + "." + key); +} + +class NotificationsTrackingTrigger extends Component { + get showFullTitle() { + return this.args.showFullTitle ?? true; + } + + get showCaret() { + return this.args.showCaret ?? true; + } + + get title() { + return constructKey( + this.args.prefix, + this.args.selectedLevel.key, + this.args.suffix, + "title" + ); + } + + +} + +export default class NotificationsTracking extends Component { + @action + registerDmenuApi(api) { + this.dmenuApi = api; + } + + @action + async setNotificationLevel(level) { + await this.dmenuApi.close(); + this.args.onChange?.(level); + } + + @action + description(level) { + return constructKey( + this.args.prefix, + level.key, + this.args.suffix, + "description" + ); + } + + @action + label(level) { + return constructKey(this.args.prefix, level.key, this.args.suffix, "title"); + } + + @action + isSelectedClass(level) { + return this.args.levelId === level.id ? "-selected" : ""; + } + + get selectedLevel() { + return buttonDetails(this.args.levelId); + } + + get levels() { + return this.args.levels ?? allLevels; + } + + +} diff --git a/app/assets/javascripts/discourse/app/components/tag-notifications-tracking.gjs b/app/assets/javascripts/discourse/app/components/tag-notifications-tracking.gjs new file mode 100644 index 00000000000..44c564430e6 --- /dev/null +++ b/app/assets/javascripts/discourse/app/components/tag-notifications-tracking.gjs @@ -0,0 +1,14 @@ +import NotificationsTracking from "discourse/components/notifications-tracking"; + +const TagNotificationsTracking = ; + +export default TagNotificationsTracking; diff --git a/app/assets/javascripts/discourse/app/components/topic-notifications-tracking.gjs b/app/assets/javascripts/discourse/app/components/topic-notifications-tracking.gjs new file mode 100644 index 00000000000..de4bb4e1dcc --- /dev/null +++ b/app/assets/javascripts/discourse/app/components/topic-notifications-tracking.gjs @@ -0,0 +1,24 @@ +import Component from "@glimmer/component"; +import NotificationsTracking from "discourse/components/notifications-tracking"; +import { topicLevels } from "discourse/lib/notification-levels"; +import { i18n } from "discourse-i18n"; + +export default class TopicNotificationsTracking extends Component { + get suffix() { + return this.args.topic?.archetype === "private_message" ? "_pm" : ""; + } + + +} diff --git a/app/assets/javascripts/discourse/app/templates/user-private-messages-group.hbs b/app/assets/javascripts/discourse/app/templates/user-private-messages-group.hbs index 80be6217763..59ae664ed5d 100644 --- a/app/assets/javascripts/discourse/app/templates/user-private-messages-group.hbs +++ b/app/assets/javascripts/discourse/app/templates/user-private-messages-group.hbs @@ -40,8 +40,8 @@ {{#in-element this.navigationControlsButton}} - {{/in-element}} diff --git a/app/assets/javascripts/discourse/tests/acceptance/tags-test.js b/app/assets/javascripts/discourse/tests/acceptance/tags-test.js index faa5690a7f4..b77bb769c3b 100644 --- a/app/assets/javascripts/discourse/tests/acceptance/tags-test.js +++ b/app/assets/javascripts/discourse/tests/acceptance/tags-test.js @@ -117,7 +117,7 @@ acceptance("Tags", function (needs) { test("hide tag notifications menu", async function (assert) { await visit("/tags/c/faq/4/test"); - assert.dom(".tag-notifications-button").doesNotExist(); + assert.dom(".tag-notifications-tracking").doesNotExist(); }); }); diff --git a/app/assets/javascripts/discourse/tests/acceptance/topic-notifications-button-test.js b/app/assets/javascripts/discourse/tests/acceptance/topic-notifications-button-test.js index 20f9901d610..b698e88eb4f 100644 --- a/app/assets/javascripts/discourse/tests/acceptance/topic-notifications-button-test.js +++ b/app/assets/javascripts/discourse/tests/acceptance/topic-notifications-button-test.js @@ -1,7 +1,7 @@ import { visit } from "@ember/test-helpers"; import { test } from "qunit"; +import notificationsTracking from "discourse/tests/helpers/notifications-tracking-helper"; import { acceptance } from "discourse/tests/helpers/qunit-helpers"; -import selectKit from "discourse/tests/helpers/select-kit-helper"; acceptance("Topic Notifications button", function (needs) { needs.user(); @@ -13,24 +13,15 @@ acceptance("Topic Notifications button", function (needs) { }); test("Updating topic notification level", async function (assert) { - const notificationOptions = selectKit( - "#topic-footer-buttons .topic-notifications-options" - ); - await visit("/t/internationalization-localization/280"); - assert.true( - notificationOptions.exists(), - "displays the notification options button in the topic's footer" - ); + await notificationsTracking().selectLevelId(3); - await notificationOptions.expand(); - await notificationOptions.selectRowByValue("3"); - - assert.strictEqual( - notificationOptions.header().label(), - "Watching", - "displays the right notification level" - ); + assert + .notificationsTracking() + .hasSelectedLevelName( + "watching", + "displays the right notification level" + ); }); }); diff --git a/app/assets/javascripts/discourse/tests/acceptance/user-private-messages-test.js b/app/assets/javascripts/discourse/tests/acceptance/user-private-messages-test.js index 21329f8cf70..31bb25e6b77 100644 --- a/app/assets/javascripts/discourse/tests/acceptance/user-private-messages-test.js +++ b/app/assets/javascripts/discourse/tests/acceptance/user-private-messages-test.js @@ -592,7 +592,7 @@ acceptance( .exists({ count: 2 }, "displays the right topic list"); assert - .dom(".group-notifications-button") + .dom(".group-notifications-tracking") .exists("displays the group notifications button"); }); diff --git a/app/assets/javascripts/discourse/tests/helpers/notifications-tracking-assertions.js b/app/assets/javascripts/discourse/tests/helpers/notifications-tracking-assertions.js new file mode 100644 index 00000000000..9e0cc8167da --- /dev/null +++ b/app/assets/javascripts/discourse/tests/helpers/notifications-tracking-assertions.js @@ -0,0 +1,31 @@ +import QUnit from "qunit"; +import { query } from "discourse/tests/helpers/qunit-helpers"; + +class NotificationsTracking { + constructor(selector, context) { + this.context = context; + if (selector instanceof HTMLElement) { + this.element = selector; + } else { + this.element = query(selector); + } + } + + hasSelectedLevelName(name, message) { + this.context + .dom(this.element) + .hasAttribute("data-level-name", name, message); + } + + hasSelectedLevelId(id, message) { + this.context.dom(this.element).hasAttribute("data-level-id", name, message); + } +} + +export function setupNotificationsTrackingAssertions() { + QUnit.assert.notificationsTracking = function ( + selector = ".notifications-tracking-trigger" + ) { + return new NotificationsTracking(selector, this); + }; +} diff --git a/app/assets/javascripts/discourse/tests/helpers/notifications-tracking-helper.js b/app/assets/javascripts/discourse/tests/helpers/notifications-tracking-helper.js new file mode 100644 index 00000000000..1d22d01fc83 --- /dev/null +++ b/app/assets/javascripts/discourse/tests/helpers/notifications-tracking-helper.js @@ -0,0 +1,31 @@ +import { click } from "@ember/test-helpers"; +import { query } from "discourse/tests/helpers/qunit-helpers"; + +class Notificationstracking { + constructor(selector) { + if (selector instanceof HTMLElement) { + this.element = selector; + } else { + this.element = query(selector); + } + } + + async selectLevelId(levelId) { + await click(this.element); + const content = this.content(); + await click(content.querySelector(`[data-level-id="${levelId}"]`)); + } + + content() { + const identifier = this.element.dataset.identifier; + return document.querySelector( + `[data-content][data-identifier="${identifier}"]` + ); + } +} + +export default function notificationsTracking( + selector = ".notifications-tracking-trigger" +) { + return new Notificationstracking(selector); +} diff --git a/app/assets/javascripts/discourse/tests/helpers/qunit-helpers.js b/app/assets/javascripts/discourse/tests/helpers/qunit-helpers.js index 0c88d8d2378..22cd682b9e1 100644 --- a/app/assets/javascripts/discourse/tests/helpers/qunit-helpers.js +++ b/app/assets/javascripts/discourse/tests/helpers/qunit-helpers.js @@ -105,6 +105,7 @@ import I18n from "discourse-i18n"; import { _clearSnapshots } from "select-kit/components/composer-actions"; import { setupDSelectAssertions } from "./d-select-assertions"; import { setupFormKitAssertions } from "./form-kit-assertions"; +import { setupNotificationsTrackingAssertions } from "./notifications-tracking-assertions"; import { cleanupTemporaryModuleRegistrations } from "./temporary-module-helper"; export function currentUser() { @@ -485,6 +486,7 @@ QUnit.assert.containsInstance = function (collection, klass, message) { setupFormKitAssertions(); setupDSelectAssertions(); +setupNotificationsTrackingAssertions(); export async function selectDate(selector, date) { const elem = document.querySelector(selector); diff --git a/app/assets/javascripts/discourse/tests/integration/components/notifications-tracking-test.gjs b/app/assets/javascripts/discourse/tests/integration/components/notifications-tracking-test.gjs new file mode 100644 index 00000000000..a76026f0c4b --- /dev/null +++ b/app/assets/javascripts/discourse/tests/integration/components/notifications-tracking-test.gjs @@ -0,0 +1,81 @@ +import { hash } from "@ember/helper"; +import { click, render } from "@ember/test-helpers"; +import { module, test } from "qunit"; +import TopicNotificationsTracking from "discourse/components/topic-notifications-tracking"; +import { setupRenderingTest } from "discourse/tests/helpers/component-test"; +import { i18n } from "discourse-i18n"; + +function extractDescriptions(rows) { + return [...rows].map((el) => + el + .querySelector(".notifications-tracking-btn__description") + .textContent.trim() + ); +} + +function getTranslations(type = "") { + return ["watching", "tracking", "regular", "muted"].map((key) => { + return i18n(`topic.notifications.${key}${type}.description`); + }); +} + +module("Integration | Component | TopicTracking", function (hooks) { + setupRenderingTest(hooks); + + test("regular topic notification level descriptions", async function (assert) { + await render(); + + await click(".notifications-tracking-trigger"); + + const uiTexts = extractDescriptions( + document.querySelectorAll(".notifications-tracking-btn") + ); + const descriptions = getTranslations(); + + assert.strictEqual( + uiTexts.length, + descriptions.length, + "has the correct copy" + ); + + uiTexts.forEach((text, index) => { + assert.strictEqual( + text.trim(), + descriptions[index].trim(), + "has the correct copy" + ); + }); + }); + + test("PM topic notification level descriptions", async function (assert) { + await render(); + + await click(".notifications-tracking-trigger"); + + const uiTexts = extractDescriptions( + document.querySelectorAll(".notifications-tracking-btn") + ); + const descriptions = getTranslations("_pm"); + + assert.strictEqual( + uiTexts.length, + descriptions.length, + "has the correct copy" + ); + + uiTexts.forEach((text, index) => { + assert.strictEqual( + text.trim(), + descriptions[index].trim(), + "has the correct copy" + ); + }); + }); +}); diff --git a/app/assets/javascripts/discourse/tests/integration/components/select-kit/topic-notifications-button-test.gjs b/app/assets/javascripts/discourse/tests/integration/components/select-kit/topic-notifications-button-test.gjs index a571535be93..f626edaad01 100644 --- a/app/assets/javascripts/discourse/tests/integration/components/select-kit/topic-notifications-button-test.gjs +++ b/app/assets/javascripts/discourse/tests/integration/components/select-kit/topic-notifications-button-test.gjs @@ -3,7 +3,6 @@ import { getOwner } from "@ember/owner"; import { render, settled } from "@ember/test-helpers"; import { module, test } from "qunit"; import { setupRenderingTest } from "discourse/tests/helpers/component-test"; -import selectKit from "discourse/tests/helpers/select-kit-helper"; import I18n, { i18n } from "discourse-i18n"; import TopicNotificationsButton from "select-kit/components/topic-notifications-button"; @@ -50,20 +49,16 @@ module( ); - assert.strictEqual( - selectKit().header().label(), - "Normal", - "has the correct label" - ); + assert + .dom(".notifications-tracking-trigger") + .hasText("Normal", "has the correct label"); state.topic = buildTopic.call(this, { level: 2 }); await settled(); - assert.strictEqual( - selectKit().header().label(), - "Tracking", - "correctly changes the label" - ); + assert + .dom(".notifications-tracking-trigger") + .hasText("Tracking", "has the correct label"); }); test("the header has a localized title", async function (assert) { @@ -77,11 +72,9 @@ module( ); - assert.strictEqual( - selectKit().header().label(), - `${originalTranslation} PM`, - "has the correct label for PMs" - ); + assert + .dom(".notifications-tracking-trigger") + .hasText(`${originalTranslation} PM`, "has the correct label for PMs"); }); test("notification reason text - user mailing list mode", async function (assert) { diff --git a/app/assets/javascripts/discourse/tests/integration/components/select-kit/topic-notifications-options-test.gjs b/app/assets/javascripts/discourse/tests/integration/components/select-kit/topic-notifications-options-test.gjs deleted file mode 100644 index 83e151fef4b..00000000000 --- a/app/assets/javascripts/discourse/tests/integration/components/select-kit/topic-notifications-options-test.gjs +++ /dev/null @@ -1,100 +0,0 @@ -import { getOwner } from "@ember/owner"; -import { render } from "@ember/test-helpers"; -import { module, test } from "qunit"; -import { setupRenderingTest } from "discourse/tests/helpers/component-test"; -import selectKit from "discourse/tests/helpers/select-kit-helper"; -import { i18n } from "discourse-i18n"; -import TopicNotificationsOptions from "select-kit/components/topic-notifications-options"; - -function extractDescriptions(rows) { - return [...rows].map((el) => el.querySelector(".desc").textContent.trim()); -} - -function getTranslations(type = "") { - return ["watching", "tracking", "regular", "muted"].map((key) => { - return i18n(`topic.notifications.${key}${type}.description`); - }); -} - -module( - "Integration | Component | select-kit/topic-notifications-options", - function (hooks) { - setupRenderingTest(hooks); - - test("regular topic notification level descriptions", async function (assert) { - const store = getOwner(this).lookup("service:store"); - const topic = store.createRecord("topic", { - id: 4563, - title: "Qunit Test Topic", - archetype: "regular", - details: { - notification_level: 1, - }, - }); - - await render(); - - await selectKit().expand(); - - const uiTexts = extractDescriptions(selectKit().rows()); - const descriptions = getTranslations(); - - assert.strictEqual( - uiTexts.length, - descriptions.length, - "has the correct copy" - ); - - uiTexts.forEach((text, index) => { - assert.strictEqual( - text.trim(), - descriptions[index].trim(), - "has the correct copy" - ); - }); - }); - - test("PM topic notification level descriptions", async function (assert) { - const store = getOwner(this).lookup("service:store"); - const topic = store.createRecord("topic", { - id: 4563, - title: "Qunit Test Topic", - archetype: "private_message", - details: { - notification_level: 1, - }, - }); - - await render(); - - await selectKit().expand(); - - const uiTexts = extractDescriptions(selectKit().rows()); - const descriptions = getTranslations("_pm"); - - assert.strictEqual( - uiTexts.length, - descriptions.length, - "has the correct copy" - ); - - uiTexts.forEach((text, index) => { - assert.strictEqual( - text.trim(), - descriptions[index].trim(), - "has the correct copy" - ); - }); - }); - } -); diff --git a/app/assets/javascripts/float-kit/addon/components/d-menu.gjs b/app/assets/javascripts/float-kit/addon/components/d-menu.gjs index e6d82597d00..15572940e71 100644 --- a/app/assets/javascripts/float-kit/addon/components/d-menu.gjs +++ b/app/assets/javascripts/float-kit/addon/components/d-menu.gjs @@ -111,6 +111,7 @@ export default class DMenu extends Component {

{{yield}}

; const EmptyWrapper =