FEATURE: Scroll to first message when clicking date in chat (#21926)

This PR adds a new parameter to fetch chat messages: `target_date`.

It can be used to fetch messages by a specific date string. Note that it does not need to be the `created_at` date of an existing message, it can be any date. Similar to `target_message_id`, it retrieves an array of past and future messages following the query limits.
This commit is contained in:
Jan Cernik 2023-06-20 10:58:38 -03:00 committed by GitHub
parent cee15d4177
commit e51bbfa4e8
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
16 changed files with 187 additions and 24 deletions

View File

@ -68,14 +68,14 @@ class Chat::Api::ChannelsController < Chat::ApiController
end end
def show def show
if params[:target_message_id].present? || params[:include_messages].present? || if should_build_channel_view?
params[:fetch_from_last_read].present?
with_service( with_service(
Chat::ChannelViewBuilder, Chat::ChannelViewBuilder,
**params.permit( **params.permit(
:channel_id, :channel_id,
:target_message_id, :target_message_id,
:thread_id, :thread_id,
:target_date,
:page_size, :page_size,
:direction, :direction,
:fetch_from_last_read, :fetch_from_last_read,
@ -83,6 +83,7 @@ class Chat::Api::ChannelsController < Chat::ApiController
:channel_id, :channel_id,
:target_message_id, :target_message_id,
:thread_id, :thread_id,
:target_date,
:page_size, :page_size,
:direction, :direction,
:fetch_from_last_read, :fetch_from_last_read,
@ -160,4 +161,9 @@ class Chat::Api::ChannelsController < Chat::ApiController
permitted_params += CATEGORY_CHANNEL_EDITABLE_PARAMS if channel.category_channel? permitted_params += CATEGORY_CHANNEL_EDITABLE_PARAMS if channel.category_channel?
params.require(:channel).permit(*permitted_params) params.require(:channel).permit(*permitted_params)
end end
def should_build_channel_view?
params[:target_message_id].present? || params[:target_date].present? ||
params[:include_messages].present? || params[:fetch_from_last_read].present?
end
end end

View File

@ -3,10 +3,10 @@
module Chat module Chat
# Queries messages for a specific channel. This can be used in two modes: # Queries messages for a specific channel. This can be used in two modes:
# #
# 1. Query messages around a target_message_id. This is used when a user # 1. Query messages around a target_message_id or target_date. This is used
# needs to jump to the middle of a messages stream or load around their # when a user needs to jump to the middle of a messages stream or load
# last read message ID. There is no pagination or direction here, just # around a target. There is no pagination or direction
# a limit on past and future messages. # here, just a limit on past and future messages.
# 2. Query messages with paginations and direction. This is used for normal # 2. Query messages with paginations and direction. This is used for normal
# scrolling of the messages stream of a channel. # scrolling of the messages stream of a channel.
# #
@ -28,6 +28,7 @@ module Chat
# @param thread_id [Integer] (optional) The thread ID to filter messages by. # @param thread_id [Integer] (optional) The thread ID to filter messages by.
# @param target_message_id [Integer] (optional) The message ID to query around. # @param target_message_id [Integer] (optional) The message ID to query around.
# It is assumed that the caller already checked if this exists. # It is assumed that the caller already checked if this exists.
# @param target_date [String] (optional) The date to query around.
# @param include_thread_messages [Boolean] (optional) Whether to include messages # @param include_thread_messages [Boolean] (optional) Whether to include messages
# that are linked to a thread. # that are linked to a thread.
# @param page_size [Integer] (optional) The number of messages to fetch when not # @param page_size [Integer] (optional) The number of messages to fetch when not
@ -42,7 +43,8 @@ module Chat
target_message_id: nil, target_message_id: nil,
include_thread_messages: false, include_thread_messages: false,
page_size: PAST_MESSAGE_LIMIT + FUTURE_MESSAGE_LIMIT, page_size: PAST_MESSAGE_LIMIT + FUTURE_MESSAGE_LIMIT,
direction: nil direction: nil,
target_date: nil
) )
messages = base_query(channel: channel) messages = base_query(channel: channel)
messages = messages.with_deleted if guardian.can_moderate_chat?(channel.chatable) messages = messages.with_deleted if guardian.can_moderate_chat?(channel.chatable)
@ -61,7 +63,11 @@ module Chat
if target_message_id.present? && direction.blank? if target_message_id.present? && direction.blank?
query_around_target(target_message_id, channel, messages) query_around_target(target_message_id, channel, messages)
else else
query_paginated_messages(direction, page_size, channel, messages, target_message_id) if target_date.present?
query_by_date(target_date, channel, messages)
else
query_paginated_messages(direction, page_size, channel, messages, target_message_id)
end
end end
end end
@ -151,5 +157,32 @@ module Chat
can_load_more_future: can_load_more_future, can_load_more_future: can_load_more_future,
} }
end end
def self.query_by_date(target_date, channel, messages)
past_messages =
messages
.where("created_at <= ?", target_date)
.order(created_at: :desc)
.limit(PAST_MESSAGE_LIMIT)
.to_a
future_messages =
messages
.where("created_at > ?", target_date)
.order(created_at: :asc)
.limit(FUTURE_MESSAGE_LIMIT)
.to_a
can_load_more_past = past_messages.size == PAST_MESSAGE_LIMIT
can_load_more_future = future_messages.size == FUTURE_MESSAGE_LIMIT
{
past_messages: past_messages,
future_messages: future_messages,
target_date: target_date,
can_load_more_past: can_load_more_past,
can_load_more_future: can_load_more_future,
}
end
end end
end end

View File

@ -51,6 +51,7 @@ module Chat
attribute :direction, :string # (optional) attribute :direction, :string # (optional)
attribute :page_size, :integer # (optional) attribute :page_size, :integer # (optional)
attribute :fetch_from_last_read, :boolean # (optional) attribute :fetch_from_last_read, :boolean # (optional)
attribute :target_date, :string # (optional)
validates :channel_id, presence: true validates :channel_id, presence: true
validates :direction, validates :direction,
@ -128,16 +129,17 @@ module Chat
include_thread_messages: include_thread_messages, include_thread_messages: include_thread_messages,
page_size: contract.page_size, page_size: contract.page_size,
direction: contract.direction, direction: contract.direction,
target_date: contract.target_date,
) )
context.can_load_more_past = messages_data[:can_load_more_past] context.can_load_more_past = messages_data[:can_load_more_past]
context.can_load_more_future = messages_data[:can_load_more_future] context.can_load_more_future = messages_data[:can_load_more_future]
if !messages_data[:target_message] if !messages_data[:target_message] && !messages_data[:target_date]
context.messages = messages_data[:messages] context.messages = messages_data[:messages]
else else
messages_data[:target_message] = ( messages_data[:target_message] = (
if !include_thread_messages && messages_data[:target_message].thread_reply? if !include_thread_messages && messages_data[:target_message]&.thread_reply?
[] []
else else
[messages_data[:target_message]] [messages_data[:target_message]]
@ -148,7 +150,7 @@ module Chat
messages_data[:past_messages].reverse, messages_data[:past_messages].reverse,
messages_data[:target_message], messages_data[:target_message],
messages_data[:future_messages], messages_data[:future_messages],
].reduce([], :concat) ].reduce([], :concat).compact
end end
end end

