compose-closed-ui: Refactor get_recipient_label.

Refactors get_recipient_label so that it's a bit clearer what the
recipient_information parameter is for and what we do when that
parameter is undefined.

In doing so, we no longer treat the constructed objects, that are
passed as the recipient_information parameter, and actual Message
objects, that we get from the current message list view, as the
same thing.

(cherry picked from commit 7d3b77e490)
This commit is contained in:
Lauryn Menard
2025-03-24 19:03:11 +01:00
committed by Tim Abbott
parent 9080684585
commit 44fdbe5f04
2 changed files with 62 additions and 31 deletions

View File

@@ -42,35 +42,11 @@ export type ReplyRecipientInformation = {
export function get_recipient_label(
recipient_information?: ReplyRecipientInformation,
): RecipientLabel | undefined {
// TODO: This code path is bit of a type-checking disaster; we mix
// actual message objects with fake objects containing just a
// couple fields, both those constructed here and potentially
// passed in.
if (recipient_information === undefined) {
// We check the current message list for information about the
// reply recipient for the closed compose box button label.
if (message_lists.current === undefined) {
return undefined;
}
if (message_lists.current.visibly_empty()) {
// For empty narrows where there's a clear reply target,
// i.e. stream+topic or a single direct message conversation,
// we label the button as replying to the thread.
const stream_id = narrow_state.stream_id(narrow_state.filter(), true);
const topic = narrow_state.topic();
if (stream_id !== undefined && topic !== undefined) {
return get_stream_recipient_label(stream_id, topic);
} else if (narrow_state.pm_ids_string()) {
const user_ids = people.user_ids_string_to_ids_array(narrow_state.pm_ids_string()!);
return {label_text: message_store.get_pm_full_names(user_ids)};
}
} else {
recipient_information = message_lists.current.selected_message();
}
}
if (recipient_information) {
if (recipient_information !== undefined) {
// If we're in either the Inbox or Recent Conversations view,
// we try to update the closed compose box button label with
// information about the reply target from the focused row in
// the view.
if (
recipient_information.stream_id !== undefined &&
recipient_information.topic !== undefined
@@ -79,10 +55,45 @@ export function get_recipient_label(
recipient_information.stream_id,
recipient_information.topic,
);
} else if (recipient_information.display_reply_to) {
}
if (recipient_information.display_reply_to !== undefined) {
return {label_text: recipient_information.display_reply_to};
}
}
// Otherwise, we check the current message list for information
// about the reply target for the closed compose box button label.
if (message_lists.current === undefined) {
return undefined;
}
if (message_lists.current.visibly_empty()) {
// For empty narrows where there's a clear reply target,
// i.e. channel and topic or a direct message conversation,
// we label the button as replying to the thread.
const stream_id = narrow_state.stream_id(narrow_state.filter(), true);
const topic = narrow_state.topic();
const user_ids_string = narrow_state.pm_ids_string();
if (stream_id !== undefined && topic !== undefined) {
return get_stream_recipient_label(stream_id, topic);
}
if (user_ids_string !== undefined) {
const user_ids = people.user_ids_string_to_ids_array(user_ids_string);
return {label_text: message_store.get_pm_full_names(user_ids)};
}
// Show the standard button text for empty narrows without
// a clear reply target, e.g., an empty search view.
return undefined;
}
const selected_message = message_lists.current.selected_message();
if (selected_message !== undefined) {
if (selected_message?.is_stream) {
return get_stream_recipient_label(selected_message.stream_id, selected_message.topic);
}
return {label_text: selected_message.display_reply_to};
}
// Fall through to show the standard button text.
return undefined;
}

View File

@@ -24,6 +24,9 @@ mock_esm("../src/message_list_view", {
mock_esm("../src/people.ts", {
maybe_get_user_by_id: noop,
});
mock_esm("../src/settings_data", {
user_has_permission_for_group_setting: () => true,
});
const stream_data = zrequire("stream_data");
// Code we're actually using/testing
@@ -31,7 +34,10 @@ const compose_closed_ui = zrequire("compose_closed_ui");
const {Filter} = zrequire("filter");
const {MessageList} = zrequire("message_list");
const {MessageListData} = zrequire("message_list_data");
const {set_realm} = zrequire("state_data");
const {set_current_user, set_realm} = zrequire("state_data");
const current_user = {};
set_current_user(current_user);
const REALM_EMPTY_TOPIC_DISPLAY_NAME = "general chat";
set_realm({realm_empty_topic_display_name: REALM_EMPTY_TOPIC_DISPLAY_NAME});
@@ -74,34 +80,48 @@ run_test("reply_label", () => {
[
{
id: 0,
is_stream: true,
is_private: false,
stream_id: stream_one.stream_id,
topic: "first_topic",
},
{
id: 1,
is_stream: true,
is_private: false,
stream_id: stream_one.stream_id,
topic: "second_topic",
},
{
id: 2,
is_stream: true,
is_private: false,
stream_id: stream_two.stream_id,
topic: "third_topic",
},
{
id: 3,
is_stream: true,
is_private: false,
stream_id: stream_two.stream_id,
topic: "second_topic",
},
{
id: 4,
is_stream: false,
is_private: true,
display_reply_to: "some user",
},
{
id: 5,
is_stream: false,
is_private: true,
display_reply_to: "some user, other user",
},
{
id: 6,
is_stream: true,
is_private: false,
stream_id: stream_two.stream_id,
topic: "",
},