From 748e5b6da6e1e55799add8db76eaa5e512927d3a Mon Sep 17 00:00:00 2001 From: Leo Franchi Date: Mon, 3 Feb 2014 16:48:25 -0500 Subject: [PATCH] Remove disabled summarization code This experiment has been disabled for everyone for a while: if we bring something like this back, it is not likely to be exactly the same, and will be different enough to require a different implementation. As it is, the summarization code was making a few code paths (rendering especially) more complex, and is worth removing for simplicity's sake. (imported from commit 6ac8cdc9f7077a5a1da01ab4268aba3db0bc43f8) --- static/js/feature_flags.js | 1 - static/js/hotkey.js | 9 --- static/js/message_flags.js | 2 - static/js/message_list.js | 50 +--------------- static/js/message_list_view.js | 91 +---------------------------- static/js/message_store.js | 8 --- static/js/narrow.js | 38 +----------- static/js/rows.js | 2 +- static/js/summary.js | 31 ---------- static/js/ui.js | 55 ----------------- static/js/unread.js | 1 - static/js/zulip.js | 6 +- static/templates/message.handlebars | 60 ------------------- zproject/settings.py | 1 - 14 files changed, 6 insertions(+), 349 deletions(-) delete mode 100644 static/js/summary.js diff --git a/static/js/feature_flags.js b/static/js/feature_flags.js index b6cc1951d0..70cfdc9ea9 100644 --- a/static/js/feature_flags.js +++ b/static/js/feature_flags.js @@ -55,7 +55,6 @@ exports.fade_at_stream_granularity = page_params.staging; exports.fade_users_when_composing = true; exports.mark_read_at_bottom = true; exports.propagate_topic_edits = true; -exports.summarize_read_while_narrowed = false; exports.clicking_notification_causes_narrow = true; exports.use_socket = true; diff --git a/static/js/hotkey.js b/static/js/hotkey.js index bb67af1707..5989b615dd 100644 --- a/static/js/hotkey.js +++ b/static/js/hotkey.js @@ -311,15 +311,6 @@ function process_hotkey(e) { return true; } - if (current_msg_list.on_expandable_row()) { - switch (event_name) { - case 'enter': - ui.expand_summary_row(current_msg_list.selected_row().expectOne()); - return true; - } - return false; - } - // Shortcuts that operate on a message switch (event_name) { case 'message_actions': diff --git a/static/js/message_flags.js b/static/js/message_flags.js index 9f6f3720c6..3304511f2e 100644 --- a/static/js/message_flags.js +++ b/static/js/message_flags.js @@ -69,8 +69,6 @@ function batched_updater(flag, op, immediate) { } exports.send_read = batched_updater('read', 'add'); -exports.send_summarize_in_stream = batched_updater('summarize_in_stream', 'add'); -exports.send_summarize_in_home = batched_updater('summarize_in_home', 'add'); function send_flag(messages, flag_name, set_flag) { var op = set_flag ? 'add' : 'remove'; diff --git a/static/js/message_list.js b/static/js/message_list.js index 7c73f968c8..8a41c9adc3 100644 --- a/static/js/message_list.js +++ b/static/js/message_list.js @@ -2,8 +2,7 @@ function MessageList(table_name, filter, opts) { _.extend(this, { collapse_messages: true, - muting_enabled: true, - summarize_read: false + muting_enabled: true }, opts); this.view = new MessageListView(this, table_name, this.collapse_messages); @@ -23,7 +22,6 @@ function MessageList(table_name, filter, opts) { this.narrowed = this.table_name === "zfilt"; this.num_appends = 0; - this.min_id_exempted_from_summaries = -1; return this; } @@ -205,10 +203,6 @@ MessageList.prototype = { return this.get_row(this._selected_id); }, - on_expandable_row: function MessageList_on_expandable_row() { - return this.view.is_expandable_row(this.selected_row()); - }, - closest_id: function MessageList_closest_id(id) { var items = this._items; @@ -279,34 +273,6 @@ MessageList.prototype = { }); }, - is_summarized_message: function (message) { - if (!feature_flags.summarize_read_while_narrowed || - message === undefined || message.flags === undefined) { - return false; - } - if (message.id >= this.min_id_exempted_from_summaries) { - return false; - } - if (this.summarize_read === 'home') { - return message.flags.indexOf('summarize_in_home') !== -1; - } else if (this.summarize_read === 'stream' ) { - return message.flags.indexOf('summarize_in_stream') !== -1; - } else { - return false; - } - }, - - summary_adjective: function (message) { - if (_.contains(message.flags, 'force_collapse')) { - return 'collapsed'; - } else if (!_.contains(message.flags, 'force_expand')) { - if (this.is_summarized_message(message)) { - return 'read'; - } - } - return null; - }, - selected_idx: function MessageList_selected_idx() { return util.lower_bound(this._items, this._selected_id, function (a, b) { return a.id < b; }); @@ -341,11 +307,6 @@ MessageList.prototype = { } }, - start_summary_exemption: function MessageList_start_summary_exemption() { - var num_exempt = 8; - this.min_id_exempted_from_summaries = this.nth_most_recent_id(num_exempt); - }, - unmuted_messages: function MessageList_unmuted_messages(messages) { return _.reject(messages, function (message) { return muting.is_topic_muted(message.stream, message.subject) && @@ -365,11 +326,6 @@ MessageList.prototype = { } this._items = this._items.concat(viewable_messages); - if (this.num_appends === 0) { - // We can't figure out which messages need to be exempt from - // summarization until we get the first batch of messages. - this.start_summary_exemption(); - } this.num_appends += 1; this._add_to_hash(messages); @@ -558,10 +514,6 @@ MessageList.prototype = { this._selected_id = new_id; } - if (this.min_id_exempted_from_summaries === old_id) { - this.min_id_exempted_from_summaries = new_id; - } - // If this message is now out of order, re-order and re-render var self = this; setTimeout(function () { diff --git a/static/js/message_list_view.js b/static/js/message_list_view.js index 82e865743c..4a599e3c72 100644 --- a/static/js/message_list_view.js +++ b/static/js/message_list_view.js @@ -73,9 +73,6 @@ MessageListView.prototype = { var current_group = []; var new_message_groups = []; - var summary_group = {}; - var has_summary = false; - var summary_start_id = 0; var self = this; if (where === "bottom") { @@ -109,11 +106,6 @@ MessageListView.prototype = { var last_row = table.find('div[zid]:last'); last_message_id = rows.id(last_row); prev = self.get_message(last_message_id); - - if (last_row.is('.summary_row')) { - // Don't group with a summary, but don't put separators before the new message - prev = _.pick(prev, 'timestamp', 'historical'); - } } function set_template_properties(message) { @@ -124,28 +116,6 @@ MessageListView.prototype = { } } - function finish_summary() { - var first = true; - - _.each(summary_group, function (summary_row) { - summary_row.count = summary_row.messages.length; - summary_row.message_ids = _.pluck(summary_row.messages, 'id').join(' '); - if (first) { - summary_row.include_bookend = true; - first = false; - } - set_template_properties(summary_row); - messages_to_render.push(summary_row); - prev = summary_row; - }); - - prev.include_footer = true; - - has_summary = false; - summary_group = {}; - prev = _.pick(prev, 'timestamp', 'historical'); - } - function finish_group() { if (current_group.length > 0) { var message_ids = _.pluck(current_group, 'id'); @@ -162,53 +132,6 @@ MessageListView.prototype = { add_display_time(message, prev); - if (has_summary && message.show_date) { - finish_summary(); - } - - var summary_adjective = list.summary_adjective(message); - - if (summary_adjective) { - if (prev) { - prev.include_footer = true; - } - - var key = util.recipient_key(message); - if (summary_group[key] === undefined) { - // Start building a new summary row for messages from this recipient. - // - // Ugly: handlebars renderer only takes messages. We don't want to modify - // the original message, so we make a fake message based on the real one - // that will trigger the right part of the handlebars template and won't - // show the content, date, etc. from the real message. - var fake_message = _.extend(_.pick(message, - 'timestamp', 'show_date', 'historical', 'stream', - 'is_stream', 'subject', 'display_recipient', 'display_reply_to'), { - is_summary: true, - include_footer: false, - include_bookend: false, - first_message_id: message.id, - summary_adjective: summary_adjective, - messages: [message] - }); - if (message.stream) { - fake_message.stream_url = narrow.by_stream_uri(message.stream); - fake_message.topic_url = narrow.by_stream_subject_uri(message.stream, message.subject); - } else { - fake_message.pm_with_url = narrow.pm_with_uri(message.reply_to); - } - - summary_group[key] = fake_message; - } else { - summary_group[key].messages.push(message); - } - has_summary = true; - prev = message; - return; - } else if (has_summary) { - finish_summary(); - } - if (util.same_recipient(prev, message) && self.collapse_messages && prev.historical === message.historical && !message.show_date) { current_group.push(message); @@ -297,10 +220,6 @@ MessageListView.prototype = { prev.include_footer = true; } - if (has_summary) { - finish_summary(); - } - if (messages_to_render.length === 0) { return; } @@ -323,11 +242,7 @@ MessageListView.prototype = { var row = $(elem); // Save DOM elements by id into self._rows for O(1) lookup - if (row.hasClass('summary_row')) { - _.each(row.attr('data-messages').split(' '), function (id) { - self._rows[id] = elem; - }); - } else if (row.hasClass('message_row')) { + if (row.hasClass('message_row')) { self._rows[row.attr('zid')] = elem; } @@ -675,10 +590,6 @@ MessageListView.prototype = { return this.get_row(this.list.selected_id()); }, - is_expandable_row: function MessageListView_is_expandable_row(row) { - return row.hasClass('summary_row'); - }, - get_message: function MessageListView_get_message(id) { return this.list.get(id); }, diff --git a/static/js/message_store.js b/static/js/message_store.js index 278b38b961..7ac7242575 100644 --- a/static/js/message_store.js +++ b/static/js/message_store.js @@ -279,14 +279,6 @@ exports.update_messages = function update_messages(events) { exports.insert_new_messages = function insert_new_messages(messages) { messages = _.map(messages, add_message_metadata); - if (feature_flags.summarize_read_while_narrowed) { - _.each(messages, function (message) { - if (message.sent_by_me) { - summary.maybe_mark_summarized(message); - } - }); - } - // You must add add messages to home_msg_list BEFORE // calling unread.process_loaded_messages. exports.add_messages(messages, home_msg_list, {messages_are_new: true}); diff --git a/static/js/narrow.js b/static/js/narrow.js index 0728421e03..948cb6de33 100644 --- a/static/js/narrow.js +++ b/static/js/narrow.js @@ -153,9 +153,7 @@ exports.activate = function (raw_operators, opts) { blueslip.debug("Narrowed", {operators: _.map(operators, function (e) { return e.operator; }), trigger: opts ? opts.trigger : undefined, - previous_id: current_msg_list.selected_id(), - previous_is_summarized: current_msg_list.is_summarized_message( - current_msg_list.get(current_msg_list.selected_id()))}); + previous_id: current_msg_list.selected_id()}); var had_message_content = compose.has_message_content(); @@ -230,8 +228,7 @@ exports.activate = function (raw_operators, opts) { var msg_list = new MessageList('zfilt', current_filter, { collapse_messages: ! current_filter.is_search(), - muting_enabled: muting_enabled, - summarize_read: this.summary_enabled() + muting_enabled: muting_enabled }); msg_list.start_time = start_time; @@ -447,13 +444,6 @@ exports.deactivate = function () { empty_ok: true }; - if (feature_flags.summarize_read_while_narrowed) { - // TODO: avoid a full re-render - // Necessary to replace messages read in the narrow with summary blocks - current_msg_list.start_summary_exemption(); - current_msg_list.rerender(); - } - // We fall back to the closest selected id, if the user has removed a // stream from the home view since leaving it the old selected id might // no longer be there @@ -626,30 +616,6 @@ exports.muting_enabled = function () { return (!exports.narrowed_to_topic() && !exports.narrowed_to_search() && !exports.narrowed_to_pms()); }; -exports.summary_enabled = function () { - if (!feature_flags.summarize_read_while_narrowed) { - return false; - } - - if (current_filter === undefined){ - return 'home'; // Home view, but this shouldn't run anyway - } - - var operators = current_filter.operators(); - - if (operators.length === 1 && ( - current_filter.has_operand("in", "home") || - current_filter.has_operand("in", "all"))) { - return 'home'; - } - - if (operators.length === 1 && ( - current_filter.operands("stream").length === 1 || - current_filter.has_operand("is", "private"))) { - return 'stream'; - } -}; - return exports; }()); diff --git a/static/js/rows.js b/static/js/rows.js index 845d000d35..27c6e04c88 100644 --- a/static/js/rows.js +++ b/static/js/rows.js @@ -54,7 +54,7 @@ exports.get_table = function (table_name) { exports.get_closest_row = function (element) { // This gets the closest message row to an element, whether it's - // a summary bar, recipient bar, or message. With our current markup, + // a recipient bar or message. With our current markup, // this is the most reliable way to do it. return $(element).closest("div.message_row, div.recipient_row"); }; diff --git a/static/js/summary.js b/static/js/summary.js deleted file mode 100644 index 06b449c436..0000000000 --- a/static/js/summary.js +++ /dev/null @@ -1,31 +0,0 @@ -var summary = (function () { - -// This module has helper functions for the August 2013 message -// summarizing experiment. - -var exports = {}; - -function mark_summarized(message) { - if (narrow.narrowed_by_reply()) { - // Narrowed to a topic or PM recipient - message_flags.send_summarize_in_stream(message); - } - - if (narrow.active() && !narrow.narrowed_to_search()) { - // Narrowed to anything except a search - message_flags.send_summarize_in_home(message); - } -} - -exports.maybe_mark_summarized = function (message) { - if (feature_flags.summarize_read_while_narrowed) { - mark_summarized(message); - } -}; - - -return exports; -}()); -if (typeof module !== 'undefined') { - module.exports = summary; -} diff --git a/static/js/ui.js b/static/js/ui.js index 7eb075be7d..d459a0dfd4 100644 --- a/static/js/ui.js +++ b/static/js/ui.js @@ -862,50 +862,6 @@ exports.collapse = function (row) { show_more_link(row); }; -exports.expand_summary_row = function (row) { - var message_ids = row.attr('data-messages').split(' '); - var messages = _.map(message_ids, function (id) { - return current_msg_list.get(id); - }); - - _.each(messages, function (msg){ - msg.flags = _.without(msg.flags, 'force_collapse'); - msg.flags.push('force_expand'); - }); - message_flags.send_force_expand(messages, true); - message_flags.send_force_collapse(messages, false); - - //TODO: Avoid a full re-render - home_msg_list.rerender(); - if (current_msg_list !== home_msg_list) { - current_msg_list.rerender(); - } - - current_msg_list.select_id(message_ids[0]); -}; - -exports.collapse_recipient_group = function (row) { - var message_ids = row.attr('data-messages').split(','); - var messages = _.map(message_ids, function (id) { - return message_store.get(id); - }); - - _.each(messages, function (msg){ - msg.flags = _.without(msg.flags, 'force_expand'); - msg.flags.push('force_collapse'); - }); - message_flags.send_force_expand(messages, false); - message_flags.send_force_collapse(messages, true); - - //TODO: Avoid a full re-render - home_msg_list.rerender(); - if (current_msg_list !== home_msg_list) { - current_msg_list.rerender(); - } - - current_msg_list.select_id(message_ids[0]); -}; - /* EXPERIMENTS */ /* This method allows an advanced user to use the console @@ -1246,17 +1202,6 @@ $(function () { $("#navbar-buttons").addClass("right-userlist"); } - if (feature_flags.summarize_read_while_narrowed) { - $("#main_div").on("click", ".summary_row .messages-expand", function (e) { - exports.expand_summary_row($(e.target).closest('.summary_row').expectOne()); - e.stopImmediatePropagation(); - }); - $("#main_div").on("click", ".recipient_row .messages-collapse", function (e) { - exports.collapse_recipient_group($(e.target).closest('.recipient_row').expectOne()); - e.stopImmediatePropagation(); - }); - } - function is_clickable_message_element(target) { return target.is("a") || target.is("img.message_inline_image") || target.is("img.twitter-avatar") || target.is("div.message_length_controller") || target.is("textarea") || target.is("input") || diff --git a/static/js/unread.js b/static/js/unread.js index 668ebf6b15..2306d473df 100644 --- a/static/js/unread.js +++ b/static/js/unread.js @@ -229,7 +229,6 @@ exports.mark_messages_as_read = function mark_messages_as_read (messages, option if (options.from !== "server") { message_flags.send_read(message); } - summary.maybe_mark_summarized(message); message.unread = false; unread.process_read_message(message, options); diff --git a/static/js/zulip.js b/static/js/zulip.js index 08d1e77b04..5b1d1fd65d 100644 --- a/static/js/zulip.js +++ b/static/js/zulip.js @@ -3,11 +3,7 @@ var all_msg_list = new MessageList( {muting_enabled: false} ); var home_msg_list = new MessageList('zhome', - new Filter([{operator: "in", operand: "home"}]), - { - muting_enabled: true, - summarize_read: feature_flags.summarize_read_while_narrowed?'home':false - } + new Filter([{operator: "in", operand: "home"}]), {muting_enabled: true} ); var narrowed_msg_list; var current_msg_list = home_msg_list; diff --git a/static/templates/message.handlebars b/static/templates/message.handlebars index d1090fff13..5f8e168d84 100644 --- a/static/templates/message.handlebars +++ b/static/templates/message.handlebars @@ -28,65 +28,6 @@
{{{show_date}}}
{{/if}} -{{#if is_summary}} -
-
- - {{! [+] }} - - - {{#if is_stream}} - - {{! invite-only lock icon }} - {{#if invite_only}} - - {{/if}} - - {{! stream }} - {{display_recipient}} - - - {{! > }} -   - - >  - - {{! topic }} - - - {{subject}} - - - {{! exterior links (e.g. to a trac ticket) }} - {{#each subject_links}} - - - - {{/each}} - - - {{else}} - - {{! You and Somebody Else, links to PM narrow}} - - You and {{display_reply_to}} - - - {{/if}} - - {{! "(5 read)" or something similar}} - ({{count}} {{summary_adjective}}) - -
-
-{{else}} - {{#include_recipient}}
{{#if is_stream}} @@ -214,7 +155,6 @@
-{{/if}} {{/with}} {{/each}} diff --git a/zproject/settings.py b/zproject/settings.py index e1227fe6fd..64bd54dec3 100644 --- a/zproject/settings.py +++ b/zproject/settings.py @@ -513,7 +513,6 @@ JS_SPECS = { 'third/marked/lib/marked.js', 'templates/compiled.js', 'js/feature_flags.js', - 'js/summary.js', 'js/util.js', 'js/dict.js', 'js/localstorage.js',