DEV: rework the chat-live-pane

This PR is introducing glimmer usage in the chat-live-pane, for components but also for models. RestModel usage has been dropped in favor of native classes.

Other changes/additions in this PR:

- sticky dates, scrolling will now keep the date separator of the current section at the top of the screen
- better unread management, marking a channel as unread will correctly mark the correct message and not mark the whole channel as read. Tracking state will also now correctly return unread count and unread mentions.
- adds an animation on bottom arrow
- better scrolling behavior, we should now always correctly keep the scroll position while loading more
- reactions are now more reactive, and will update their tooltip without needed to close/reopen it
- skeleton has been improved with placeholder images and reactions
- when making a reaction on the desktop message actions, the menu won't move anymore
- simplify logic and stop maintaining a list of unloaded messages

plugins/chat/spec/queries/chat_channel_memberships_query_spec.rb

@@ -17,7 +17,7 @@ describe ChatChannelMembershipsQuery do
 
       context "when no memberships exists" do
         it "returns an empty array" do
-          expect(described_class.call(channel_1)).to eq([])
+          expect(described_class.call(channel: channel_1)).to eq([])
         end
       end
 
@@ -28,7 +28,7 @@ describe ChatChannelMembershipsQuery do
         end
 
         it "returns the memberships" do
-          memberships = described_class.call(channel_1)
+          memberships = described_class.call(channel: channel_1)
 
           expect(memberships.pluck(:user_id)).to contain_exactly(user_1.id, user_2.id)
         end
@@ -49,7 +49,7 @@ describe ChatChannelMembershipsQuery do
           end
 
           it "lists the user" do
-            memberships = described_class.call(channel_1)
+            memberships = described_class.call(channel: channel_1)
 
             expect(memberships.pluck(:user_id)).to include(user_1.id)
           end
@@ -62,14 +62,16 @@ describe ChatChannelMembershipsQuery do
               permission_type: CategoryGroup.permission_types[:full],
             )
 
-            expect(described_class.call(channel_1).pluck(:user_id)).to contain_exactly(user_1.id)
+            expect(described_class.call(channel: channel_1).pluck(:user_id)).to contain_exactly(
+              user_1.id,
+            )
           end
 
           it "returns the membership if the user still has access through a staff group" do
             chatters_group.remove(user_1)
             Group.find_by(id: Group::AUTO_GROUPS[:staff]).add(user_1)
 
-            memberships = described_class.call(channel_1)
+            memberships = described_class.call(channel: channel_1)
 
             expect(memberships.pluck(:user_id)).to include(user_1.id)
           end
@@ -77,7 +79,7 @@ describe ChatChannelMembershipsQuery do
 
         context "when membership doesn’t exist" do
           it "doesn’t list the user" do
-            memberships = described_class.call(channel_1)
+            memberships = described_class.call(channel: channel_1)
 
             expect(memberships.pluck(:user_id)).to be_empty
           end
@@ -91,7 +93,7 @@ describe ChatChannelMembershipsQuery do
           end
 
           it "doesn’t list the user" do
-            memberships = described_class.call(channel_1)
+            memberships = described_class.call(channel: channel_1)
 
             expect(memberships).to be_empty
           end
@@ -99,7 +101,7 @@ describe ChatChannelMembershipsQuery do
 
         context "when membership doesn’t exist" do
           it "doesn’t list the user" do
-            memberships = described_class.call(channel_1)
+            memberships = described_class.call(channel: channel_1)
 
             expect(memberships).to be_empty
           end
@@ -114,7 +116,7 @@ describe ChatChannelMembershipsQuery do
         end
 
         it "returns an empty array" do
-          expect(described_class.call(channel_1)).to eq([])
+          expect(described_class.call(channel: channel_1)).to eq([])
         end
       end
 
@@ -122,7 +124,7 @@ describe ChatChannelMembershipsQuery do
         fab!(:channel_1) { Fabricate(:direct_message_channel, users: [user_1, user_2]) }
 
         it "returns the memberships" do
-          memberships = described_class.call(channel_1)
+          memberships = described_class.call(channel: channel_1)
 
           expect(memberships.pluck(:user_id)).to contain_exactly(user_1.id, user_2.id)
         end
@@ -139,7 +141,7 @@ describe ChatChannelMembershipsQuery do
 
       describe "offset param" do
         it "offsets the results" do
-          memberships = described_class.call(channel_1, offset: 1)
+          memberships = described_class.call(channel: channel_1, offset: 1)
 
           expect(memberships.length).to eq(1)
         end
@@ -147,7 +149,7 @@ describe ChatChannelMembershipsQuery do
 
       describe "limit param" do
         it "limits the results" do
