diff --git a/frontend_tests/node_tests/buddy_data.js b/frontend_tests/node_tests/buddy_data.js index d182b438a3..3b4113d098 100644 --- a/frontend_tests/node_tests/buddy_data.js +++ b/frontend_tests/node_tests/buddy_data.js @@ -12,8 +12,11 @@ const page_params = set_global("page_params", {}); const timerender = mock_esm("../../static/js/timerender"); +const compose_fade_helper = zrequire("compose_fade_helper"); +const peer_data = zrequire("peer_data"); const people = zrequire("people"); const presence = zrequire("presence"); +const stream_data = zrequire("stream_data"); const user_status = zrequire("user_status"); const buddy_data = zrequire("buddy_data"); @@ -93,6 +96,9 @@ function add_canned_users() { function test(label, f) { run_test(label, (override) => { + compose_fade_helper.clear_focused_recipient(); + stream_data.clear_subscriptions(); + peer_data.clear_for_testing(); user_status.initialize({user_status: {}}); presence.presence_info.clear(); people.init(); @@ -166,6 +172,121 @@ test("user_circle", () => { assert.equal(buddy_data.get_user_circle_class(me.user_id), "user_circle_green"); }); +test("compose fade interactions (streams)", () => { + const sub = { + stream_id: 101, + name: "Devel", + subscribed: true, + }; + stream_data.add_sub(sub); + stream_data.subscribe_myself(sub); + stream_data.update_calculated_fields(sub); + + people.add_active_user(fred); + + set_presence(fred.user_id, "active"); + + function faded() { + return buddy_data.get_item(fred.user_id).faded; + } + + // If we are not narrowed, then we don't fade fred in the buddy list. + assert.equal(faded(), false); + + // If we narrow to a stream that fred has not subscribed + // to, we will fade him. + compose_fade_helper.set_focused_recipient({ + type: "stream", + stream_id: sub.stream_id, + topic: "whatever", + }); + assert.equal(faded(), true); + + // If we subscribe, we don't fade. + peer_data.add_subscriber(sub.stream_id, fred.user_id); + assert.equal(faded(), false); + + // Test our punting logic. + const bogus_stream_id = 99999; + assert.equal(stream_data.get_sub_by_id(bogus_stream_id), undefined); + + compose_fade_helper.set_focused_recipient({ + type: "stream", + stream_id: bogus_stream_id, + }); + + assert.equal(faded(), false); +}); + +test("compose fade interactions (missing topic)", () => { + const sub = { + stream_id: 102, + name: "Social", + subscribed: true, + }; + stream_data.add_sub(sub); + stream_data.subscribe_myself(sub); + stream_data.update_calculated_fields(sub); + + people.add_active_user(fred); + + set_presence(fred.user_id, "active"); + + function faded() { + return buddy_data.get_item(fred.user_id).faded; + } + + // If we are not narrowed, then we don't fade fred in the buddy list. + assert.equal(faded(), false); + + // If we narrow to a stream that fred has not subscribed + // to, we will fade him. + compose_fade_helper.set_focused_recipient({ + type: "stream", + stream_id: sub.stream_id, + topic: "whatever", + }); + assert.equal(faded(), true); + + // If the user clears the topic, we won't fade fred. + compose_fade_helper.set_focused_recipient({ + type: "stream", + stream_id: sub.stream_id, + topic: "", + }); + assert.equal(faded(), false); +}); + +test("compose fade interactions (PMs)", () => { + people.add_active_user(fred); + + set_presence(fred.user_id, "active"); + + function faded() { + return buddy_data.get_item(fred.user_id).faded; + } + + // Dont fade if we're not in a narrow. + assert.equal(faded(), false); + + // Fade fred if we are narrowed to a PM narrow that does + // not include him. + compose_fade_helper.set_focused_recipient({ + type: "private", + to_user_ids: "9999999", + }); + assert.equal(faded(), true); + + // Now include fred in a narrow with jill, and we will + // stop fading him. + compose_fade_helper.set_focused_recipient({ + type: "private", + to_user_ids: [fred.user_id, jill.user_id].join(","), + }); + + assert.equal(faded(), false); +}); + test("buddy_status", () => { set_presence(selma.user_id, "active"); set_presence(me.user_id, "active"); diff --git a/frontend_tests/node_tests/compose_fade.js b/frontend_tests/node_tests/compose_fade.js index 0b9ecc127f..3024510851 100644 --- a/frontend_tests/node_tests/compose_fade.js +++ b/frontend_tests/node_tests/compose_fade.js @@ -28,6 +28,7 @@ const stream_data = zrequire("stream_data"); const peer_data = zrequire("peer_data"); const people = zrequire("people"); const compose_fade = zrequire("compose_fade"); +const compose_fade_helper = zrequire("compose_fade_helper"); const me = { email: "me@example.com", @@ -60,14 +61,22 @@ run_test("set_focused_recipient", () => { subscribed: true, can_access_subscribers: true, }; - stream_data.add_sub(sub); - peer_data.set_subscribers(sub.stream_id, [me.user_id, alice.user_id]); compose_fade.set_focused_recipient("stream"); - assert.equal(compose_fade.would_receive_message(me.user_id), true); - assert.equal(compose_fade.would_receive_message(alice.user_id), true); - assert.equal(compose_fade.would_receive_message(bob.user_id), false); + // If a stream is unknown, then we turn off the compose-fade + // feature, since a mix won't happen if the message can't be + // delivered. + stream_data.clear_subscriptions(); + assert.equal(compose_fade_helper.would_receive_message(bob.user_id), true); + + stream_data.add_sub(sub); + peer_data.set_subscribers(sub.stream_id, [me.user_id, alice.user_id]); + compose_fade.set_focused_recipient("stream"); + + assert.equal(compose_fade_helper.would_receive_message(me.user_id), true); + assert.equal(compose_fade_helper.would_receive_message(alice.user_id), true); + assert.equal(compose_fade_helper.would_receive_message(bob.user_id), false); const good_msg = { type: "stream", @@ -79,6 +88,6 @@ run_test("set_focused_recipient", () => { stream_id: 999, topic: "lunch", }; - assert(!compose_fade.should_fade_message(good_msg)); - assert(compose_fade.should_fade_message(bad_msg)); + assert(!compose_fade_helper.should_fade_message(good_msg)); + assert(compose_fade_helper.should_fade_message(bad_msg)); }); diff --git a/static/js/buddy_data.js b/static/js/buddy_data.js index b1be838140..19bf716cc8 100644 --- a/static/js/buddy_data.js +++ b/static/js/buddy_data.js @@ -1,5 +1,5 @@ import * as blueslip from "./blueslip"; -import * as compose_fade from "./compose_fade"; +import * as compose_fade_users from "./compose_fade_users"; import * as hash_util from "./hash_util"; import * as people from "./people"; import * as presence from "./presence"; @@ -268,7 +268,7 @@ export function get_title_data(user_ids_string, is_group) { export function get_item(user_id) { const info = info_for(user_id); - compose_fade.update_user_info([info], fade_config); + compose_fade_users.update_user_info([info], fade_config); return info; } @@ -334,7 +334,7 @@ export function get_filtered_and_sorted_user_ids(user_filter_text) { export function get_items_for_users(user_ids) { const user_info = user_ids.map((user_id) => info_for(user_id)); - compose_fade.update_user_info(user_info, fade_config); + compose_fade_users.update_user_info(user_info, fade_config); return user_info; } diff --git a/static/js/compose_fade.js b/static/js/compose_fade.js index 6846fa7d92..6e3ccee3ad 100644 --- a/static/js/compose_fade.js +++ b/static/js/compose_fade.js @@ -2,6 +2,8 @@ import $ from "jquery"; import _ from "lodash"; import {buddy_list} from "./buddy_list"; +import * as compose_fade_helper from "./compose_fade_helper"; +import * as compose_fade_users from "./compose_fade_users"; import * as compose_state from "./compose_state"; import * as floating_recipient_bar from "./floating_recipient_bar"; import * as message_viewport from "./message_viewport"; @@ -10,21 +12,16 @@ import * as rows from "./rows"; import * as stream_data from "./stream_data"; import * as util from "./util"; -let focused_recipient; let normal_display = false; -export function should_fade_message(message) { - return !util.same_recipient(focused_recipient, message); -} - export function set_focused_recipient(msg_type) { if (msg_type === undefined) { - focused_recipient = undefined; + compose_fade_helper.clear_focused_recipient(); } // Construct focused_recipient as a mocked up element which has all the // fields of a message used by util.same_recipient() - focused_recipient = { + const focused_recipient = { type: msg_type, }; @@ -43,6 +40,8 @@ export function set_focused_recipient(msg_type) { focused_recipient.reply_to = reply_to; focused_recipient.to_user_ids = people.reply_to_to_user_ids_string(reply_to); } + + compose_fade_helper.set_focused_recipient(focused_recipient); } function display_messages_normally() { @@ -74,7 +73,7 @@ function fade_messages() { for (i = 0; i < visible_groups.length; i += 1) { first_row = rows.first_message_in_group(visible_groups[i]); first_message = current_msg_list.get(rows.id(first_row)); - should_fade_group = should_fade_message(first_message); + should_fade_group = compose_fade_helper.should_fade_message(first_message); change_fade_state($(visible_groups[i]), should_fade_group); } @@ -98,7 +97,9 @@ function fade_messages() { // sorted as it would be displayed in the message view for (i = 0; i < all_groups.length; i += 1) { const group_elt = $(all_groups[i]); - should_fade_group = should_fade_message(rows.recipient_from_group(group_elt)); + should_fade_group = compose_fade_helper.should_fade_message( + rows.recipient_from_group(group_elt), + ); change_fade_state(group_elt, should_fade_group); } @@ -110,23 +111,6 @@ function fade_messages() { ); } -export function would_receive_message(user_id) { - if (focused_recipient.type === "stream") { - const sub = stream_data.get_sub_by_id(focused_recipient.stream_id); - if (!sub) { - // If the stream isn't valid, there is no risk of a mix - // yet, so we sort of "lie" and say they would receive a - // message. - return true; - } - - return stream_data.is_user_subscribed(focused_recipient.stream_id, user_id); - } - - // PM, so check if the given email is in the recipients list. - return util.is_pm_recipient(user_id, focused_recipient); -} - const user_fade_config = { get_user_id(li) { return buddy_list.get_key_from_li({li}); @@ -139,66 +123,17 @@ const user_fade_config = { }, }; -function update_user_row_when_fading(li, conf) { - const user_id = conf.get_user_id(li); - const would_receive = would_receive_message(user_id); - - if (would_receive || people.is_my_user_id(user_id)) { - conf.unfade(li); - } else { - conf.fade(li); - } -} - -function display_users_normally(items, conf) { - for (const li of items) { - conf.unfade(li); - } -} - -function fade_users(items, conf) { - for (const li of items) { - update_user_row_when_fading(li, conf); - } -} - -function want_normal_display() { - // If we're not composing show a normal display. - if (focused_recipient === undefined) { - return true; - } - - // If the user really hasn't specified anything let, then we want a normal display - if (focused_recipient.type === "stream") { - // If a stream doesn't exist, there is no real chance of a mix, so fading - // is just noise to the user. - if (!stream_data.get_sub_by_id(focused_recipient.stream_id)) { - return true; - } - - // This is kind of debatable. If the topic is empty, it could be that - // the user simply hasn't started typing it yet, but disabling fading here - // means the feature doesn't help realms where topics aren't mandatory - // (which is most realms as of this writing). - if (focused_recipient.topic === "") { - return true; - } - } - - return focused_recipient.type === "private" && focused_recipient.reply_to === ""; -} - function do_update_all() { const user_items = buddy_list.get_items(); - if (want_normal_display()) { + if (compose_fade_helper.want_normal_display()) { if (!normal_display) { display_messages_normally(); - display_users_normally(user_items, user_fade_config); + compose_fade_users.display_users_normally(user_items, user_fade_config); } } else { fade_messages(); - fade_users(user_items, user_fade_config); + compose_fade_users.fade_users(user_items, user_fade_config); } } @@ -208,15 +143,7 @@ function do_update_all() { export function update_faded_users() { const user_items = buddy_list.get_items(); - update_user_info(user_items, user_fade_config); -} - -export function update_user_info(items, conf) { - if (want_normal_display()) { - display_users_normally(items, conf); - } else { - fade_users(items, conf); - } + compose_fade_users.update_user_info(user_items, user_fade_config); } // This gets called on keyup events, hence the throttling. @@ -228,13 +155,13 @@ export function start_compose(msg_type) { } export function clear_compose() { - focused_recipient = undefined; + compose_fade_helper.clear_focused_recipient(); display_messages_normally(); update_faded_users(); } export function update_message_list() { - if (want_normal_display()) { + if (compose_fade_helper.want_normal_display()) { display_messages_normally(); } else { fade_messages(); @@ -242,7 +169,7 @@ export function update_message_list() { } export function update_rendered_message_groups(message_groups, get_element) { - if (want_normal_display()) { + if (compose_fade_helper.want_normal_display()) { return; } @@ -252,7 +179,7 @@ export function update_rendered_message_groups(message_groups, get_element) { for (const message_group of message_groups) { const elt = get_element(message_group); const first_message = message_group.message_containers[0].msg; - const should_fade = should_fade_message(first_message); + const should_fade = compose_fade_helper.should_fade_message(first_message); change_fade_state(elt, should_fade); } } diff --git a/static/js/compose_fade_helper.js b/static/js/compose_fade_helper.js new file mode 100644 index 0000000000..0f676d9198 --- /dev/null +++ b/static/js/compose_fade_helper.js @@ -0,0 +1,59 @@ +import * as stream_data from "./stream_data"; +import * as util from "./util"; + +let focused_recipient; + +export function should_fade_message(message) { + return !util.same_recipient(focused_recipient, message); +} + +export function clear_focused_recipient() { + focused_recipient = undefined; +} + +export function set_focused_recipient(recipient) { + focused_recipient = recipient; +} + +export function would_receive_message(user_id) { + if (focused_recipient.type === "stream") { + const sub = stream_data.get_sub_by_id(focused_recipient.stream_id); + if (!sub) { + // If the stream isn't valid, there is no risk of a mix + // yet, so we sort of "lie" and say they would receive a + // message. + return true; + } + + return stream_data.is_user_subscribed(focused_recipient.stream_id, user_id); + } + + // PM, so check if the given email is in the recipients list. + return util.is_pm_recipient(user_id, focused_recipient); +} + +export function want_normal_display() { + // If we're not composing show a normal display. + if (focused_recipient === undefined) { + return true; + } + + // If the user really hasn't specified anything let, then we want a normal display + if (focused_recipient.type === "stream") { + // If a stream doesn't exist, there is no real chance of a mix, so fading + // is just noise to the user. + if (!stream_data.get_sub_by_id(focused_recipient.stream_id)) { + return true; + } + + // This is kind of debatable. If the topic is empty, it could be that + // the user simply hasn't started typing it yet, but disabling fading here + // means the feature doesn't help realms where topics aren't mandatory + // (which is most realms as of this writing). + if (focused_recipient.topic === "") { + return true; + } + } + + return focused_recipient.type === "private" && focused_recipient.reply_to === ""; +} diff --git a/static/js/compose_fade_users.js b/static/js/compose_fade_users.js new file mode 100644 index 0000000000..9d8d4948b2 --- /dev/null +++ b/static/js/compose_fade_users.js @@ -0,0 +1,33 @@ +import * as compose_fade_helper from "./compose_fade_helper"; +import * as people from "./people"; + +function update_user_row_when_fading(li, conf) { + const user_id = conf.get_user_id(li); + const would_receive = compose_fade_helper.would_receive_message(user_id); + + if (would_receive || people.is_my_user_id(user_id)) { + conf.unfade(li); + } else { + conf.fade(li); + } +} + +export function fade_users(items, conf) { + for (const li of items) { + update_user_row_when_fading(li, conf); + } +} + +export function display_users_normally(items, conf) { + for (const li of items) { + conf.unfade(li); + } +} + +export function update_user_info(items, conf) { + if (compose_fade_helper.want_normal_display()) { + display_users_normally(items, conf); + } else { + fade_users(items, conf); + } +}