Mirror
/
zulip
mirror of https://github.com/zulip/zulip.git
1
0
Fork 0
Code Issues Projects Releases Wiki Activity

left-sidebar: Use same logic as of Topics view to render PMs. 

Change the logic for rendering PM threads in PM section to
be in the same as that of topics view --
In default view, only recent 5 PM threads would be shown
and append the active conversation as the 6th one at last
if not present in those 5.

In PM section with unreads, a maximum of 8 conversations
would be shown and rest of them would be hidden behind
the 'more conversations' li-item, clicking on which takes
to the zoomedIn view of PM section where all the present
PM threads would be visible.

Co-authored-by: Tim Abbott <tabbott@zulip.com>
This commit is contained in:
jai2201 2022-03-27 19:51:49 +05:30 committed by Tim Abbott
parent 3388eaf644
commit c6f2b9c636
2 changed files with 332 additions and 5 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

260
frontend_tests/node_tests/pm_list_data.js
View File

@ -34,6 +34,46 @@ const me = {
user_id: 103,
full_name: "Me Myself",
};
const zoe = {
email: "zoe@zulip.com",
user_id: 104,
full_name: "Zoe",
};
const cardelio = {
email: "cardelio@zulip.com",
user_id: 105,
full_name: "Cardelio",
};
const shiv = {
email: "shiv@zulip.com",
user_id: 106,
full_name: "Shiv",
};
const desdemona = {
email: "desdemona@zulip.com",
user_id: 107,
full_name: "Desdemona",
};
const lago = {
email: "lago@zulip.com",
user_id: 108,
full_name: "Lago",
};
const aaron = {
email: "aaron@zulip.com",
user_id: 109,
full_name: "Aaron",
};
const jai = {
email: "jai@zulip.com",
user_id: 110,
full_name: "Jai",
};
const shivam = {
email: "shivam@zulip.com",
user_id: 111,
full_name: "Shivam",
};
const bot_test = {
email: "outgoingwebhook@zulip.com",
user_id: 314,
@ -44,6 +84,14 @@ const bot_test = {
people.add_active_user(alice);
people.add_active_user(bob);
people.add_active_user(me);
people.add_active_user(zoe);
people.add_active_user(cardelio);
people.add_active_user(shiv);
people.add_active_user(desdemona);
people.add_active_user(lago);
people.add_active_user(aaron);
people.add_active_user(jai);
people.add_active_user(shivam);
people.add_active_user(bot_test);
people.initialize_current_user(me.user_id);
@ -55,6 +103,10 @@ function test(label, f) {
});
}
function get_list_info(zoomed) {
return pm_list_data.get_list_info(zoomed);
}
test("get_conversations", ({override}) => {
pm_conversations.recent.insert([alice.user_id, bob.user_id], 1);
pm_conversations.recent.insert([me.user_id], 2);
@ -179,3 +231,211 @@ test("is_all_privates", () => {
narrow_state.set_current_filter(private_filter());
assert.equal(pm_list_data.is_all_privates(), true);
});
test("get_list_info", ({override}) => {
let list_info;
assert.equal(narrow_state.filter(), undefined);
// Initialize an empty list to start.
const empty_list_info = get_list_info();
assert.deepEqual(empty_list_info, {
conversations_to_be_shown: [],
more_conversations_unread_count: 0,
});
// TODO: We should just initialize a Filter object with `new
// Filter` rather than creating a mock.
function set_filter_result(emails) {
const active_filter = {
operands: (operand) => {
assert.equal(operand, "pm-with");
return emails;
},
};
narrow_state.set_current_filter(active_filter);
}
set_filter_result([]);
assert.equal(pm_list_data.get_active_user_ids_string(), undefined);
// Mock to arrange that each user has exactly 1 unread.
const num_unread_for_person = 1;
override(unread, "num_unread_for_person", () => num_unread_for_person);
// Initially, we append 2 conversations and check for the
// `conversations_to_be_shown` returned in list_info.
pm_conversations.recent.insert([alice.user_id, bob.user_id], 1);
pm_conversations.recent.insert([me.user_id], 2);
list_info = get_list_info(false);
const expected_list_info = [
{
is_active: false,
is_group: false,
is_zero: false,
recipients: "Me Myself",
status_emoji_info: {
emoji_code: 20,
},
unread: 1,
url: "#narrow/pm-with/103-me",
user_circle_class: "user_circle_empty",
user_ids_string: "103",
},
{
recipients: "Alice, Bob",
user_ids_string: "101,102",
unread: 1,
is_active: false,
url: "#narrow/pm-with/101,102-group",
user_circle_class: undefined,
status_emoji_info: undefined,
is_group: true,
is_zero: false,
},
];
assert.deepEqual(list_info, {
conversations_to_be_shown: expected_list_info,
more_conversations_unread_count: 0,
});
// Now, add additional converstaions until we exceed
// `max_conversations_to_show_with_unreads`.
pm_conversations.recent.insert([zoe.user_id], 3);
pm_conversations.recent.insert([cardelio.user_id], 4);
pm_conversations.recent.insert([zoe.user_id, cardelio.user_id], 5);
pm_conversations.recent.insert([shiv.user_id], 6);
pm_conversations.recent.insert([cardelio.user_id, shiv.user_id], 7);
pm_conversations.recent.insert([desdemona.user_id], 8);
// We've now added a total of 8 conversations, which is the value
// of `max_conversations_to_show_with_unreads` so even now, the
// number of unreads in `more conversations` li-item should be 0.
list_info = get_list_info(false);
assert.deepEqual(list_info.conversations_to_be_shown.length, 8);
assert.deepEqual(list_info.more_conversations_unread_count, 0);
// Verify just the ordering of the conversations with unreads.
assert.deepEqual(
list_info.conversations_to_be_shown.map((conversation) => conversation.recipients),
[
"Desdemona",
"Cardelio, Shiv",
"Shiv",
"Cardelio, Zoe",
"Cardelio",
"Zoe",
"Me Myself",
"Alice, Bob",
],
);
// After adding one more conversation, there will be 10
// conversations, which exceeds
// `max_conversations_to_show_with_unreads`. Verify that the
// oldest conversations are not shown and their unreads counted in
// more_conversations_unread_count.
pm_conversations.recent.insert([lago.user_id], 9);
pm_conversations.recent.insert([zoe.user_id, lago.user_id], 10);
list_info = get_list_info(false);
assert.deepEqual(list_info.conversations_to_be_shown.length, 8);
assert.deepEqual(list_info.more_conversations_unread_count, 2);
assert.deepEqual(
list_info.conversations_to_be_shown.map((conversation) => conversation.recipients),
[
"Lago, Zoe",
"Lago",
"Desdemona",
"Cardelio, Shiv",
"Shiv",
"Cardelio, Zoe",
"Cardelio",
"Zoe",
],
);
// If we are narrowed to an older topic, then that one gets
// included despite not being among the 8 most recent.
set_filter_result(["alice@zulip.com,bob@zulip.com"]);
list_info = get_list_info(false);
assert.deepEqual(list_info.conversations_to_be_shown.length, 9);
assert.deepEqual(list_info.more_conversations_unread_count, 1);
assert.deepEqual(
list_info.conversations_to_be_shown.map((conversation) => conversation.recipients),
[
"Lago, Zoe",
"Lago",
"Desdemona",
"Cardelio, Shiv",
"Shiv",
"Cardelio, Zoe",
"Cardelio",
"Zoe",
"Alice, Bob",
],
);
// Verify that if the list is zoomed, we'll include all 9
// conversations in the correct order.
list_info = get_list_info(true);
assert.deepEqual(list_info.conversations_to_be_shown.length, 10);
assert.deepEqual(
list_info.conversations_to_be_shown.map((conversation) => conversation.recipients),
[
"Lago, Zoe",
"Lago",
"Desdemona",
"Cardelio, Shiv",
"Shiv",
"Cardelio, Zoe",
"Cardelio",
"Zoe",
"Me Myself",
"Alice, Bob",
],
);
// We now test some no unreads cases.
override(unread, "num_unread_for_person", () => 0);
pm_conversations.clear_for_testing();
pm_conversations.recent.insert([alice.user_id], 1);
pm_conversations.recent.insert([bob.user_id], 2);
pm_conversations.recent.insert([me.user_id], 3);
pm_conversations.recent.insert([zoe.user_id], 4);
pm_conversations.recent.insert([cardelio.user_id], 5);
pm_conversations.recent.insert([shiv.user_id], 6);
pm_conversations.recent.insert([alice.user_id, bob.user_id], 7);
// We have 7 conversations in total, but only most recent 5 are visible.
list_info = get_list_info(false);
assert.deepEqual(list_info.conversations_to_be_shown.length, 5);
// If we set the oldest conversation as active, it will be
// included as well as the 5 most recent.
set_filter_result(["alice@zulip.com"]);
assert.equal(pm_list_data.get_active_user_ids_string(), "101");
list_info = get_list_info(false);
assert.deepEqual(list_info.conversations_to_be_shown.length, 6);
assert.deepEqual(list_info.conversations_to_be_shown[5], {
recipients: "Alice",
user_ids_string: "101",
unread: 0,
is_zero: true,
is_active: true,
url: "#narrow/pm-with/101-alice",
status_emoji_info: {emoji_code: 20},
user_circle_class: "user_circle_empty",
is_group: false,
});
assert.deepEqual(
list_info.conversations_to_be_shown.map((conversation) => conversation.recipients),
["Alice, Bob", "Shiv", "Cardelio", "Zoe", "Me Myself", "Alice"],
);
});

77
static/js/pm_list_data.js
View File

@ -6,6 +6,12 @@ import * as pm_conversations from "./pm_conversations";
import * as unread from "./unread";
import * as user_status from "./user_status";
// Maximum number of conversation threads to show in default view.
const max_conversations_to_show = 5;
// Maximum number of conversation threads to show in default view with unreads.
const max_conversations_to_show_with_unreads = 8;
export function get_active_user_ids_string() {
const filter = narrow_state.filter();
@ -25,17 +31,17 @@ export function get_active_user_ids_string() {
export function get_conversations() {
const private_messages = pm_conversations.recent.get();
const display_objects = [];
// The user_ids_string for the current view, if any.
const active_user_ids_string = get_active_user_ids_string();
for (const private_message_obj of private_messages) {
const user_ids_string = private_message_obj.user_ids_string;
for (const conversation of private_messages) {
const user_ids_string = conversation.user_ids_string;
const reply_to = people.user_ids_string_to_emails_string(user_ids_string);
const recipients_string = people.get_recipients(user_ids_string);
const num_unread = unread.num_unread_for_person(user_ids_string);
const is_group = user_ids_string.includes(",");
const is_active = user_ids_string === active_user_ids_string;
let user_circle_class;
@ -47,8 +53,10 @@ export function get_conversations() {
const recipient_user_obj = people.get_by_user_id(user_id);
if (recipient_user_obj.is_bot) {
// Bots do not have status emoji, and are modeled as
// always present. We may want to use this space for a
// bot icon in the future.
user_circle_class = "user_circle_green";
// bots do not have status emoji
} else {
status_emoji_info = user_status.get_status_emoji(user_id);
}
@ -80,3 +88,62 @@ export function is_all_privates() {
return filter.operands("is").includes("private");
}
// Designed to closely match topic_list_data.get_list_info().
export function get_list_info(zoomed) {
const conversations = get_conversations();
if (zoomed || conversations.length <= max_conversations_to_show) {
return {
conversations_to_be_shown: conversations,
more_conversations_unread_count: 0,
};
}
const conversations_to_be_shown = [];
let more_conversations_unread_count = 0;
for (const [idx, conversation] of conversations.entries()) {
function should_show_conversation(conversation) {
// We always show the active conversation; see the similar
// comment in topic_list_data.js.
if (conversation.is_active) {
return true;
}
// We don't need to filter muted users here, because
// pm_conversations.js takes care of this for us.
// We include the most recent max_conversations_to_show
// conversations, regardless of whether they have unread
// messages.
if (idx < max_conversations_to_show) {
return true;
}
// We include older conversations with unread messages up
// until max_conversations_to_show_with_unreads total
// topics have been included.
if (
conversation.unread > 0 &&
conversations_to_be_shown.length < max_conversations_to_show_with_unreads
) {
return true;
}
// Otherwise, this conversation should only be visible in
// the unzoomed view.
return false;
}
if (should_show_conversation(conversation)) {
conversations_to_be_shown.push(conversation);
} else {
more_conversations_unread_count += conversation.unread;
}
}
return {
conversations_to_be_shown,
more_conversations_unread_count,
};
}