From 21570410aba084f2f19172ce5ae88b85e9a2a18e Mon Sep 17 00:00:00 2001 From: Joffrey JAFFEUX Date: Tue, 8 Nov 2022 16:23:13 +0100 Subject: [PATCH] FIX: makes sidebar links respect drawer mode (#18918) Clicking a link of the sidebar will now open the drawer and load the correct channel. This solution should correctly solve these cases: closing drawer, clicking sidebar channel, should open the drawer on correct channel visiting /chat then visiting / and clicking sidebar channel, should open full page chat on correct channel --- .../discourse/components/chat-live-pane.js | 8 +---- .../discourse/routes/chat-channel.js | 2 ++ .../discourse/routes/chat-index.js | 8 ++--- .../javascripts/discourse/routes/chat.js | 18 +++++++++-- .../discourse/services/full-page-chat.js | 19 +++++++++-- plugins/chat/spec/system/navigation_spec.rb | 32 +++++++++++++++++++ .../spec/system/page_objects/chat/chat.rb | 4 +++ .../unit/services/full-page-chat-test.js | 6 ++-- 8 files changed, 80 insertions(+), 17 deletions(-) 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 f55af46a07..85ca678d63 100644 --- a/plugins/chat/assets/javascripts/discourse/components/chat-live-pane.js +++ b/plugins/chat/assets/javascripts/discourse/components/chat-live-pane.js @@ -27,7 +27,6 @@ import { } from "discourse/lib/user-presence"; import isZoomed from "discourse/plugins/chat/discourse/lib/zoom-check"; import { isTesting } from "discourse-common/config/environment"; -import { defaultHomepage } from "discourse/lib/utilities"; const MAX_RECENT_MSGS = 100; const STICKY_SCROLL_LENIENCE = 50; @@ -1268,12 +1267,7 @@ export default Component.extend({ onCloseFullScreen(channel) { this.chatPreferredMode.setDrawer(); - let previousURL = this.fullPageChat.exit(); - if (!previousURL || previousURL === "/") { - previousURL = this.router.urlFor(`discovery.${defaultHomepage()}`); - } - - this.router.replaceWith(previousURL).then(() => { + this.router.replaceWith(this.fullPageChat.exit()).then(() => { this.appEvents.trigger("chat:open-channel", channel); }); }, diff --git a/plugins/chat/assets/javascripts/discourse/routes/chat-channel.js b/plugins/chat/assets/javascripts/discourse/routes/chat-channel.js index 83f792c682..97e42858d1 100644 --- a/plugins/chat/assets/javascripts/discourse/routes/chat-channel.js +++ b/plugins/chat/assets/javascripts/discourse/routes/chat-channel.js @@ -8,6 +8,8 @@ import slugifyChannel from "discourse/plugins/chat/discourse/lib/slugify-channel export default class ChatChannelRoute extends DiscourseRoute { @service chat; + @service fullPageChat; + @service chatPreferredMode; async model(params) { let [chatChannel, channels] = await Promise.all([ diff --git a/plugins/chat/assets/javascripts/discourse/routes/chat-index.js b/plugins/chat/assets/javascripts/discourse/routes/chat-index.js index aeb6402be6..fbf8cd0a3c 100644 --- a/plugins/chat/assets/javascripts/discourse/routes/chat-index.js +++ b/plugins/chat/assets/javascripts/discourse/routes/chat-index.js @@ -3,8 +3,9 @@ import { inject as service } from "@ember/service"; export default class ChatIndexRoute extends DiscourseRoute { @service chat; + @service router; - beforeModel() { + redirect() { if (this.site.mobileView) { return; // Always want the channel index on mobile. } @@ -14,11 +15,10 @@ export default class ChatIndexRoute extends DiscourseRoute { return this.chat.getIdealFirstChannelIdAndTitle().then((channelInfo) => { if (channelInfo) { return this.chat.getChannelBy("id", channelInfo.id).then((c) => { - this.chat.openChannel(c); - return; + return this.chat.openChannel(c); }); } else { - return this.transitionTo("chat.browse"); + return this.router.transitionTo("chat.browse"); } }); } diff --git a/plugins/chat/assets/javascripts/discourse/routes/chat.js b/plugins/chat/assets/javascripts/discourse/routes/chat.js index bacf20cb49..4d07230321 100644 --- a/plugins/chat/assets/javascripts/discourse/routes/chat.js +++ b/plugins/chat/assets/javascripts/discourse/routes/chat.js @@ -9,14 +9,27 @@ export default class ChatRoute extends DiscourseRoute { @service chat; @service router; @service fullPageChat; + @service chatPreferredMode; titleToken() { return I18n.t("chat.title_capitalized"); } - beforeModel() { + beforeModel(transition) { + if ( + transition.from && // don't intercept when directly loading chat + this.chatPreferredMode.isDrawer && + transition.intent?.name === "chat.channel" // sidebar can only load a channel + ) { + transition.abort(); + const id = transition.intent.contexts[0]; + return this.chat.getChannelBy("id", id).then((channel) => { + this.appEvents.trigger("chat:open-channel", channel); + }); + } + if (!this.chat.userCanChat) { - return this.transitionTo(`discovery.${defaultHomepage()}`); + return this.router.transitionTo(`discovery.${defaultHomepage()}`); } this.fullPageChat.enter(this.router.currentURL); @@ -34,6 +47,7 @@ export default class ChatRoute extends DiscourseRoute { deactivate() { this.fullPageChat.exit(); this.chat.setActiveChannel(null); + schedule("afterRender", () => { document.body.classList.remove("has-full-page-chat"); document.documentElement.classList.remove("has-full-page-chat"); 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 56ad082c6a..9b7d2cd6f9 100644 --- a/plugins/chat/assets/javascripts/discourse/services/full-page-chat.js +++ b/plugins/chat/assets/javascripts/discourse/services/full-page-chat.js @@ -1,6 +1,9 @@ -import Service from "@ember/service"; +import Service, { inject as service } from "@ember/service"; +import { defaultHomepage } from "discourse/lib/utilities"; export default class FullPageChat extends Service { + @service router; + _previousURL = null; _isActive = false; @@ -10,8 +13,20 @@ export default class FullPageChat extends Service { } exit() { + if (this.isDestroyed || this.isDestroying) { + return; + } + this._isActive = false; - return this._previousURL; + + let previousURL = this._previousURL; + if (!previousURL || previousURL === "/") { + previousURL = this.router.urlFor(`discovery.${defaultHomepage()}`); + } + + this._previousURL = null; + + return previousURL; } get isActive() { diff --git a/plugins/chat/spec/system/navigation_spec.rb b/plugins/chat/spec/system/navigation_spec.rb index 4bc4e41f57..12ca87100d 100644 --- a/plugins/chat/spec/system/navigation_spec.rb +++ b/plugins/chat/spec/system/navigation_spec.rb @@ -94,4 +94,36 @@ RSpec.describe "Navigation", type: :system, js: true do expect(page).to have_css(".chat-message-container[data-id='#{message.id}']") end end + + context "when sidebar is enabled" do + before do + SiteSetting.enable_experimental_sidebar_hamburger = true + SiteSetting.enable_sidebar = true + end + + context "when opening channel from sidebar with drawer preferred" do + it "opens channel in drawer" do + visit("/t/-/#{topic.id}") + chat_page.open_from_header + chat_page.close_drawer + find("a[title='#{category_channel.title}']").click + + expect(page).to have_css(".chat-message-container[data-id='#{message.id}']") + end + end + + context "when opening channel from sidebar with full page preferred" do + it "opens channel in full page" do + visit("/") + chat_page.open_from_header + chat_page.maximize_drawer + visit("/") + find("a[title='#{category_channel.title}']").click + + expect(page).to have_current_path( + chat.channel_path(category_channel.id, category_channel.slug), + ) + end + end + end end diff --git a/plugins/chat/spec/system/page_objects/chat/chat.rb b/plugins/chat/spec/system/page_objects/chat/chat.rb index a2e9e840ae..bed05c97bd 100644 --- a/plugins/chat/spec/system/page_objects/chat/chat.rb +++ b/plugins/chat/spec/system/page_objects/chat/chat.rb @@ -15,6 +15,10 @@ module PageObjects find(".topic-chat-drawer-header__full-screen-btn").click end + def close_drawer + find(".topic-chat-drawer-header__close-btn").click + end + def minimize_full_page find(".chat-full-screen-button").click end 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 798370f8ed..a893799cd2 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 @@ -1,9 +1,11 @@ import { module, test } from "qunit"; -import { getOwner } from "discourse-common/lib/get-owner"; +import { setupTest } from "ember-qunit"; module("Discourse Chat | Unit | Service | full-page-chat", function (hooks) { + setupTest(hooks); + hooks.beforeEach(function () { - this.fullPageChat = getOwner(this).lookup("service:full-page-chat"); + this.fullPageChat = this.owner.lookup("service:full-page-chat"); }); hooks.afterEach(function () {