-          memberships = described_class.call(channel_1, limit: 1)
+          memberships = described_class.call(channel: channel_1, limit: 1)
 
           expect(memberships.length).to eq(1)
         end
@@ -163,7 +165,7 @@ describe ChatChannelMembershipsQuery do
       end
 
       it "filters the results" do
-        memberships = described_class.call(channel_1, username: user_1.username)
+        memberships = described_class.call(channel: channel_1, username: user_1.username)
 
         expect(memberships.length).to eq(1)
         expect(memberships[0].user).to eq(user_1)
@@ -182,7 +184,7 @@ describe ChatChannelMembershipsQuery do
         before { SiteSetting.prioritize_username_in_ux = true }
 
         it "is using ascending order on username" do
-          memberships = described_class.call(channel_1)
+          memberships = described_class.call(channel: channel_1)
 
           expect(memberships[0].user).to eq(user_1)
           expect(memberships[1].user).to eq(user_2)
@@ -193,7 +195,7 @@ describe ChatChannelMembershipsQuery do
         before { SiteSetting.prioritize_username_in_ux = false }
 
         it "is using ascending order on name" do
-          memberships = described_class.call(channel_1)
+          memberships = described_class.call(channel: channel_1)
 
           expect(memberships[0].user).to eq(user_2)
           expect(memberships[1].user).to eq(user_1)
@@ -203,7 +205,7 @@ describe ChatChannelMembershipsQuery do
           before { SiteSetting.enable_names = false }
 
           it "is using ascending order on username" do
-            memberships = described_class.call(channel_1)
+            memberships = described_class.call(channel: channel_1)
 
             expect(memberships[0].user).to eq(user_1)
             expect(memberships[1].user).to eq(user_2)
@@ -222,7 +224,7 @@ describe ChatChannelMembershipsQuery do
     end
 
     it "doesn’t list staged users" do
-      memberships = described_class.call(channel_1)
+      memberships = described_class.call(channel: channel_1)
       expect(memberships).to be_blank
     end
   end
@@ -242,7 +244,7 @@ describe ChatChannelMembershipsQuery do
     end
 
     it "doesn’t list suspended users" do
-      memberships = described_class.call(channel_1)
+      memberships = described_class.call(channel: channel_1)
       expect(memberships).to be_blank
     end
   end
@@ -260,7 +262,7 @@ describe ChatChannelMembershipsQuery do
     end
 
     it "doesn’t list inactive users" do
-      memberships = described_class.call(channel_1)
+      memberships = described_class.call(channel: channel_1)
       expect(memberships).to be_blank
     end
   end

plugins/chat/spec/queries/chat_channel_unreads_query_spec.rb

@@ -0,0 +1,51 @@
+# frozen_string_literal: true
+
+require "rails_helper"
+
+describe ChatChannelUnreadsQuery do
+  fab!(:channel_1) { Fabricate(:category_channel) }
+  fab!(:current_user) { Fabricate(:user) }
+
+  before do
+    SiteSetting.chat_enabled = true
+    SiteSetting.chat_allowed_groups = Group::AUTO_GROUPS[:everyone]
+    channel_1.add(current_user)
+  end
+
+  context "with unread message" do
+    it "returns a correct unread count" do
+      Fabricate(:chat_message, chat_channel: channel_1)
+
+      expect(described_class.call(channel_id: channel_1.id, user_id: current_user.id)).to eq(
+        { mention_count: 0, unread_count: 1 },
+      )
+    end
+  end
+
+  context "with unread mentions" do
+    before { Jobs.run_immediately! }
+
+    it "returns a correct unread mention" do
+      message = Fabricate(:chat_message)
+      notification =
+        Notification.create!(
+          notification_type: Notification.types[:chat_mention],
+          user_id: current_user.id,
+          data: { chat_message_id: message.id, chat_channel_id: channel_1.id }.to_json,
+        )
+      ChatMention.create!(notification: notification, user: current_user, chat_message: message)
+
+      expect(described_class.call(channel_id: channel_1.id, user_id: current_user.id)).to eq(
+        { mention_count: 1, unread_count: 0 },
+      )
+    end
+  end
+
+  context "with nothing unread" do
+    it "returns a correct state" do
+      expect(described_class.call(channel_id: channel_1.id, user_id: current_user.id)).to eq(
+        { mention_count: 0, unread_count: 0 },
+      )
+    end
+  end
+end

plugins/chat/spec/requests/chat_controller_spec.rb

@@ -126,15 +126,17 @@ RSpec.describe Chat::ChatController do
     it "correctly marks reactions as 'reacted' for the current_user" do
       heart_emoji = ":heart:"
       smile_emoji = ":smile"
