From a569e34b334b9790b5143b02549d17b456a4439b Mon Sep 17 00:00:00 2001 From: lilia Date: Sat, 7 Nov 2015 14:11:13 -0800 Subject: [PATCH] Refactor new message notification and frontend updates Create a cleaner seperation between generating notifications and updating frontend conversation views. The former is now handled by `conversation.notify` while the latter is achieved by triggering an event on the conversation model, which will only be acted on if there are any views listening for it. Additionally, instead of re-fetching the entire message history, which is overkill, just add or update the new/modified message. This will help speed up the newmessage event handler and also help avoid unnecessary re-rendering when resolving key conflicts. // FREEBIE --- js/background.js | 28 +++++++++++------------ js/models/conversations.js | 25 ++++++++++++++++++++ js/models/messages.js | 7 ++---- js/panel_controller.js | 43 ++++++++--------------------------- js/views/conversation_view.js | 7 +++--- 5 files changed, 53 insertions(+), 57 deletions(-) diff --git a/js/background.js b/js/background.js index 78faacb8d..142492d3d 100644 --- a/js/background.js +++ b/js/background.js @@ -211,7 +211,8 @@ active_at: Date.now(), unreadCount: conversation.get('unreadCount') + 1 }); - notifyConversation(message); + conversation.trigger('newmessage', message); + conversation.notify(message); }); }); return; @@ -223,13 +224,6 @@ // lazy hack window.receipts = new Backbone.Collection(); - function updateConversation(conversationId) { - var conversation = ConversationController.get(conversationId); - if (conversation) { - conversation.trigger('newmessages'); - } - } - function onDeliveryReceipt(ev) { var pushMessage = ev.proto; var timestamp = pushMessage.timestamp.toNumber(); @@ -238,20 +232,24 @@ console.log('delivery receipt', pushMessage.source, timestamp); messages.fetchSentAt(timestamp).then(function() { groups.fetchGroups(pushMessage.source).then(function() { - for (var i in messages.where({type: 'outgoing'})) { - var message = messages.at(i); + var found = false; + messages.where({type: 'outgoing'}).forEach(function(message) { var deliveries = message.get('delivered') || 0; var conversationId = message.get('conversationId'); if (conversationId === pushMessage.source || groups.get(conversationId)) { - message.save({delivered: deliveries + 1, sent: true}).then( + message.save({delivered: deliveries + 1}).then(function() { // notify frontend listeners - updateConversation.bind(null, conversationId) - ); - return; + var conversation = ConversationController.get(conversationId); + if (conversation) { + conversation.trigger('newmessage', message); + } + }); + found = true; // TODO: consider keeping a list of numbers we've // successfully delivered to? } - } + }); + if (found) { return; } // if we get here, we didn't find a matching message. // keep the receipt in memory in case it shows up later // as a sync message. diff --git a/js/models/conversations.js b/js/models/conversations.js index 2a9efb136..ad5f233d2 100644 --- a/js/models/conversations.js +++ b/js/models/conversations.js @@ -314,6 +314,31 @@ }.bind(this)); }.bind(this)); }, + notify: function(message) { + if (!message.isIncoming()) { + this.markRead(); + return; + } + if (window.isOpen() && window.isFocused()) { + return; + } + window.drawAttention(); + var sender = ConversationController.create({ + id: message.get('source'), type: 'private' + }); + var conversationId = this.id; + sender.fetch().then(function() { + sender.getNotificationIcon().then(function(iconUrl) { + Whisper.Notifications.add({ + title : sender.getTitle(), + message : message.getNotificationText(), + iconUrl : iconUrl, + imageUrl : message.getImageUrl(), + conversationId : conversationId + }); + }); + }); + }, hashCode: function() { if (this.hash === undefined) { var string = this.getTitle() || ''; diff --git a/js/models/messages.js b/js/models/messages.js index d88635e77..871e0088b 100644 --- a/js/models/messages.js +++ b/js/models/messages.js @@ -319,11 +319,8 @@ conversation.save().then(function() { message.save().then(function() { - if (message.isIncoming()) { - notifyConversation(message); - } else { - conversation.trigger('newmessages'); - } + conversation.trigger('newmessage', message); + conversation.notify(message); }); }); }); diff --git a/js/panel_controller.js b/js/panel_controller.js index 4c6a15951..6c4e925a6 100644 --- a/js/panel_controller.js +++ b/js/panel_controller.js @@ -17,41 +17,16 @@ } }; - window.notifyConversation = function(message) { - var conversationId = message.get('conversationId'); - var conversation = ConversationController.get(conversationId); - if (!conversation) { - conversation = ConversationController.create({id: conversationId}); - conversation.fetch(); - } - - if (inboxOpened) { - conversation.trigger('newmessages'); - if (inboxFocused) { - return; - } - if (inboxOpened) { - extension.windows.drawAttention(inboxWindowId); - } - } + window.isFocused = function() { + return inboxFocused; + }; + window.isOpen = function() { + return inboxOpened; + }; - if (Whisper.Notifications.isEnabled()) { - var sender = ConversationController.create({id: message.get('source')}); - conversation.fetch().then(function() { - sender.fetch().then(function() { - sender.getNotificationIcon().then(function(iconUrl) { - Whisper.Notifications.add({ - title : sender.getTitle(), - message : message.getNotificationText(), - iconUrl : iconUrl, - imageUrl : message.getImageUrl(), - conversationId: conversation.id - }); - }); - }); - }); - } else { - openConversation(conversation); + window.drawAttention = function() { + if (inboxOpened && !inboxFocused) { + extension.windows.drawAttention(inboxWindowId); } }; diff --git a/js/views/conversation_view.js b/js/views/conversation_view.js index 7c6b59db3..b1eb29a50 100644 --- a/js/views/conversation_view.js +++ b/js/views/conversation_view.js @@ -23,7 +23,7 @@ initialize: function(options) { this.listenTo(this.model, 'destroy', this.stopListening); this.listenTo(this.model, 'change:name', this.updateTitle); - this.listenTo(this.model, 'newmessages', this.fetchMessages); + this.listenTo(this.model, 'newmessage', this.addMessage); this.listenTo(this.model, 'change:unreadCount', this.onUnread); this.listenTo(this.model, 'opened', this.focusMessageField); @@ -87,8 +87,9 @@ focusMessageField: function() { this.$messageField.focus(); }, - fetchMessages: function() { - this.model.fetchMessages(); + + addMessage: function(message) { + this.model.messageCollection.add(message, {merge: true}); }, viewMembers: function() {