FEATURE: Conditionally change back button route for thread (#22129)

When clicking back from a thread, we want to either go back to the
channel if the thread was opened from an indicator, or to the thread
list if we opened it from there. Since ember doesn't give a nice way
to get the previous route, we need to store this ourselves. We only
do this on mobile, on desktop we just follow existing behaviour.

Also implements a chat router history.

---------

Co-authored-by: Joffrey JAFFEUX <j.jaffeux@gmail.com>

plugins/chat/assets/javascripts/discourse/components/chat-drawer/thread.js

@@ -7,14 +7,14 @@ export default class ChatDrawerThread extends Component {
   @service chat;
   @service chatStateManager;
   @service chatChannelsManager;
-  @service chatDrawerRouter;
+  @service chatHistory;
 
   get backLink() {
     const link = {
       models: this.chat.activeChannel.routeModels,
     };
 
-    if (this.chatDrawerRouter.previousRoute?.name === "chat.channel.threads") {
+    if (this.chatHistory.previousRoute?.name === "chat.channel.threads") {
       link.title = "chat.return_to_threads_list";
       link.route = "chat.channel.threads";
     } else {

plugins/chat/assets/javascripts/discourse/components/chat/thread/header.hbs

@@ -2,8 +2,9 @@
   <div class="chat-thread-header__left-buttons">
     {{#if @thread}}
       <LinkTo
-        class="chat-thread__back-to-list btn-flat btn btn-icon no-text"
+        class="chat-thread__back-to-previous-route btn-flat btn btn-icon no-text"
-        @route="chat.channel.threads"
+        @route={{this.backLink.route}}
+        @models={{this.backLink.models}}
         title={{i18n "chat.return_to_threads_list"}}
       >
         <Chat::Thread::HeaderUnreadIndicator @channel={{@thread.channel}} />

plugins/chat/assets/javascripts/discourse/components/chat/thread/header.js

@@ -11,9 +11,29 @@ export default class ChatThreadHeader extends Component {
   @service currentUser;
   @service chatApi;
   @service router;
+  @service chatStateManager;
+  @service chatHistory;
+  @service site;
 
   @tracked persistedNotificationLevel = true;
 
+  get backLink() {
+    if (
+      this.chatHistory.previousRoute?.name === "chat.channel.index" &&
+      this.site.mobileView
+    ) {
+      return {
+        route: "chat.channel.index",
+        models: this.args.channel.routeModels,
+      };
+    } else {
+      return {
+        route: "chat.channel.threads",
+        models: [],
+      };
+    }
+  }
+
   get label() {
     return this.args.thread.escapedTitle;
   }

plugins/chat/assets/javascripts/discourse/initializers/chat-setup.js

@@ -18,7 +18,9 @@ export default {
   before: "hashtag-css-generator",
 
   initialize(container) {
+    this.router = container.lookup("service:router");
     this.chatService = container.lookup("service:chat");
+    this.chatHistory = container.lookup("service:chat-history");
     this.site = container.lookup("service:site");
     this.siteSettings = container.lookup("service:site-settings");
     this.currentUser = container.lookup("service:current-user");
@@ -30,6 +32,13 @@ export default {
     }
 
     withPluginApi("0.12.1", (api) => {
+      api.onPageChange((path) => {
+        const route = this.router.recognize(path);
+        if (route.name.startsWith("chat.")) {
+          this.chatHistory.visit(route);
+        }
+      });
+
       api.registerHashtagType("channel", new ChannelHashtagType(container));
 
       api.registerChatComposerButton({

plugins/chat/assets/javascripts/discourse/routes/chat-channel-thread.js

@@ -24,7 +24,6 @@ export default class ChatChannelThread extends DiscourseRoute {
 
   afterModel(model) {
     this.chat.activeChannel.activeThread = model;
-    this.chatThreadPane.open(model);
   }
 
   @action

plugins/chat/assets/javascripts/discourse/routes/chat-channel-threads.js

@@ -25,8 +25,4 @@ export default class ChatChannelThreads extends DiscourseRoute {
       this.chatStateManager.closeSidePanel();
     }
   }
-
-  activate() {
-    this.chatThreadListPane.open();
-  }
 }

plugins/chat/assets/javascripts/discourse/services/chat-drawer-router.js

@@ -49,30 +49,16 @@ const ROUTES = {
 
 export default class ChatDrawerRouter extends Service {
   @service router;
+  @service chatHistory;
+
   @tracked component = null;
   @tracked drawerRoute = null;
   @tracked params = null;
-  @tracked history = [];
-
-  get previousRoute() {
-    if (this.history.length > 1) {
-      return this.history[this.history.length - 2];
-    }
-  }
-
-  get currentRoute() {
-    if (this.history.length > 0) {
-      return this.history[this.history.length - 1];
-    }
-  }
 
   stateFor(route) {
-    this.drawerRoute?.deactivate?.(this.currentRoute);
+    this.drawerRoute?.deactivate?.(this.chatHistory.currentRoute);
 
-    this.history.push(route);
+    this.chatHistory.visit(route);
-    if (this.history.length > 10) {
-      this.history.shift();
-    }
 
     this.drawerRoute = ROUTES[route.name];
     this.params = this.drawerRoute?.extractParams?.(route) || route.params;

plugins/chat/assets/javascripts/discourse/services/chat-history.js

@@ -0,0 +1,22 @@
+import Service from "@ember/service";
+import { tracked } from "@glimmer/tracking";
+
+export default class ChatHistory extends Service {
+  @tracked history;
+
+  get previousRoute() {
+    if (this.history?.length > 1) {
+      return this.history[this.history.length - 2];
+    }
+  }
+
+  get currentRoute() {
+    if (this.history?.length > 0) {
+      return this.history[this.history.length - 1];
+    }
+  }
+
+  visit(route) {
+    this.history = (this.history || []).slice(-9).concat([route]);
+  }
+}

plugins/chat/assets/javascripts/discourse/services/chat-state-manager.js

@@ -21,11 +21,13 @@ export function resetChatDrawerStateCallbacks() {
 }
 export default class ChatStateManager extends Service {
   @service chat;
+  @service chatHistory;
   @service router;
 
   @tracked isSidePanelExpanded = false;
   @tracked isDrawerExpanded = false;
   @tracked isDrawerActive = false;
+
   @tracked _chatURL = null;
   @tracked _appURL = null;
 

plugins/chat/assets/stylesheets/common/chat-thread-header.scss

@@ -8,7 +8,7 @@
   align-items: center;
   padding-inline: 0.5rem;
 
-  .chat-thread__back-to-list {
+  .chat-thread__back-to-previous-route {
     padding: 0.5rem 0;
     margin-right: 0.5rem;
   }

plugins/chat/spec/system/navigation_spec.rb

@@ -4,18 +4,22 @@ RSpec.describe "Navigation", type: :system do
   fab!(:category) { Fabricate(:category) }
   fab!(:topic) { Fabricate(:topic) }
   fab!(:post) { Fabricate(:post, topic: topic) }
-  fab!(:user) { Fabricate(:admin) }
+  fab!(:current_user) { Fabricate(:admin) }
   fab!(:category_channel) { Fabricate(:category_channel) }
   fab!(:category_channel_2) { Fabricate(:category_channel) }
   fab!(:message) { Fabricate(:chat_message, chat_channel: category_channel) }
   let(:chat_page) { PageObjects::Pages::Chat.new }
+  let(:thread_page) { PageObjects::Pages::ChatThread.new }
+  let(:thread_list_page) { PageObjects::Components::Chat::ThreadList.new }
+  let(:channel_page) { PageObjects::Pages::ChatChannel.new }
+  let(:side_panel_page) { PageObjects::Pages::ChatSidePanel.new }
   let(:sidebar_page) { PageObjects::Pages::Sidebar.new }
   let(:sidebar_component) { PageObjects::Components::Sidebar.new }
   let(:chat_drawer_page) { PageObjects::Pages::ChatDrawer.new }
 
   before do
-    chat_system_bootstrap(user, [category_channel, category_channel_2])
+    chat_system_bootstrap(current_user, [category_channel, category_channel_2])
-    sign_in(user)
+    sign_in(current_user)
   end
 
   context "when clicking chat icon and drawer is viewing channel" do
@@ -127,6 +131,66 @@ RSpec.describe "Navigation", type: :system do
     end
   end
 
+  context "when opening a thread" do
+    fab!(:thread) { Fabricate(:chat_thread, channel: category_channel) }
+
+    before do
+      SiteSetting.enable_experimental_chat_threaded_discussions = true
+      category_channel.update!(threading_enabled: true)
+      Fabricate(:chat_message, thread: thread)
+      thread.add(current_user)
+    end
+
+    context "when opening a thread from the thread list" do
+      it "goes back to the thread list when clicking the back button" do
+        visit("/chat")
+        chat_page.visit_channel(category_channel)
+        channel_page.open_thread_list
+        expect(thread_list_page).to have_loaded
+        thread_list_page.open_thread(thread)
+        expect(side_panel_page).to have_open_thread(thread)
+        thread_page.back_to_previous_route
+        expect(thread_list_page).to have_loaded
+      end
+
+      context "for mobile" do
+        it "goes back to the thread list when clicking the back button", mobile: true do
+          visit("/chat")
+          chat_page.visit_channel(category_channel)
+          channel_page.open_thread_list
+          expect(thread_list_page).to have_loaded
+          thread_list_page.open_thread(thread)
+          expect(side_panel_page).to have_open_thread(thread)
+          thread_page.back_to_previous_route
+          expect(thread_list_page).to have_loaded
+        end
+      end
+    end
+
+    context "when opening a thread from indicator" do
+      it "goes back to the thread list when clicking the back button" do
+        visit("/chat")
+        chat_page.visit_channel(category_channel)
+        channel_page.message_thread_indicator(thread.original_message).click
+        expect(side_panel_page).to have_open_thread(thread)
+        thread_page.back_to_previous_route
+        expect(thread_list_page).to have_loaded
+      end
+
+      context "for mobile" do
+        it "closes the thread and goes back to the channel when clicking the back button",
+           mobile: true do
+          visit("/chat")
+          chat_page.visit_channel(category_channel)
+          channel_page.message_thread_indicator(thread.original_message).click
+          expect(side_panel_page).to have_open_thread(thread)
+          thread_page.back_to_previous_route
+          expect(side_panel_page).not_to be_open
+        end
+      end
+    end
+  end
+
   context "when sidebar is configured as the navigation menu" do
     before { SiteSetting.navigation_menu = "sidebar" }
 
@@ -244,7 +308,7 @@ RSpec.describe "Navigation", type: :system do
 
     context "when opening a channel in full page" do
       fab!(:other_user) { Fabricate(:user) }
-      fab!(:dm_channel) { Fabricate(:direct_message_channel, users: [user, other_user]) }
+      fab!(:dm_channel) { Fabricate(:direct_message_channel, users: [current_user, other_user]) }
 
       it "activates the channel in the sidebar" do
         visit("/chat/c/#{category_channel.slug}/#{category_channel.id}")

plugins/chat/spec/system/page_objects/chat/chat_side_panel.rb

@@ -3,6 +3,10 @@
 module PageObjects
   module Pages
     class ChatSidePanel < PageObjects::Pages::Base
+      def open?
+        has_css?(".chat-side-panel")
+      end
+
       def has_open_thread?(thread = nil)
         if thread
           has_css?(".chat-side-panel .chat-thread[data-id='#{thread.id}']")

plugins/chat/spec/system/page_objects/chat/chat_thread.rb

@@ -58,17 +58,17 @@ module PageObjects
         header.find(".chat-thread__close").click
       end
 
-      def back_to_list
+      def back_to_previous_route
-        header.find(".chat-thread__back-to-list").click
+        header.find(".chat-thread__back-to-previous-route").click
       end
 
       def has_no_unread_list_indicator?
-        has_no_css?(".chat-thread__back-to-list .chat-thread-header-unread-indicator")
+        has_no_css?(".chat-thread__back-to-previous-route .chat-thread-header-unread-indicator")
       end
 
       def has_unread_list_indicator?(count:)
         has_css?(
-          ".chat-thread__back-to-list .chat-thread-header-unread-indicator  .chat-thread-header-unread-indicator__number",
+          ".chat-thread__back-to-previous-route .chat-thread-header-unread-indicator  .chat-thread-header-unread-indicator__number",
           text: count.to_s,
         )
       end

plugins/chat/spec/system/page_objects/chat/components/thread_list.rb

@@ -15,6 +15,10 @@ module PageObjects
           component.has_no_css?(".spinner")
         end
 
+        def open_thread(thread)
+          item_by_id(thread.id).click
+        end
+
         def has_thread?(thread)
           item_by_id(thread.id)
         end

plugins/chat/spec/system/thread_tracking/full_page_spec.rb

@@ -40,7 +40,7 @@ describe "Thread tracking state | full page", type: :system do
       channel_page.open_thread_list
       thread_list_page.item_by_id(thread.id).click
       expect(thread_page).to have_no_unread_list_indicator
-      thread_page.back_to_list
+      thread_page.back_to_previous_route
       expect(thread_list_page).to have_no_unread_item(thread.id)
     end