-
       last_message = chat_channel.chat_messages.last
       last_message.reactions.create(user: user, emoji: heart_emoji)
       last_message.reactions.create(user: admin, emoji: smile_emoji)
 
       get "/chat/#{chat_channel.id}/messages.json", params: { page_size: page_size }
+
       reactions = response.parsed_body["chat_messages"].last["reactions"]
-      expect(reactions[heart_emoji]["reacted"]).to be true
-      expect(reactions[smile_emoji]["reacted"]).to be false
+      heart_reaction = reactions.find { |r| r["emoji"] == heart_emoji }
+      expect(heart_reaction["reacted"]).to be true
+      smile_reaction = reactions.find { |r| r["emoji"] == smile_emoji }
+      expect(smile_reaction["reacted"]).to be false
     end
 
     it "sends the last message bus id for the channel" do

plugins/chat/spec/serializer/chat_message_serializer_spec.rb

@@ -21,12 +21,14 @@ describe ChatMessageSerializer do
       it "doesn’t return the reaction" do
         Emoji.clear_cache
 
-        expect(subject.as_json[:reactions]["trout"]).to be_present
+        trout_reaction = subject.as_json[:reactions].find { |r| r[:emoji] == "trout" }
+        expect(trout_reaction).to be_present
 
         custom_emoji.destroy!
         Emoji.clear_cache
 
-        expect(subject.as_json[:reactions]["trout"]).to_not be_present
+        trout_reaction = subject.as_json[:reactions].find { |r| r[:emoji] == "trout" }
+        expect(trout_reaction).to_not be_present
       end
     end
   end

plugins/chat/spec/system/chat_channel_spec.rb

@@ -183,7 +183,7 @@ RSpec.describe "Chat channel", type: :system, js: true do
     it "shows a date separator" do
       chat.visit_channel(channel_1)
 
-      expect(page).to have_selector(".first-daily-message", text: "Today")
+      expect(page).to have_selector(".chat-message-separator__text", text: "Today")
     end
   end
 

plugins/chat/spec/system/create_channel_spec.rb

@@ -81,6 +81,7 @@ RSpec.describe "Create channel", type: :system, js: true do
         chat_page.visit_browse
         chat_page.new_channel_button.click
         channel_modal.select_category(private_category_1)
