UX: opens thread on channel with unread threads (#23361)

When visiting a channel which has unread threads, we will now open the threads list panel.

Note that:

mobile
linking to message
linking to a thread

Won't open the threads list.
This commit is contained in:
Joffrey JAFFEUX 2023-10-11 12:19:30 +02:00 committed by GitHub
parent f25388428e
commit b77b0ee1c8
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
11 changed files with 230 additions and 146 deletions

View File

@ -19,7 +19,7 @@
{{#if this.chatStateManager.isDrawerExpanded}}
<div class="chat-drawer-content" {{did-insert this.fetchChannel}}>
{{#if this.chat.activeChannel}}
<Chat::ThreadList
<ChatThreadList
@channel={{this.chat.activeChannel}}
@includeHeader={{false}}
/>

View File

@ -1,8 +1,15 @@
import Component from "@glimmer/component";
import { cached } from "@glimmer/tracking";
import { action } from "@ember/object";
import { inject as service } from "@ember/service";
import { modifier } from "ember-modifier";
import ConditionalLoadingSpinner from "discourse/components/conditional-loading-spinner";
import isElementInViewport from "discourse/lib/is-element-in-viewport";
import { bind } from "discourse-common/utils/decorators";
import I18n from "I18n";
import eq from "truth-helpers/helpers/eq";
import ChatThreadListHeader from "discourse/plugins/chat/discourse/components/chat/thread-list/header";
import ChatThreadListItem from "discourse/plugins/chat/discourse/components/chat/thread-list/item";
import ChatTrackMessage from "discourse/plugins/chat/discourse/modifiers/chat/track-message";
export default class ChatThreadList extends Component {
@service chat;
@ -10,6 +17,58 @@ export default class ChatThreadList extends Component {
@service messageBus;
@service chatTrackingStateManager;
noThreadsLabel = I18n.t("chat.threads.none");
subscribe = modifier((element, [channel]) => {
this.messageBus.subscribe(
`/chat/${channel.id}`,
this.onMessageBus,
channel.channelMessageBusLastId
);
return () => {
// TODO (joffrey) In drawer we won't have channel anymore at this point
if (!channel) {
return;
}
this.messageBus.unsubscribe(`/chat/${channel.id}`, this.onMessageBus);
};
});
fill = modifier((element) => {
this.resizeObserver = new ResizeObserver(() => {
if (isElementInViewport(element)) {
this.loadThreads();
}
});
this.resizeObserver.observe(element);
return () => {
this.resizeObserver.disconnect();
};
});
loadMore = modifier((element) => {
this.intersectionObserver = new IntersectionObserver(this.loadThreads);
this.intersectionObserver.observe(element);
return () => {
this.intersectionObserver.disconnect();
};
});
@cached
get threadsCollection() {
return this.chatApi.threads(this.args.channel.id, this.handleLoadedThreads);
}
@bind
loadThreads() {
this.threadsCollection.load({ limit: 10 });
}
get threadsManager() {
return this.args.channel.threadsManager;
}
@ -65,22 +124,6 @@ export default class ChatThreadList extends Component {
return !!this.args.channel;
}
@action
loadThreads() {
return this.threadsCollection.load({ limit: 10 });
}
@action
subscribe() {
this.#unsubscribe();
this.messageBus.subscribe(
`/chat/${this.args.channel.id}`,
this.onMessageBus,
this.args.channel.messageBusLastId
);
}
@bind
onMessageBus(busData) {
switch (busData.type) {
@ -119,11 +162,6 @@ export default class ChatThreadList extends Component {
restoredOriginalMessageThread.originalMessage.deletedAt = null;
}
@cached
get threadsCollection() {
return this.chatApi.threads(this.args.channel.id, this.handleLoadedThreads);
}
@bind
handleLoadedThreads(result) {
return result.threads.map((thread) => {
@ -140,20 +178,40 @@ export default class ChatThreadList extends Component {
});
}
@action
teardown() {
this.#unsubscribe();
}
<template>
{{! template-lint-disable modifier-name-case }}
{{#if this.shouldRender}}
<div class="chat-thread-list" {{this.subscribe @channel}}>
{{#if @includeHeader}}
<ChatThreadListHeader @channel={{@channel}} />
{{/if}}
#unsubscribe() {
// TODO (joffrey) In drawer we won't have channel anymore at this point
if (!this.args.channel) {
return;
}
<div class="chat-thread-list__items" {{this.fill}}>
{{#each this.sortedThreads key="id" as |thread|}}
<ChatThreadListItem
@thread={{thread}}
{{(if
(eq thread this.lastThread)
(modifier ChatTrackMessage this.load)
)}}
/>
{{else}}
{{#if this.threadsCollection.fetchedOnce}}
<div class="chat-thread-list__no-threads">
{{this.noThreadsLabel}}
</div>
{{/if}}
{{/each}}
this.messageBus.unsubscribe(
`/chat/${this.args.channel.id}`,
this.onMessageBus
);
}
<ConditionalLoadingSpinner
@condition={{this.threadsCollection.loading}}
/>
<div {{this.loadMore}}>
<br />
</div>
</div>
</div>
{{/if}}
</template>
}

View File

@ -1,35 +0,0 @@
{{#if this.shouldRender}}
<div
class="chat-thread-list"
{{did-insert this.subscribe}}
{{did-insert this.loadThreads}}
{{did-update this.loadThreads @channel}}
{{did-update this.subscribe @channel}}
{{will-destroy this.teardown}}
>
{{#if @includeHeader}}
<Chat::ThreadList::Header @channel={{@channel}} />
{{/if}}
<div class="chat-thread-list__items">
{{#each this.sortedThreads as |thread|}}
<Chat::ThreadList::Item
@thread={{thread}}
{{(if
(eq thread this.lastThread)
(modifier "chat/track-message" this.loadThreads)
)}}
/>
{{else}}
{{#if this.threadsCollection.fetchedOnce}}
<div class="chat-thread-list__no-threads">
{{i18n "chat.threads.none"}}
</div>
{{/if}}
{{/each}}
<ConditionalLoadingSpinner
@condition={{this.threadsCollection.loading}}
/>
</div>
</div>
{{/if}}

View File

@ -1,5 +1,21 @@
import { inject as service } from "@ember/service";
import DiscourseRoute from "discourse/routes/discourse";
import withChatChannel from "./chat-channel-decorator";
@withChatChannel
export default class ChatChannelRoute extends DiscourseRoute {}
export default class ChatChannelRoute extends DiscourseRoute {
@service site;
redirect(model) {
if (this.site.mobileView) {
return;
}
const messageId = this.paramsFor("chat.channel.near-message").messageId;
const threadId = this.paramsFor("chat.channel.thread").threadId;
if (!messageId && !threadId && model.threadsManager.unreadThreadCount > 0) {
this.transitionTo("chat.channel.threads", ...model.routeModels);
}
}
}

View File

@ -1 +1 @@
<Chat::ThreadList @channel={{this.model}} @includeHeader={{true}} />
<ChatThreadList @channel={{this.model}} @includeHeader={{true}} />

View File

@ -5,9 +5,10 @@ RSpec.describe "Chat channel", type: :system do
fab!(:channel_1) { Fabricate(:chat_channel) }
fab!(:message_1) { Fabricate(:chat_message, chat_channel: channel_1) }
let(:chat) { PageObjects::Pages::Chat.new }
let(:chat_page) { PageObjects::Pages::Chat.new }
let(:channel_page) { PageObjects::Pages::ChatChannel.new }
let(:sidebar_page) { PageObjects::Pages::Sidebar.new }
let(:side_panel_page) { PageObjects::Pages::ChatSidePanel.new }
before do
chat_system_bootstrap
@ -15,11 +16,53 @@ RSpec.describe "Chat channel", type: :system do
sign_in(current_user)
end
context "when has unread threads" do
fab!(:thread_1) { Fabricate(:chat_thread, channel: channel_1) }
before do
channel_1.update!(threading_enabled: true)
thread_1.add(current_user)
Fabricate(:chat_message, thread: thread_1, use_service: true)
end
context "when visiting channel" do
it "opens thread panel" do
chat_page.visit_channel(channel_1)
expect(side_panel_page).to have_open_thread_list
end
end
context "when visiting channel on mobile", mobile: true do
it "doesnt open thread panel" do
chat_page.visit_channel(channel_1)
expect(side_panel_page).to have_no_open_thread_list
end
end
context "when visiting thread" do
it "doesnt open thread panel" do
chat_page.visit_thread(thread_1)
expect(side_panel_page).to have_no_open_thread_list
end
end
context "when opening channel message" do
it "doesnt open thread panel" do
chat_page.visit_channel(channel_1, message_id: message_1.id)
expect(side_panel_page).to have_no_open_thread_list
end
end
end
context "when first batch of messages doesnt fill page" do
before { 30.times { Fabricate(:chat_message, user: current_user, chat_channel: channel_1) } }
it "autofills for more messages" do
chat.prefers_full_page
chat_page.prefers_full_page
visit("/")
# cheap trick to ensure the messages don't fill the initial page
page.execute_script(
@ -37,7 +80,7 @@ RSpec.describe "Chat channel", type: :system do
it "loads most recent messages" do
unloaded_message = Fabricate(:chat_message, chat_channel: channel_1)
chat.visit_channel(channel_1, message_id: message_1.id)
chat_page.visit_channel(channel_1, message_id: message_1.id)
expect(channel_page.messages).to have_no_message(id: unloaded_message.id)
@ -51,12 +94,12 @@ RSpec.describe "Chat channel", type: :system do
it "syncs the messages" do
using_session(:tab_1) do
sign_in(current_user)
chat.visit_channel(channel_1)
chat_page.visit_channel(channel_1)
end
using_session(:tab_2) do
sign_in(current_user)
chat.visit_channel(channel_1)
chat_page.visit_channel(channel_1)
end
using_session(:tab_1) do |session|
@ -72,7 +115,7 @@ RSpec.describe "Chat channel", type: :system do
end
it "allows to edit this message once persisted" do
chat.visit_channel(channel_1)
chat_page.visit_channel(channel_1)
channel_page.send_message("aaaaaa")
expect(channel_page.messages).to have_message(persisted: true, text: "aaaaaa")
@ -107,7 +150,7 @@ RSpec.describe "Chat channel", type: :system do
it "jumps to the bottom of the channel" do
channel_1.membership_for(current_user).update!(last_read_message: message_1)
messages = 50.times.map { Fabricate(:chat_message, chat_channel: channel_1) }
chat.visit_channel(channel_1)
chat_page.visit_channel(channel_1)
expect(page).to have_css("[data-id='#{messages.first.id}']")
expect(page).to have_no_css("[data-id='#{messages.last.id}']")
@ -145,7 +188,7 @@ RSpec.describe "Chat channel", type: :system do
before { channel_1.add(other_user) }
it "highlights the mentions" do
chat.visit_channel(channel_1)
chat_page.visit_channel(channel_1)
expect(page).to have_selector(".mention.highlighted.valid-mention", text: "@here")
expect(page).to have_selector(".mention.highlighted.valid-mention", text: "@all")
@ -164,7 +207,7 @@ RSpec.describe "Chat channel", type: :system do
Fabricate(:chat_mention, user: current_user, chat_message: message)
Fabricate(:chat_mention, user: other_user, chat_message: message)
chat.visit_channel(channel_1)
chat_page.visit_channel(channel_1)
expect(page).to have_selector(
".mention .user-status-message img[alt='#{current_user.user_status.emoji}']",
@ -184,7 +227,7 @@ RSpec.describe "Chat channel", type: :system do
end
it "doesnt show the reply-to line" do
chat.visit_channel(channel_1)
chat_page.visit_channel(channel_1)
expect(page).to have_no_selector(".chat-reply__excerpt")
end
@ -200,7 +243,7 @@ RSpec.describe "Chat channel", type: :system do
end
it "shows the reply-to line" do
chat.visit_channel(channel_1)
chat_page.visit_channel(channel_1)
expect(page).to have_selector(".chat-reply__excerpt")
end
@ -224,7 +267,7 @@ RSpec.describe "Chat channel", type: :system do
end
it "renders text in the reply-to" do
chat.visit_channel(channel_1)
chat_page.visit_channel(channel_1)
expect(find(".chat-reply .chat-reply__excerpt")["innerHTML"].strip).to eq(
"&lt;mark&gt;not marked&lt;/mark&gt;",
@ -234,7 +277,7 @@ RSpec.describe "Chat channel", type: :system do
it "renders safe HTML like mentions (which are just links) in the reply-to" do
message_2.update!(message: "@#{other_user.username} <mark>not marked</mark>")
message_2.rebake!
chat.visit_channel(channel_1)
chat_page.visit_channel(channel_1)
expect(find(".chat-reply .chat-reply__excerpt")["innerHTML"].strip).to eq(
"@#{other_user.username} &lt;mark&gt;not marked&lt;/mark&gt;",
@ -246,7 +289,7 @@ RSpec.describe "Chat channel", type: :system do
before { Fabricate(:chat_message, chat_channel: channel_1, created_at: 2.days.ago) }
it "shows a date separator" do
chat.visit_channel(channel_1)
chat_page.visit_channel(channel_1)
expect(page).to have_selector(".chat-message-separator__text", text: "Today")
end
@ -262,7 +305,7 @@ RSpec.describe "Chat channel", type: :system do
MESSAGE
it "adds the correct lang" do
chat.visit_channel(channel_1)
chat_page.visit_channel(channel_1)
expect(page).to have_selector("code.lang-ruby")
end
@ -272,7 +315,7 @@ RSpec.describe "Chat channel", type: :system do
before { 50.times { Fabricate(:chat_message, chat_channel: channel_1) } }
it "resets the active message" do
chat.visit_channel(channel_1)
chat_page.visit_channel(channel_1)
last_message = find(".chat-message-container:last-child")
last_message.hover
@ -290,7 +333,7 @@ RSpec.describe "Chat channel", type: :system do
context "when opening message secondary options" do
it "doesnt hide dropdown on mouseleave" do
chat.visit_channel(channel_1)
chat_page.visit_channel(channel_1)
last_message = find(".chat-message-container:last-child")
last_message.hover

View File

@ -13,10 +13,7 @@ RSpec.describe "Message notifications - mobile", type: :system, mobile: true do
end
def create_message(text: "this is fine", channel:, creator: Fabricate(:user))
sign_in(creator)
chat_page.visit_channel(channel)
chat_channel_page.send_message(text)
expect(chat_channel_page.messages).to have_message(text: text)
Fabricate(:chat_message_with_service, chat_channel: channel, message: text, user: creator)
end
context "as a user" do

View File

@ -50,8 +50,7 @@ module PageObjects
def visit_channel(channel, message_id: nil)
visit(channel.url + (message_id ? "/#{message_id}" : ""))
has_no_css?(".chat-channel--not-loaded-once")
has_no_css?(".chat-skeleton")
has_finished_loading?
end
def visit_thread(thread)
@ -59,6 +58,11 @@ module PageObjects
has_css?(".chat-thread:not(.loading)[data-id=\"#{thread.id}\"]")
end
def visit_threads_list(channel)
visit(channel.url + "/t")
has_finished_loading?
end
def visit_channel_settings(channel)
visit(channel.url + "/info/settings")
end
@ -78,6 +82,11 @@ module PageObjects
PageObjects::Pages::ChatBrowse.new.has_finished_loading?
end
def has_finished_loading?
has_no_css?(".chat-channel--not-loaded-once")
has_no_css?(".chat-skeleton")
end
def minimize_full_page
find(".open-drawer-btn").click
end

View File

@ -27,6 +27,10 @@ module PageObjects
def has_open_thread_list?
has_css?(".chat-side-panel .chat-thread-list")
end
def has_no_open_thread_list?
has_no_css?(".chat-side-panel .chat-thread-list")
end
end
end
end

View File

@ -39,17 +39,9 @@ describe "Thread list in side panel | full page", type: :system do
it "does not show new threads in the channel in the thread list if the user is not tracking them" do
chat_page.visit_channel(channel)
using_session(:other_user) do |session|
sign_in(other_user)
chat_page.visit_channel(channel)
channel_page.reply_to(thread_om)
thread_page.send_message("hey everyone!")
expect(channel_page).to have_thread_indicator(thread_om)
session.quit
end
Fabricate(:chat_message_with_service, chat_channel: channel, in_reply_to: thread_om)
channel_page.open_thread_list
expect(page).to have_content(I18n.t("js.chat.threads.none"))
end
@ -57,7 +49,7 @@ describe "Thread list in side panel | full page", type: :system do
it "does not double up the staged thread and the actual thread in the list" do
chat_page.visit_channel(channel)
channel_page.reply_to(thread_om)
thread_page.send_message("hey everyone!")
thread_page.send_message
expect(channel_page).to have_thread_indicator(thread_om)
thread_page.close
channel_page.open_thread_list
@ -84,16 +76,15 @@ describe "Thread list in side panel | full page", type: :system do
end
it "shows the OM excerpt for threads without a title" do
chat_page.visit_channel(channel)
channel_page.open_thread_list
chat_page.visit_threads_list(channel)
expect(page).to have_content(thread_1.original_message.excerpt)
end
it "shows the thread title with emoji" do
thread_1.update!(title: "What is for dinner? :hamburger:")
chat_page.visit_channel(channel)
channel_page.open_thread_list
chat_page.visit_threads_list(channel)
expect(thread_list_page.item_by_id(thread_1.id)).to have_content("What is for dinner?")
expect(thread_list_page.item_by_id(thread_1.id)).to have_css("img.emoji[alt='hamburger']")
end
@ -101,16 +92,16 @@ describe "Thread list in side panel | full page", type: :system do
it "shows an excerpt of the original message of the thread" do
thread_1.original_message.update!(message: "This is a long message for the excerpt")
thread_1.original_message.rebake!
chat_page.visit_channel(channel)
channel_page.open_thread_list
chat_page.visit_threads_list(channel)
expect(thread_list_page.item_by_id(thread_1.id)).to have_content(
"This is a long message for the excerpt",
)
end
it "doesnt show the thread original message user avatar" do
chat_page.visit_channel(channel)
channel_page.open_thread_list
chat_page.visit_threads_list(channel)
expect(thread_list_page.item_by_id(thread_1.id)).to have_no_css(
thread_list_page.avatar_selector(thread_1.original_message.user),
)
@ -119,16 +110,15 @@ describe "Thread list in side panel | full page", type: :system do
it "shows the last reply date of the thread" do
freeze_time
last_reply = Fabricate(:chat_message, thread: thread_1, use_service: true)
chat_page.visit_channel(channel)
channel_page.open_thread_list
chat_page.visit_threads_list(channel)
expect(thread_list_page.item_by_id(thread_1.id)).to have_css(
thread_list_page.last_reply_datetime_selector(last_reply),
)
end
it "shows participants" do
chat_page.visit_channel(channel)
channel_page.open_thread_list
chat_page.visit_threads_list(channel)
expect(thread_list_page.item_by_id(thread_1.id)).to have_css(
".avatar[title='#{current_user.username}']",
@ -139,8 +129,8 @@ describe "Thread list in side panel | full page", type: :system do
end
it "opens a thread" do
chat_page.visit_channel(channel)
channel_page.open_thread_list
chat_page.visit_threads_list(channel)
thread_list_page.item_by_id(thread_1.id).click
expect(side_panel).to have_open_thread(thread_1)
end
@ -160,17 +150,15 @@ describe "Thread list in side panel | full page", type: :system do
end
it "hides the thread in the list when another user deletes the original message" do
chat_page.visit_channel(channel)
channel_page.open_thread_list
chat_page.visit_threads_list(channel)
expect(thread_list_page).to have_thread(thread_1)
using_session(:tab_2) do |session|
sign_in(other_user)
chat_page.visit_thread(thread_1)
expect(side_panel_page).to have_open_thread(thread_1)
thread_page.messages.delete(thread_1.original_message)
session.quit
end
Chat::TrashMessage.call(
message_id: thread_1.original_message.id,
channel_id: thread_1.channel.id,
guardian: other_user.guardian,
)
expect(thread_list_page).to have_no_thread(thread_1)
end
@ -181,8 +169,8 @@ describe "Thread list in side panel | full page", type: :system do
current_user.update!(admin: true)
thread_1.original_message.trash!
chat_page.visit_channel(channel)
channel_page.open_thread_list
chat_page.visit_threads_list(channel)
expect(thread_list_page).to have_no_thread(thread_1)
using_session(:tab_2) do |session|
@ -203,39 +191,35 @@ describe "Thread list in side panel | full page", type: :system do
describe "updating the title of the thread" do
let(:new_title) { "wow new title" }
def open_thread_list
chat_page.visit_channel(channel)
channel_page.open_thread_list
expect(side_panel).to have_open_thread_list
end
it "allows updating when user is admin" do
current_user.update!(admin: true)
open_thread_list
chat_page.visit_threads_list(channel)
thread_list_page.item_by_id(thread_1.id).click
thread_page.header.open_settings
find(".chat-modal-thread-settings__title-input").fill_in(with: new_title)
find(".modal-footer .btn-primary").click
expect(thread_page.header).to have_title_content(new_title)
end
it "allows updating when user is same as the chat original message user" do
thread_1.update!(original_message_user: current_user)
thread_1.original_message.update!(user: current_user)
open_thread_list
chat_page.visit_threads_list(channel)
thread_list_page.item_by_id(thread_1.id).click
thread_page.header.open_settings
find(".chat-modal-thread-settings__title-input").fill_in(with: new_title)
find(".modal-footer .btn-primary").click
expect(thread_page.header).to have_title_content(new_title)
end
it "does not allow updating if user is neither admin nor original message user" do
thread_1.update!(original_message_user: other_user)
thread_1.original_message.update!(user: other_user)
open_thread_list
chat_page.visit_threads_list(channel)
thread_list_page.item_by_id(thread_1.id).click
expect(thread_page.header).to have_no_settings_button
end
end

View File

@ -26,6 +26,8 @@ describe "Thread tracking state | full page", type: :system do
it "shows the count of threads with unread messages on the thread list button" do
chat_page.visit_channel(channel)
thread_page.close
expect(channel_page).to have_unread_thread_indicator(count: 1)
end
@ -37,16 +39,18 @@ describe "Thread tracking state | full page", type: :system do
it "shows an indicator on the unread thread in the list" do
chat_page.visit_channel(channel)
channel_page.open_thread_list
expect(thread_list_page).to have_unread_item(thread.id, count: 1)
end
it "marks the thread as read and removes both indicators when the user opens it" do
chat_page.visit_channel(channel)
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_previous_route
expect(thread_list_page).to have_no_unread_item(thread.id)
end
@ -80,9 +84,11 @@ describe "Thread tracking state | full page", type: :system do
Fabricate(:chat_message, thread: new_thread, use_service: true)
chat_page.visit_thread(new_thread)
thread_page.notification_level = :tracking
expect(thread_page).to have_notification_level("tracking")
chat_page.visit_channel(channel)
channel_page.open_thread_list
expect(thread_list_page).to have_thread(new_thread)
end
@ -107,6 +113,8 @@ describe "Thread tracking state | full page", type: :system do
it "clears the sidebar unread indicator for the channel when opening it but keeps the thread list unread indicator" do
chat_page.visit_channel(channel)
thread_page.close
expect(sidebar_page).to have_no_unread_channel(channel)
expect(channel_page).to have_unread_thread_indicator(count: 1)
end