From 5a826040d83c31280f08059afa16efeed5ec565b Mon Sep 17 00:00:00 2001 From: Steve Howell Date: Wed, 5 Jun 2013 15:04:06 -0400 Subject: [PATCH] Clean up code for unread counts and notifications. The core simplification here is that zephyr.js no longer has: * the global home_unread_messages * the function unread_in_current_view() [which used the global] The logic that used to be in zephyr is now in its proper home of unread.js, which has these changes: * the structure returned from unread.get_counts() includes a new member called unread_in_current_view * there's a helper function unread.num_unread_current_messages() Deprecating zephyr.unread_in_current_view() affected two callers: * notifications.update_title_count() * notifications_bar.update() The above functions used to call back to zephyr to get counts, but there was no nice way to enforce that they were getting counts at the right time in the code flow, because they depended on functions like process_visible_unread_messages() to orchestrate updating internal unread counts before pushing out counts to the DOM. Now both of those function take a parameter with the unread count, and we then had to change all of their callers appropriately. This went hand in hand with another goal, which is that we want all the unread-counts logic to funnel though basically one place, which is zephyr.update_unread_counts(). So now that function always calls notifications_bar.update() [NEW] as well as calling into the modules unread.js, stream_list.js, and notifications.js [OLD]. Adding the call to notifications_bar.update() in update_unread_counts() made it so that some other places in the code no longer needed to call notifications_bar.update(), so you'll see some lines of code removed. There are also cases where notifications.update_title_count() was called redundantly, since the callers were already reaching update_unread_counts() via other calls. Finally, in ui.resizehandler, you'll see a simple case where the call to notifications_bar.update() is preceded by an explicit call to unread.get_counts(). (imported from commit ce84b9c8076c1f9bb20a61209913f0cb0dae098c) --- zephyr/static/js/notifications.js | 7 +++---- zephyr/static/js/notifications_bar.js | 4 ++-- zephyr/static/js/ui.js | 18 +++++++++++------ zephyr/static/js/unread.js | 19 ++++++++++++++++++ zephyr/static/js/zephyr.js | 28 ++++----------------------- 5 files changed, 40 insertions(+), 36 deletions(-) diff --git a/zephyr/static/js/notifications.js b/zephyr/static/js/notifications.js index eee5229120..a6596f9580 100644 --- a/zephyr/static/js/notifications.js +++ b/zephyr/static/js/notifications.js @@ -21,13 +21,13 @@ function browser_desktop_notifications_on () { exports.initialize = function () { $(window).focus(function () { window_has_focus = true; - exports.update_title_count(); $.each(notice_memory, function (index, notice_mem_entry) { notice_mem_entry.obj.cancel(); }); - + // Update many places on the DOM to reflect unread + // counts. process_visible_unread_messages(); }).blur(function () { window_has_focus = false; @@ -64,11 +64,10 @@ exports.initialize = function () { } }; -exports.update_title_count = function () { +exports.update_title_count = function (new_message_count) { // Update window title and favicon to reflect unread messages in current view var n; - var new_message_count = unread_in_current_view(); var new_title = (new_message_count ? ("(" + new_message_count + ") ") : "") + page_params.domain + " - Humbug"; diff --git a/zephyr/static/js/notifications_bar.js b/zephyr/static/js/notifications_bar.js index 07cb6644ef..931f77fe1d 100644 --- a/zephyr/static/js/notifications_bar.js +++ b/zephyr/static/js/notifications_bar.js @@ -28,10 +28,10 @@ function hide() { // If there's a custom message, or if the last message is off the bottom of the // screen, then show the notifications bar. -exports.update = function () { +exports.update = function (num_unread) { if (rows.last_visible().offset() !== null // otherwise the next line will error && rows.last_visible().offset().top + rows.last_visible().height() > viewport.scrollTop() + viewport.height() - $("#compose").height() - && unread_in_current_view() > 0) + && num_unread > 0) show(); else hide(); diff --git a/zephyr/static/js/ui.js b/zephyr/static/js/ui.js index 70b97f94de..6220c1b961 100644 --- a/zephyr/static/js/ui.js +++ b/zephyr/static/js/ui.js @@ -355,8 +355,15 @@ function resizehandler(e) { if (current_msg_list.selected_id() !== -1) { scroll_to_selected(); } - // When the screen resizes, it may cause some messages to go off the screen - notifications_bar.update(); + + // When the screen resizes, it can make it so that messages are + // now on the page, so we need to update the notifications bar. + // We may want to do more here in terms of updating unread counts, + // but it's possible that resize events can happen as part of + // screen resolution changes, so we might want to wait for a more + // intentional action to say that the user has "read" a message. + var res = unread.get_counts(); + notifications_bar.update(res.unread_in_current_view); } $(function () { @@ -763,10 +770,9 @@ $(function () { have_scrolled_away_from_top = true; } // When the window scrolls, it may cause some messages to - // enter the screen and become read - notifications_bar.update(); - notifications.update_title_count(); - + // enter the screen and become read. Calling + // process_visible_unread_messages will update necessary + // data structures and DOM elements. setTimeout(process_visible_unread_messages, 0); } } diff --git a/zephyr/static/js/unread.js b/zephyr/static/js/unread.js index 8f3b9c7884..85c193eee5 100644 --- a/zephyr/static/js/unread.js +++ b/zephyr/static/js/unread.js @@ -110,6 +110,18 @@ exports.declare_bankruptcy = function () { unread_counts = {'stream': {}, 'private': {}}; }; +exports.num_unread_current_messages = function () { + var num_unread = 0; + + $.each(current_msg_list.all(), function (idx, msg) { + if ((msg.id > current_msg_list.selected_id()) && exports.message_unread(msg)) { + num_unread += 1; + } + }); + + return num_unread; +}; + exports.get_counts = function () { var res = {}; @@ -159,6 +171,13 @@ exports.get_counts = function () { res.private_message_count = pm_count; res.home_unread_messages += pm_count; + if (narrow.active()) { + res.unread_in_current_view = exports.num_unread_current_messages(); + } + else { + res.unread_in_current_view = res.home_unread_messages; + } + return res; }; diff --git a/zephyr/static/js/zephyr.js b/zephyr/static/js/zephyr.js index 7c1a54aaf9..6283aadb8a 100644 --- a/zephyr/static/js/zephyr.js +++ b/zephyr/static/js/zephyr.js @@ -271,36 +271,16 @@ function send_queued_flags() { success: on_success}); } -var home_unread_messages = 0; - -function unread_in_current_view() { - var num_unread = 0; - if (!narrow.active()) { - num_unread = home_unread_messages; - } else { - $.each(current_msg_list.all(), function (idx, msg) { - if (unread.message_unread(msg) && msg.id > current_msg_list.selected_id()) { - num_unread += 1; - } - }); - } - return num_unread; -} - function update_unread_counts() { // Pure computation: var res = unread.get_counts(); // Side effects from here down: - - // TODO: deprecate this global. - home_unread_messages = res.home_unread_messages; - // This updates some DOM elements directly, so try to // avoid excessive calls to this. stream_list.update_dom_with_unread_counts(res); - - notifications.update_title_count(); + notifications.update_title_count(res.unread_in_current_view); + notifications_bar.update(res.unread_in_current_view); } function mark_all_as_read(cont) { @@ -696,8 +676,8 @@ function add_messages(messages, msg_list) { typeahead_helper.update_autocomplete(); } - // If the new messages are off the screen, show a notification - notifications_bar.update(); + // There are some other common tasks that happen when adding messages, but these + // happen higher up in the stack, notably logic related to unread counts. } function deduplicate_messages(messages) {