+        expect(page).to have_no_css(".loading-permissions")
 
         expect(channel_modal.create_channel_hint["innerHTML"].strip).to include(
           "<script>e</script>",

plugins/chat/spec/system/flag_message_spec.rb

@@ -32,7 +32,7 @@ RSpec.describe "Flag message", type: :system, js: true do
 
   context "when direct message channel" do
     fab!(:dm_channel_1) { Fabricate(:direct_message_channel, users: [current_user]) }
-    fab!(:message_1) { Fabricate(:chat_message, chat_channel: dm_channel_1, user: current_user) }
+    fab!(:message_1) { Fabricate(:chat_message, chat_channel: dm_channel_1) }
 
     it "doesn’t allow to flag a message" do
       chat.visit_channel(dm_channel_1)

plugins/chat/spec/system/message_user_info.rb

@@ -0,0 +1,60 @@
+# frozen_string_literal: true
+
+RSpec.describe "Sticky date", type: :system, js: true do
+  fab!(:current_user) { Fabricate(:user) }
+  fab!(:channel_1) { Fabricate(:category_channel) }
+
+  let(:chat_page) { PageObjects::Pages::Chat.new }
+
+  before do
+    chat_system_bootstrap
+    sign_in(current_user)
+  end
+
+  context "when previous message is from a different user" do
+    fab!(:message_1) { Fabricate(:chat_message, chat_channel: channel_1) }
+    fab!(:message_2) { Fabricate(:chat_message, chat_channel: channel_1) }
+
+    it "shows user info on the message" do
+      chat_page.visit_channel(channel_1)
+
+      expect(page.find("[data-id='#{message_2.id}']")).to have_css(".chat-message-avatar")
+    end
+  end
+
+  context "when previous message is from the same user" do
+    fab!(:message_1) { Fabricate(:chat_message, chat_channel: channel_1, user: current_user) }
+    fab!(:message_2) { Fabricate(:chat_message, chat_channel: channel_1, user: current_user) }
+
+    it "doesn’t show user info on the message" do
+      chat_page.visit_channel(channel_1)
+
+      expect(page.find("[data-id='#{message_2.id}']")).to have_no_css(".chat-message-avatar")
+    end
+
+    context "when previous message is old" do
+      fab!(:message_1) do
+        Fabricate(
+          :chat_message,
+          chat_channel: channel_1,
+          user: current_user,
+          created_at: DateTime.parse("2018-11-10 17:00"),
+        )
+      end
+      fab!(:message_2) do
+        Fabricate(
+          :chat_message,
+          chat_channel: channel_1,
+          user: current_user,
+          created_at: DateTime.parse("2018-11-10 17:30"),
+        )
+      end
+
+      it "shows user info on the message" do
+        chat_page.visit_channel(channel_1)
+
+        expect(page.find("[data-id='#{message_2.id}']")).to have_no_css(".chat-message-avatar")
+      end
+    end
+  end
+end

plugins/chat/spec/system/navigating_to_message_spec.rb

@@ -60,8 +60,9 @@ RSpec.describe "Navigating to message", type: :system, js: true do
 
       it "highlights the correct message after using the bottom arrow" do
         chat_page.visit_channel(channel_1)
+
         click_link(link)
-        click_link(I18n.t("js.chat.scroll_to_bottom"))
+        click_button(class: "chat-scroll-to-bottom")
         click_link(link)
 
         expect(page).to have_css(
@@ -149,8 +150,9 @@ RSpec.describe "Navigating to message", type: :system, js: true do
         visit("/")
         chat_page.open_from_header
         chat_drawer_page.open_channel(channel_1)
+
         click_link(link)
-        click_link(I18n.t("js.chat.scroll_to_bottom"))
+        click_button(class: "chat-scroll-to-bottom")
         click_link(link)
 
         expect(page).to have_css(

plugins/chat/spec/system/shortcuts/chat_composer_spec.rb

@@ -5,6 +5,7 @@ RSpec.describe "Shortcuts | chat composer", type: :system, js: true do
   fab!(:current_user) { Fabricate(:user) }
 
   let(:chat) { PageObjects::Pages::Chat.new }
+  let(:channel_page) { PageObjects::Pages::ChatChannel.new }
 
   KEY_MODIFIER = RUBY_PLATFORM =~ /darwin/i ? :meta : :control
 
@@ -63,8 +64,9 @@ RSpec.describe "Shortcuts | chat composer", type: :system, js: true do
 
     it "edits last editable message" do
       chat.visit_channel(channel_1)
+      expect(channel_page).to have_message(id: message_1.id)
 
-      within(".chat-composer-input") { |composer| composer.send_keys(:arrow_up) }
+      find(".chat-composer-input").send_keys(:arrow_up)
 
       expect(page.find(".chat-composer-message-details")).to have_content(message_1.message)
     end

plugins/chat/spec/system/sticky_date_spec.rb

@@ -0,0 +1,32 @@
+# frozen_string_literal: true
+
+RSpec.describe "Sticky date", type: :system, js: true do
+  fab!(:current_user) { Fabricate(:user) }
+  fab!(:channel_1) { Fabricate(:category_channel) }
+
+  let(:chat_page) { PageObjects::Pages::Chat.new }
+
+  before do
+    chat_system_bootstrap
+    channel_1.add(current_user)
+    20.times { Fabricate(:chat_message, chat_channel: channel_1, created_at: 1.day.ago) }
+    25.times { Fabricate(:chat_message, chat_channel: channel_1) }
+    sign_in(current_user)
+  end
+
+  context "when today separator is out of screen" do
+    it "shows it as a sticky date" do
+      chat_page.visit_channel(channel_1)
+
+      expect(page.find(".chat-message-separator__text-container.is-pinned")).to have_content(
+        I18n.t("js.chat.chat_message_separator.today"),
+      )
+      expect(page).to have_css(
+        ".chat-message-separator__text-container:not(.is-pinned)",
+        visible: :hidden,
+        text:
+          "#{I18n.t("js.chat.chat_message_separator.yesterday")} - #{I18n.t("js.chat.last_visit")}",
+      )
+    end
+  end
+end

plugins/chat/spec/system/uploads_spec.rb

@@ -36,12 +36,21 @@ describe "Uploading files in chat messages", type: :system, js: true do
 
     it "allows uploading multiple files" do
       chat.visit_channel(channel_1)
+
       file_path_1 = file_from_fixtures("logo.png", "images").path
-      file_path_2 = file_from_fixtures("logo.jpg", "images").path
-      attach_file([file_path_1, file_path_2]) do
+      attach_file([file_path_1]) do
         channel.open_action_menu
         channel.click_action_button("chat-upload-btn")
+        find(".chat-composer-input").click
       end
+
+      file_path_2 = file_from_fixtures("logo.jpg", "images").path
+      attach_file([file_path_2]) do
+        channel.open_action_menu
+        channel.click_action_button("chat-upload-btn")
+        find(".chat-composer-input").click
+      end
+
       expect(page).to have_css(".chat-composer-upload .preview .preview-img", count: 2)
       channel.send_message("upload testing")