mirror of
https://github.com/zulip/zulip.git
synced 2025-10-23 04:52:12 +00:00
compose fade: Extract compose_fade_users class.
We extract compose_fade_users and compose_fade_helper. This is a pretty verbatim extraction of code, apart from adding a few exports and changing the callers. This change makes the buddy_data module no longer sit "above" these files in the dependency graph (at least not via compose_fade): * jquery * lodash (not a big deal) * compose_state * floating_recipient_bar * message_viewport * rows The new moules have dependencies that buddy_data already had directly for other reasons: * people * util And then buddy_data still depends on stream_data indirectly through the compose-fade logic for stream_data. Even without compose-fade, it would depend indirectly on stream_data via hash_util. Note that we could have lifted the calls to compose_fade out of buddy_data to move some dependencies around, but it's useful to have buddy_data fully encapsulate what goes into the buddy list without spreading responsibilities to things like activity.js and buddy_list.js. We can now unit-test the logic at the level of buddy_data, which is a lot easier than trying to do it via modules that delegate drawing or do drawing (such as activity.js and buddy_list.js). Note that we still don't have 100% line coverage on the compose_fade.js module, but all the code that we extracted now is covered, mostly via buddy_data tests.
This commit is contained in:
@@ -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");
|
||||
|
@@ -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));
|
||||
});
|
||||
|
@@ -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;
|
||||
}
|
||||
|
||||
|
@@ -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);
|
||||
}
|
||||
}
|
||||
|
59
static/js/compose_fade_helper.js
Normal file
59
static/js/compose_fade_helper.js
Normal file
@@ -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 === "";
|
||||
}
|
33
static/js/compose_fade_users.js
Normal file
33
static/js/compose_fade_users.js
Normal file
@@ -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);
|
||||
}
|
||||
}
|
Reference in New Issue
Block a user