From 18c81958e5f581d9d03a6707bac717be20c40439 Mon Sep 17 00:00:00 2001
From: Joffrey JAFFEUX <j.jaffeux@gmail.com>
Date: Thu, 13 Apr 2023 10:08:12 +0200
Subject: [PATCH] UX: various tweaks on thread (#21083)

- Back button in drawer will bring you back to channel
- Larger font for thread indicator
- Prevents screen flashing due to clearing messages when they were already loaded
- Fixes a bug where did-update params were inverted causing an error when expanding/collapsing drawer
---
 .../chat-drawer/header/back-link.hbs          |  7 +++---
 .../chat-drawer/header/left-actions.hbs       |  5 +++-
 .../components/chat-drawer/thread.hbs         | 17 +++++++++----
 .../discourse/components/chat-live-pane.hbs   |  1 +
 .../discourse/components/chat-thread.hbs      | 24 ++++++++++---------
 .../discourse/components/chat-thread.js       |  3 ++-
 .../templates/chat-channel-thread.hbs         |  2 +-
 .../stylesheets/common/chat-drawer.scss       |  2 +-
 .../common/chat-message-thread-indicator.scss |  5 ++--
 plugins/chat/spec/system/navigation_spec.rb   |  2 +-
 .../page_objects/chat_drawer/chat_drawer.rb   |  8 +++++--
 .../chat/spec/system/single_thread_spec.rb    | 12 ++++++++++
 12 files changed, 59 insertions(+), 29 deletions(-)

diff --git a/plugins/chat/assets/javascripts/discourse/components/chat-drawer/header/back-link.hbs b/plugins/chat/assets/javascripts/discourse/components/chat-drawer/header/back-link.hbs
index 9463ae6f084..2d2c0243581 100644
--- a/plugins/chat/assets/javascripts/discourse/components/chat-drawer/header/back-link.hbs
+++ b/plugins/chat/assets/javascripts/discourse/components/chat-drawer/header/back-link.hbs
@@ -1,7 +1,8 @@
 <LinkTo
-  title={{i18n "chat.return_to_list"}}
-  class="chat-drawer-header__return-to-channels-btn"
-  @route="chat"
+  title={{@title}}
+  class="chat-drawer-header__back-btn"
+  @route={{@route}}
+  @models={{or @routeModels (array)}}
 >
   {{d-icon "chevron-left"}}
 </LinkTo>
\ No newline at end of file
diff --git a/plugins/chat/assets/javascripts/discourse/components/chat-drawer/header/left-actions.hbs b/plugins/chat/assets/javascripts/discourse/components/chat-drawer/header/left-actions.hbs
index 6da5ccc2ef9..93917adec23 100644
--- a/plugins/chat/assets/javascripts/discourse/components/chat-drawer/header/left-actions.hbs
+++ b/plugins/chat/assets/javascripts/discourse/components/chat-drawer/header/left-actions.hbs
@@ -1,7 +1,10 @@
 {{#if this.chatStateManager.isDrawerExpanded}}
   <div class="chat-drawer-header__left-actions">
     <div class="chat-drawer-header__top-line">
-      <ChatDrawer::Header::BackLink />
+      <ChatDrawer::Header::BackLink
+        @route="chat"
+        @title={{i18n "chat.return_to_list"}}
+      />
     </div>
   </div>
 {{/if}}
\ No newline at end of file
diff --git a/plugins/chat/assets/javascripts/discourse/components/chat-drawer/thread.hbs b/plugins/chat/assets/javascripts/discourse/components/chat-drawer/thread.hbs
index 3b2e19a4abd..b448a84a32f 100644
--- a/plugins/chat/assets/javascripts/discourse/components/chat-drawer/thread.hbs
+++ b/plugins/chat/assets/javascripts/discourse/components/chat-drawer/thread.hbs
@@ -1,10 +1,17 @@
 <ChatDrawer::Header @toggleExpand={{@drawerActions.toggleExpand}}>
-  <ChatDrawer::Header::LeftActions />
+  {{#if (and this.chatStateManager.isDrawerExpanded this.chat.activeChannel)}}
+    <div class="chat-drawer-header__left-actions">
+      <div class="chat-drawer-header__top-line">
+        <ChatDrawer::Header::BackLink
+          @route="chat.channel"
+          @title={{i18n "chat.return_to_list"}}
+          @routeModels={{this.chat.activeChannel.routeModels}}
+        />
+      </div>
+    </div>
+  {{/if}}
 
-  <ChatDrawer::Header::ChannelTitle
-    @channel={{this.chat.activeChannel}}
-    @drawerActions={{@drawerActions}}
-  />
+  <ChatDrawer::Header::Title @title="chat.thread.label" />
 
   <ChatDrawer::Header::RightActions @drawerActions={{@drawerActions}} />
 </ChatDrawer::Header>
diff --git a/plugins/chat/assets/javascripts/discourse/components/chat-live-pane.hbs b/plugins/chat/assets/javascripts/discourse/components/chat-live-pane.hbs
index b00f0efc748..5681ea0b925 100644
--- a/plugins/chat/assets/javascripts/discourse/components/chat-live-pane.hbs
+++ b/plugins/chat/assets/javascripts/discourse/components/chat-live-pane.hbs
@@ -12,6 +12,7 @@
   {{did-update this.updateChannel @channel.id}}
   {{did-insert this.addAutoFocusEventListener}}
   {{will-destroy this.removeAutoFocusEventListener}}
+  data-id={{@channel.id}}
 >
   <ChatFullPageHeader
     @channel={{@channel}}
diff --git a/plugins/chat/assets/javascripts/discourse/components/chat-thread.hbs b/plugins/chat/assets/javascripts/discourse/components/chat-thread.hbs
index 61e6df014c3..baaa67e3f9c 100644
--- a/plugins/chat/assets/javascripts/discourse/components/chat-thread.hbs
+++ b/plugins/chat/assets/javascripts/discourse/components/chat-thread.hbs
@@ -2,18 +2,20 @@
   class={{concat-class "chat-thread" (if this.loading "loading")}}
   data-id={{this.thread.id}}
   {{did-insert this.loadMessages}}
-  {{did-update this.thread.id this.loadMessages}}
+  {{did-update this.loadMessages this.thread.id}}
 >
-  <div class="chat-thread__header">
-    <span class="chat-thread__label">{{i18n "chat.thread.label"}}</span>
-    <LinkTo
-      class="chat-thread__close"
-      @route="chat.channel"
-      @models={{this.chat.activeChannel.routeModels}}
-    >
-      {{d-icon "times"}}
-    </LinkTo>
-  </div>
+  {{#if @includeHeader}}
+    <div class="chat-thread__header">
+      <span class="chat-thread__label">{{i18n "chat.thread.label"}}</span>
+      <LinkTo
+        class="chat-thread__close"
+        @route="chat.channel"
+        @models={{this.chat.activeChannel.routeModels}}
+      >
+        {{d-icon "times"}}
+      </LinkTo>
+    </div>
+  {{/if}}
 
   <div
     class="chat-thread__body popper-viewport"
diff --git a/plugins/chat/assets/javascripts/discourse/components/chat-thread.js b/plugins/chat/assets/javascripts/discourse/components/chat-thread.js
index eb1e12521c8..9b147442f55 100644
--- a/plugins/chat/assets/javascripts/discourse/components/chat-thread.js
+++ b/plugins/chat/assets/javascripts/discourse/components/chat-thread.js
@@ -44,6 +44,8 @@ export default class ChatThreadPanel extends Component {
 
   @action
   loadMessages() {
+    this.thread.messagesManager.clearMessages();
+
     if (this.args.targetMessageId) {
       this.requestedTargetMessageId = parseInt(this.args.targetMessageId, 10);
     }
@@ -73,7 +75,6 @@ export default class ChatThreadPanel extends Component {
 
     this.loadingMorePast = true;
     this.loading = true;
-    this.thread.messagesManager.clearMessages();
 
     const findArgs = { pageSize: PAGE_SIZE };
 
diff --git a/plugins/chat/assets/javascripts/discourse/templates/chat-channel-thread.hbs b/plugins/chat/assets/javascripts/discourse/templates/chat-channel-thread.hbs
index 52c32e8d5ae..6a0689fc057 100644
--- a/plugins/chat/assets/javascripts/discourse/templates/chat-channel-thread.hbs
+++ b/plugins/chat/assets/javascripts/discourse/templates/chat-channel-thread.hbs
@@ -1,2 +1,2 @@
 {{! ChatThreadList will go here later }}
-<ChatThread />
\ No newline at end of file
+<ChatThread @includeHeader={{true}} />
\ No newline at end of file
diff --git a/plugins/chat/assets/stylesheets/common/chat-drawer.scss b/plugins/chat/assets/stylesheets/common/chat-drawer.scss
index ffc08c688b7..296491de2c9 100644
--- a/plugins/chat/assets/stylesheets/common/chat-drawer.scss
+++ b/plugins/chat/assets/stylesheets/common/chat-drawer.scss
@@ -204,7 +204,7 @@ a.chat-drawer-header__title {
   }
 
   &__close-btn,
-  &__return-to-channels-btn,
+  &__back-btn,
   &__full-screen-btn,
   &__expand-btn {
     height: 30px;
diff --git a/plugins/chat/assets/stylesheets/common/chat-message-thread-indicator.scss b/plugins/chat/assets/stylesheets/common/chat-message-thread-indicator.scss
index 2308b9a92ea..82e5a2b8e48 100644
--- a/plugins/chat/assets/stylesheets/common/chat-message-thread-indicator.scss
+++ b/plugins/chat/assets/stylesheets/common/chat-message-thread-indicator.scss
@@ -18,12 +18,11 @@
 
   &__replies-count {
     color: var(--primary-medium);
-
-    font-size: var(--font-down-2);
+    font-size: var(--font-down-1);
   }
 
   &__view-thread {
-    font-size: var(--font-down-2);
+    font-size: var(--font-down-1);
 
     .chat-message-thread-indicator:hover & {
       text-decoration: underline;
diff --git a/plugins/chat/spec/system/navigation_spec.rb b/plugins/chat/spec/system/navigation_spec.rb
index 003c173cba4..14a3117eabf 100644
--- a/plugins/chat/spec/system/navigation_spec.rb
+++ b/plugins/chat/spec/system/navigation_spec.rb
@@ -216,7 +216,7 @@ RSpec.describe "Navigation", type: :system, js: true do
         visit("/")
         chat_page.open_from_header
         chat_drawer_page.open_channel(category_channel_2)
-        chat_drawer_page.open_index
+        chat_drawer_page.back
         chat_drawer_page.close
         chat_page.open_from_header
 
diff --git a/plugins/chat/spec/system/page_objects/chat_drawer/chat_drawer.rb b/plugins/chat/spec/system/page_objects/chat_drawer/chat_drawer.rb
index f5bb23bcfd5..64f5816abbc 100644
--- a/plugins/chat/spec/system/page_objects/chat_drawer/chat_drawer.rb
+++ b/plugins/chat/spec/system/page_objects/chat_drawer/chat_drawer.rb
@@ -16,8 +16,8 @@ module PageObjects
         find("#{VISIBLE_DRAWER} .chat-drawer-header__close-btn").click
       end
 
-      def open_index
-        find("#{VISIBLE_DRAWER} .chat-drawer-header__return-to-channels-btn").click
+      def back
+        find("#{VISIBLE_DRAWER} .chat-drawer-header__back-btn").click
       end
 
       def open_channel(channel)
@@ -34,6 +34,10 @@ module PageObjects
       def has_open_thread?(thread)
         has_css?("#{VISIBLE_DRAWER} .chat-thread[data-id='#{thread.id}']")
       end
+
+      def has_open_channel?(channel)
+        has_css?("#{VISIBLE_DRAWER} .chat-live-pane[data-id='#{channel.id}']")
+      end
     end
   end
 end
diff --git a/plugins/chat/spec/system/single_thread_spec.rb b/plugins/chat/spec/system/single_thread_spec.rb
index ebacbf7cb0f..c5e56cb0b69 100644
--- a/plugins/chat/spec/system/single_thread_spec.rb
+++ b/plugins/chat/spec/system/single_thread_spec.rb
@@ -58,6 +58,18 @@ describe "Single thread in side panel", type: :system, js: true do
       expect(chat_drawer_page).to have_open_thread(thread)
     end
 
+    it "navigates back to the channel when clicking back button from a thread" do
+      visit("/latest")
+      chat_page.open_from_header
+      chat_drawer_page.open_channel(channel)
+      channel_page.message_thread_indicator(thread.original_message).click
+      expect(chat_drawer_page).to have_open_thread(thread)
+
+      chat_drawer_page.back
+
+      expect(chat_drawer_page).to have_open_channel(channel)
+    end
+
     it "opens the side panel for a single thread from the indicator" do
       chat_page.visit_channel(channel)
       channel_page.message_thread_indicator(thread.original_message).click