View File

@ -36,12 +36,12 @@
class="chat-messages-container" class="chat-messages-container"
{{chat/on-resize this.didResizePane (hash delay=100 immediate=true)}} {{chat/on-resize this.didResizePane (hash delay=100 immediate=true)}}
> >
{{#if this.loadedOnce}} {{#if this.loadedOnce}}
{{#each @channel.messages key="id" as |message|}} {{#each @channel.messages key="id" as |message|}}
<ChatMessage <ChatMessage
@message={{message}} @message={{message}}
@resendStagedMessage={{this.resendStagedMessage}} @resendStagedMessage={{this.resendStagedMessage}}
@fetchMessagesByDate={{this.fetchMessagesByDate}}
@context="channel" @context="channel"
/> />
{{/each}} {{/each}}

View File

@ -183,6 +183,8 @@ export default class ChatLivePane extends Component {
if (this.requestedTargetMessageId) { if (this.requestedTargetMessageId) {
findArgs.targetMessageId = this.requestedTargetMessageId; findArgs.targetMessageId = this.requestedTargetMessageId;
scrollToMessageId = this.requestedTargetMessageId; scrollToMessageId = this.requestedTargetMessageId;
} else if (this.requestedTargetDate) {
findArgs.targetDate = this.requestedTargetDate;
} else if (fetchingFromLastRead) { } else if (fetchingFromLastRead) {
findArgs.fetchFromLastRead = true; findArgs.fetchFromLastRead = true;
scrollToMessageId = scrollToMessageId =
@ -230,6 +232,15 @@ export default class ChatLivePane extends Component {
highlight: true, highlight: true,
}); });
return; return;
} else if (this.requestedTargetDate) {
const message = this.args.channel?.findFirstMessageOfDay(
this.requestedTargetDate
);
this.scrollToMessage(message.id, {
highlight: true,
});
return;
} }
if ( if (
@ -240,7 +251,6 @@ export default class ChatLivePane extends Component {
this.scrollToMessage(scrollToMessageId); this.scrollToMessage(scrollToMessageId);
return; return;
} }
this.scrollToBottom(); this.scrollToBottom();
}) })
.catch(this._handleErrors) .catch(this._handleErrors)
@ -251,6 +261,7 @@ export default class ChatLivePane extends Component {
this.loadedOnce = true; this.loadedOnce = true;
this.requestedTargetMessageId = null; this.requestedTargetMessageId = null;
this.requestedTargetDate = null;
this.loadingMorePast = false; this.loadingMorePast = false;
this.debounceFillPaneAttempt(); this.debounceFillPaneAttempt();
this.updateLastReadMessage(); this.updateLastReadMessage();
@ -364,6 +375,16 @@ export default class ChatLivePane extends Component {
} }
@bind @bind
fetchMessagesByDate(date) {
const message = this.args.channel?.findFirstMessageOfDay(date);
if (message.firstOfResults && this.args.channel?.canLoadMorePast) {
this.requestedTargetDate = date;
this.debounceFetchMessages();
} else {
this.highlightOrFetchMessage(message.id);
}
}
fillPaneAttempt() { fillPaneAttempt() {
if (this._selfDeleted) { if (this._selfDeleted) {
return; return;
@ -392,9 +413,7 @@ export default class ChatLivePane extends Component {
let foundFirstNew = false; let foundFirstNew = false;
result.chat_messages.forEach((messageData, index) => { result.chat_messages.forEach((messageData, index) => {
if (index === 0) { messageData.firstOfResults = index === 0;
messageData.firstOfResults = true;
}
if (this.currentUser.ignored_users) { if (this.currentUser.ignored_users) {
// If a message has been hidden it is because the current user is ignoring // If a message has been hidden it is because the current user is ignoring

View File

@ -1,16 +1,22 @@
{{#if @message.firstMessageOfTheDayAt}} {{#if @message.formattedFirstMessageDate}}
<div <div
class={{concat-class class={{concat-class
"chat-message-separator-date" "chat-message-separator-date"
(if @message.newest "with-last-visit") (if @message.newest "with-last-visit")
}} }}
role="button"
{{on
"click"
(fn @fetchMessagesByDate @message.firstMessageOfTheDayAt)
passive=true
}}
> >
<div <div
class="chat-message-separator__text-container" class="chat-message-separator__text-container"
{{chat/track-message-separator-date}} {{chat/track-message-separator-date}}
> >
<span class="chat-message-separator__text"> <span class="chat-message-separator__text">
<span>{{@message.firstMessageOfTheDayAt}}</span> {{@message.formattedFirstMessageDate}}
{{#if @message.newest}} {{#if @message.newest}}
<span class="chat-message-separator__last-visit"> <span class="chat-message-separator__last-visit">

View File

@ -1,4 +1,4 @@
{{#if (and @message.newest (not @message.firstMessageOfTheDayAt))}} {{#if (and @message.newest (not @message.formattedFirstMessageDate))}}
<div class="chat-message-separator-new"> <div class="chat-message-separator-new">
<div class="chat-message-separator__text-container"> <div class="chat-message-separator__text-container">
<span class="chat-message-separator__text"> <span class="chat-message-separator__text">

View File

@ -2,7 +2,10 @@
{{#if this.shouldRender}} {{#if this.shouldRender}}
{{#if (eq @context "channel")}} {{#if (eq @context "channel")}}
<ChatMessageSeparatorDate @message={{@message}} /> <ChatMessageSeparatorDate
@fetchMessagesByDate={{@fetchMessagesByDate}}
@message={{@message}}
/>
<ChatMessageSeparatorNew @message={{@message}} /> <ChatMessageSeparatorNew @message={{@message}} />
{{/if}} {{/if}}

View File

@ -35,6 +35,13 @@ export default class ChatMessagesManager {
); );
} }
findFirstMessageOfDay(messageDate) {
const targetDay = new Date(messageDate).toDateString();
return this.messages.find(
(message) => new Date(message.createdAt).toDateString() === targetDay
);
}
removeMessage(message) { removeMessage(message) {
return this.messages.removeObject(message); return this.messages.removeObject(message);
} }

View File

@ -157,6 +157,10 @@ export default class ChatChannel {
return this.messagesManager.findMessage(id); return this.messagesManager.findMessage(id);
} }
findFirstMessageOfDay(date) {
return this.messagesManager.findFirstMessageOfDay(date);
}
addMessages(messages) { addMessages(messages) {
this.messagesManager.addMessages(messages); this.messagesManager.addMessages(messages);
} }

View File

@ -194,7 +194,7 @@ export default class ChatMessage {
get firstMessageOfTheDayAt() { get firstMessageOfTheDayAt() {
if (!this.previousMessage) { if (!this.previousMessage) {
return this.#calendarDate(this.createdAt); return this.#startOfDay(this.createdAt);
} }
if ( if (
@ -203,7 +203,13 @@ export default class ChatMessage {
new Date(this.createdAt) new Date(this.createdAt)
) )
) { ) {
return this.#calendarDate(this.createdAt); return this.#startOfDay(this.createdAt);
}
}
get formattedFirstMessageDate() {
if (this.firstMessageOfTheDayAt) {
return this.#calendarDate(this.firstMessageOfTheDayAt);
} }
} }
@ -363,4 +369,8 @@ export default class ChatMessage {
a.getDate() === b.getDate() a.getDate() === b.getDate()
); );
} }
#startOfDay(date) {
return moment(date).startOf("day").format();
}
} }

View File

@ -46,6 +46,10 @@ export default class ChatApi extends Service {
if (data.threadId) { if (data.threadId) {
args.thread_id = data.threadId; args.thread_id = data.threadId;
} }
if (data.targetDate) {
args.target_date = data.targetDate;
}
} }
return this.#getRequest(`/channels/${channelId}`, args).then((result) => { return this.#getRequest(`/channels/${channelId}`, args).then((result) => {

View File

@ -69,6 +69,10 @@
border-radius: 4px; border-radius: 4px;
color: var(--primary-800); color: var(--primary-800);
background: var(--primary-50); background: var(--primary-50);
&:hover {
border: 1px solid var(--secondary-high);
}
} }
.chat-message-separator__last-visit { .chat-message-separator__last-visit {
@ -94,6 +98,30 @@
padding: 0.25rem 0.5rem; padding: 0.25rem 0.5rem;
box-sizing: border-box; box-sizing: border-box;
display: flex; display: flex;
cursor: pointer;
pointer-events: all;
.no-touch & {
&:hover {
border: 1px solid var(--secondary-high);
border-radius: 4px;
color: var(--primary-800);
background: var(--primary-50);
}
}
.touch & {
&:active {
border: 1px solid var(--secondary-high);
border-radius: 4px;
color: var(--primary-800);
background: var(--primary-50);
}
}
&:active {
transform: scale(0.98);
}
} }
& + .chat-message-separator__line-container { & + .chat-message-separator__line-container {

View File

@ -9,6 +9,7 @@ RSpec.describe Chat::MessagesQuery do
let(:page_size) { nil } let(:page_size) { nil }
let(:direction) { nil } let(:direction) { nil }
let(:target_message_id) { nil } let(:target_message_id) { nil }
let(:target_date) { nil }
let(:options) do let(:options) do
{ {
thread_id: thread_id, thread_id: thread_id,
@ -16,6 +17,7 @@ RSpec.describe Chat::MessagesQuery do
page_size: page_size, page_size: page_size,
direction: direction, direction: direction,
target_message_id: target_message_id, target_message_id: target_message_id,
target_date: target_date,
} }
end end
@ -118,6 +120,20 @@ RSpec.describe Chat::MessagesQuery do
end end
end end
context "when target_date provided" do
let(:target_date) { 1.day.ago }
it "queries messages in the channel and finds the past and future messages" do
expect(subject).to eq(
past_messages: [message_1],
future_messages: [message_2, message_3],
target_date: target_date,
can_load_more_past: false,
can_load_more_future: false,
)
end
end
context "when target_message_id not provided" do context "when target_message_id not provided" do
it "queries messages in the channel" do it "queries messages in the channel" do
expect(subject).to eq( expect(subject).to eq(

View File

@ -21,6 +21,7 @@ RSpec.describe Chat::ChannelViewBuilder do
let(:direction) { nil } let(:direction) { nil }
let(:thread_id) { nil } let(:thread_id) { nil }
let(:fetch_from_last_read) { nil } let(:fetch_from_last_read) { nil }
let(:target_date) { nil }
let(:params) do let(:params) do
{ {
guardian: guardian, guardian: guardian,
@ -30,6 +31,7 @@ RSpec.describe Chat::ChannelViewBuilder do
page_size: page_size, page_size: page_size,
direction: direction, direction: direction,
thread_id: thread_id, thread_id: thread_id,
target_date: target_date,
} }
end end
@ -54,6 +56,7 @@ RSpec.describe Chat::ChannelViewBuilder do
include_thread_messages: true, include_thread_messages: true,
page_size: page_size, page_size: page_size,
direction: direction, direction: direction,
target_date: target_date,
) )
.returns({ messages: [] }) .returns({ messages: [] })
subject subject
@ -337,5 +340,24 @@ RSpec.describe Chat::ChannelViewBuilder do
end end
end end
end end
context "when target_date provided" do
fab!(:past_message) do
msg = Fabricate(:chat_message, chat_channel: channel)
msg.update!(created_at: 3.days.ago)
msg
end
fab!(:future_message) do
msg = Fabricate(:chat_message, chat_channel: channel)
msg.update!(created_at: 1.days.ago)
msg
end
let(:target_date) { 2.days.ago }
it "includes past and future messages" do
expect(subject.view.chat_messages).to eq([past_message, future_message])
end
end
end end
end end

View File

@ -11,9 +11,12 @@ module(
test("first message of the day", async function (assert) { test("first message of the day", async function (assert) {
this.set("date", moment().format("LLL")); this.set("date", moment().format("LLL"));
this.set("message", { firstMessageOfTheDayAt: this.date }); this.set("message", { formattedFirstMessageDate: this.date });
this.set("fetchMessagesByDate", () => {});
await render(hbs`<ChatMessageSeparatorDate @message={{this.message}} />`); await render(
hbs`<ChatMessageSeparatorDate @message={{this.message}} @fetchMessagesByDate={{this.fetchMessagesByDate}} />`
);
assert.strictEqual( assert.strictEqual(
query(".chat-message-separator-date").innerText.trim(), query(".chat-message-separator-date").innerText.trim(),