From a51e5e198743f42345230ea69713767bd8e11d4a Mon Sep 17 00:00:00 2001 From: Joffrey JAFFEUX Date: Mon, 7 Nov 2022 09:04:43 +0100 Subject: [PATCH] DEV: separates preferred-chat-mode service (#18883) Also adds end to end system tests to ensure navigation scenarios are working correctly. This separation will make it easier to implement state in/out from chat. --- .../discourse/components/chat-live-pane.js | 3 +- .../discourse/components/topic-chat-float.js | 6 +- .../discourse/services/chat-preferred-mode.js | 35 +++++++++++ .../javascripts/discourse/services/chat.js | 3 +- .../discourse/services/full-page-chat.js | 13 ---- .../discourse/widgets/chat-header-icon.js | 6 +- plugins/chat/spec/system/navigation_spec.rb | 60 ++++++++++++++++--- .../unit/services/chat-preferred-mode-test.js | 38 ++++++++++++ .../unit/services/full-page-chat-test.js | 6 -- 9 files changed, 136 insertions(+), 34 deletions(-) create mode 100644 plugins/chat/assets/javascripts/discourse/services/chat-preferred-mode.js create mode 100644 plugins/chat/test/javascripts/unit/services/chat-preferred-mode-test.js diff --git a/plugins/chat/assets/javascripts/discourse/components/chat-live-pane.js b/plugins/chat/assets/javascripts/discourse/components/chat-live-pane.js index e4200f06c92..6258e7a0ec2 100644 --- a/plugins/chat/assets/javascripts/discourse/components/chat-live-pane.js +++ b/plugins/chat/assets/javascripts/discourse/components/chat-live-pane.js @@ -74,6 +74,7 @@ export default Component.extend({ router: service(), chatEmojiPickerManager: service(), chatComposerPresenceManager: service(), + chatPreferredMode: service(), fullPageChat: service(), getCachedChannelDetails: null, @@ -1265,7 +1266,7 @@ export default Component.extend({ @action onCloseFullScreen(channel) { - this.fullPageChat.isPreferred = false; + this.chatPreferredMode.setDrawer(); this.appEvents.trigger("chat:open-channel", channel); const previousRouteInfo = this.fullPageChat.exit(); diff --git a/plugins/chat/assets/javascripts/discourse/components/topic-chat-float.js b/plugins/chat/assets/javascripts/discourse/components/topic-chat-float.js index 17ae2dac5fb..740823856f7 100644 --- a/plugins/chat/assets/javascripts/discourse/components/topic-chat-float.js +++ b/plugins/chat/assets/javascripts/discourse/components/topic-chat-float.js @@ -18,7 +18,7 @@ export default Component.extend({ classNameBindings: [":topic-chat-float-container", "hidden"], chat: service(), router: service(), - fullPageChat: service(), + chatPreferredMode: service(), hidden: true, loading: false, expanded: true, // TODO - false when not first-load topic @@ -237,12 +237,12 @@ export default Component.extend({ @action openInFullPage(e) { - const channel = this.chat.activeChannel; + this.chatPreferredMode.setFullPage(); + const channel = this.chat.activeChannel; this.set("expanded", false); this.set("hidden", true); this.chat.setActiveChannel(null); - this.fullPageChat.isPreferred = true; if (!channel) { return this.router.transitionTo("chat"); diff --git a/plugins/chat/assets/javascripts/discourse/services/chat-preferred-mode.js b/plugins/chat/assets/javascripts/discourse/services/chat-preferred-mode.js new file mode 100644 index 00000000000..fa49c428ec1 --- /dev/null +++ b/plugins/chat/assets/javascripts/discourse/services/chat-preferred-mode.js @@ -0,0 +1,35 @@ +import KeyValueStore from "discourse/lib/key-value-store"; +import Service from "@ember/service"; +import Site from "discourse/models/site"; + +const PREFERRED_MODE_KEY = "preferred_mode"; +const PREFERRED_MODE_STORE_NAMESPACE = "discourse_chat_"; +const FULL_PAGE_CHAT = "FULL_PAGE_CHAT"; +const DRAWER_CHAT = "DRAWER_CHAT"; + +export default class ChatPreferredMode extends Service { + _store = new KeyValueStore(PREFERRED_MODE_STORE_NAMESPACE); + + setFullPage() { + this._store.setObject({ key: PREFERRED_MODE_KEY, value: FULL_PAGE_CHAT }); + } + + setDrawer() { + this._store.setObject({ key: PREFERRED_MODE_KEY, value: DRAWER_CHAT }); + } + + get isFullPage() { + return !!( + Site.currentProp("mobileView") || + this._store.getObject(PREFERRED_MODE_KEY) === FULL_PAGE_CHAT + ); + } + + get isDrawer() { + return !!( + !Site.currentProp("mobileView") && + (!this._store.getObject(PREFERRED_MODE_KEY) || + this._store.getObject(PREFERRED_MODE_KEY) === DRAWER_CHAT) + ); + } +} diff --git a/plugins/chat/assets/javascripts/discourse/services/chat.js b/plugins/chat/assets/javascripts/discourse/services/chat.js index 86c98ec0720..af4925489b7 100644 --- a/plugins/chat/assets/javascripts/discourse/services/chat.js +++ b/plugins/chat/assets/javascripts/discourse/services/chat.js @@ -35,6 +35,7 @@ const READ_INTERVAL = 1000; export default class Chat extends Service { @service appEvents; @service chatNotificationManager; + @service chatPreferredMode; @service fullPageChat; @service presence; @service router; @@ -527,7 +528,7 @@ export default class Chat extends Service { if ( this.fullPageChat.isActive || this.site.mobileView || - this.fullPageChat.isPreferred + this.chatPreferredMode.isFullPage ) { const queryParams = messageId ? { messageId } : {}; diff --git a/plugins/chat/assets/javascripts/discourse/services/full-page-chat.js b/plugins/chat/assets/javascripts/discourse/services/full-page-chat.js index f50630d5f2e..27587388725 100644 --- a/plugins/chat/assets/javascripts/discourse/services/full-page-chat.js +++ b/plugins/chat/assets/javascripts/discourse/services/full-page-chat.js @@ -1,11 +1,6 @@ -import KeyValueStore from "discourse/lib/key-value-store"; import Service from "@ember/service"; -const FULL_PAGE = "fullPage"; -const STORE_NAMESPACE_CHAT_WINDOW = "discourse_chat_window_"; - export default class FullPageChat extends Service { - store = new KeyValueStore(STORE_NAMESPACE_CHAT_WINDOW); _previousRouteInfo = null; _isActive = false; @@ -22,12 +17,4 @@ export default class FullPageChat extends Service { get isActive() { return this._isActive; } - - get isPreferred() { - return !!this.store.getObject(FULL_PAGE); - } - - set isPreferred(value) { - this.store.setObject({ key: FULL_PAGE, value }); - } } diff --git a/plugins/chat/assets/javascripts/discourse/widgets/chat-header-icon.js b/plugins/chat/assets/javascripts/discourse/widgets/chat-header-icon.js index 8ae4e92467e..215c382f45c 100644 --- a/plugins/chat/assets/javascripts/discourse/widgets/chat-header-icon.js +++ b/plugins/chat/assets/javascripts/discourse/widgets/chat-header-icon.js @@ -8,7 +8,7 @@ export default createWidget("header-chat-link", { chat: null, tagName: "li.header-dropdown-toggle.open-chat", title: "chat.title", - services: ["chat", "router", "fullPageChat"], + services: ["chat", "router", "chatPreferredMode", "fullPageChat"], html() { if (!this.chat.userCanChat) { @@ -68,9 +68,9 @@ export default createWidget("header-chat-link", { if ( this.chat.sidebarActive || this.site.mobileView || - this.fullPageChat.isPreferred + this.chatPreferredMode.isFullPage ) { - this.fullPageChat.isPreferred = true; + this.chatPreferredMode.setFullPage(); return this.router.transitionTo("chat"); } else { this.appEvents.trigger("chat:toggle-open"); diff --git a/plugins/chat/spec/system/navigation_spec.rb b/plugins/chat/spec/system/navigation_spec.rb index 0fec209f77c..c964f873192 100644 --- a/plugins/chat/spec/system/navigation_spec.rb +++ b/plugins/chat/spec/system/navigation_spec.rb @@ -1,22 +1,26 @@ # frozen_string_literal: true RSpec.describe "Navigation", type: :system, js: true do - fab!(:user) { Fabricate(:user) } + fab!(:category) { Fabricate(:category) } + fab!(:topic) { Fabricate(:topic) } + fab!(:post) { Fabricate(:post, topic: topic) } + fab!(:user) { Fabricate(:admin) } fab!(:category_channel) { Fabricate(:category_channel) } fab!(:message) { Fabricate(:chat_message, chat_channel: category_channel) } before do + # ensures we have one valid registered admin + user.activate + SiteSetting.chat_enabled = true SiteSetting.chat_allowed_groups = Group::AUTO_GROUPS[:everyone] + category_channel.add(user) + + sign_in(user) end context "when visiting /chat" do - before do - category_channel.add(user) - sign_in(user) - end - - it "it opens full page" do + it "opens full page" do visit("/chat") expect(page).to have_current_path( @@ -26,4 +30,46 @@ RSpec.describe "Navigation", type: :system, js: true do expect(page).to have_css(".chat-message-container[data-id='#{message.id}']") end end + + context "when opening chat" do + it "opens the drawer by default" do + visit("/") + find(".open-chat").click + + expect(page).to have_css(".topic-chat-container.expanded.visible") + end + end + + context "when opening chat with full page as preferred mode" do + it "opens the full page" do + visit("/") + find(".open-chat").click + find(".topic-chat-drawer-header__full-screen-btn").click + + expect(page).to have_current_path( + chat.channel_path(category_channel.id, category_channel.slug), + ) + + visit("/") + find(".open-chat").click + + expect(page).to have_current_path( + chat.channel_path(category_channel.id, category_channel.slug), + ) + end + end + + context "when opening chat with drawer as preferred mode" do + it "opens the full page" do + visit("/chat") + find(".chat-full-screen-button").click + + expect(page).to have_css(".topic-chat-container.expanded.visible") + + visit("/") + find(".open-chat").click + + expect(page).to have_css(".topic-chat-container.expanded.visible") + end + end end diff --git a/plugins/chat/test/javascripts/unit/services/chat-preferred-mode-test.js b/plugins/chat/test/javascripts/unit/services/chat-preferred-mode-test.js new file mode 100644 index 00000000000..7736fc30519 --- /dev/null +++ b/plugins/chat/test/javascripts/unit/services/chat-preferred-mode-test.js @@ -0,0 +1,38 @@ +import { module, test } from "qunit"; +import { getOwner } from "discourse-common/lib/get-owner"; +import Site from "discourse/models/site"; + +module( + "Discourse Chat | Unit | Service | chat-preferred-mode", + function (hooks) { + hooks.beforeEach(function () { + Site.currentProp("mobileView", false); + + this.chatPreferredMode = getOwner(this).lookup( + "service:chat-preferred-mode" + ); + }); + + test("defaults", function (assert) { + assert.strictEqual(this.chatPreferredMode.isDrawer, true); + assert.strictEqual(this.chatPreferredMode.isFullPage, false); + + Site.currentProp("mobileView", true); + + assert.strictEqual(this.chatPreferredMode.isDrawer, false); + assert.strictEqual(this.chatPreferredMode.isFullPage, true); + }); + + test("setFullPage", function (assert) { + this.chatPreferredMode.setFullPage(); + assert.strictEqual(this.chatPreferredMode.isFullPage, true); + assert.strictEqual(this.chatPreferredMode.isDrawer, false); + }); + + test("setDrawer", function (assert) { + this.chatPreferredMode.setDrawer(); + assert.strictEqual(this.chatPreferredMode.isFullPage, false); + assert.strictEqual(this.chatPreferredMode.isDrawer, true); + }); + } +); diff --git a/plugins/chat/test/javascripts/unit/services/full-page-chat-test.js b/plugins/chat/test/javascripts/unit/services/full-page-chat-test.js index 19cd3809592..798370f8eda 100644 --- a/plugins/chat/test/javascripts/unit/services/full-page-chat-test.js +++ b/plugins/chat/test/javascripts/unit/services/full-page-chat-test.js @@ -26,12 +26,6 @@ module("Discourse Chat | Unit | Service | full-page-chat", function (hooks) { assert.strictEqual(this.fullPageChat.isActive, false); }); - test("isPreferred", function (assert) { - assert.strictEqual(this.fullPageChat.isPreferred, false); - this.fullPageChat.isPreferred = true; - assert.strictEqual(this.fullPageChat.isPreferred, true); - }); - test("previous route", function (assert) { const name = "foo"; const params = { id: 1, slug: "bar" };