From ccd356082095020206fc31bdb763664564e63bbb Mon Sep 17 00:00:00 2001 From: Daniel Gasienica Date: Wed, 9 May 2018 18:12:31 -0400 Subject: [PATCH] Notification improvements - remove on read, on focus, on exit - show multi-message notifications like '5 new messages' --- _locales/en/messages.json | 12 ++++++ js/background.js | 4 ++ js/models/conversations.js | 23 +++++++++--- js/modules/types/message.js | 3 ++ js/notifications.js | 75 +++++++++++++++++++++++++++---------- js/read_syncs.js | 1 + ts/types/Message.ts | 13 +++++++ 7 files changed, 106 insertions(+), 25 deletions(-) diff --git a/_locales/en/messages.json b/_locales/en/messages.json index 5fe98b563..0efe17ca4 100644 --- a/_locales/en/messages.json +++ b/_locales/en/messages.json @@ -713,6 +713,18 @@ "message": "New Messages", "description": "Displayed in notifications for multiple messages" }, + "notificationMostRecentFrom": { + "message": "Most recent from:", + "description": "Displayed in notifications when setting is 'name only' and more than one message is waiting" + }, + "notificationFrom": { + "message": "From:", + "description": "Displayed in notifications when setting is 'name only' and one message is waiting" + }, + "notificationMostRecent": { + "message": "Most recent:", + "description": "Displayed in notifications when setting is 'name and message' and " + }, "messageNotSent": { "message": "Message not sent.", "description": "Informational label, appears on messages that failed to send" diff --git a/js/background.js b/js/background.js index 5c0dc8878..19fe65408 100644 --- a/js/background.js +++ b/js/background.js @@ -238,6 +238,10 @@ appView.openInbox(); } }); + + window.addEventListener('focus', () => Whisper.Notifications.clear()); + window.addEventListener('unload', () => Whisper.Notifications.clear()); + Whisper.Notifications.on('click', function(conversation) { showWindow(); if (conversation) { diff --git a/js/models/conversations.js b/js/models/conversations.js index d4034969f..f159ca027 100644 --- a/js/models/conversations.js +++ b/js/models/conversations.js @@ -1629,14 +1629,27 @@ 'private' ).then(sender => sender.getNotificationIcon().then(iconUrl => { - console.log('adding notification'); + const messageJSON = message.toJSON(); + const messageSentAt = messageJSON.sent_at; + const messageId = message.id; + const isExpiringMessage = Signal.Types.Message.hasExpiration( + messageJSON + ); + + console.log('Add notification', { + conversationId, + messageSentAt, + isExpiringMessage, + }); Whisper.Notifications.add({ - title: sender.getTitle(), - message: message.getNotificationText(), + conversationId, iconUrl, imageUrl: message.getImageUrl(), - conversationId, - messageId: message.id, + isExpiringMessage, + message: message.getNotificationText(), + messageId, + messageSentAt, + title: sender.getTitle(), }); }) ); diff --git a/js/modules/types/message.js b/js/modules/types/message.js index 1c5ddb89a..4c3a57d75 100644 --- a/js/modules/types/message.js +++ b/js/modules/types/message.js @@ -6,6 +6,7 @@ const SchemaVersion = require('./schema_version'); const { initializeAttachmentMetadata, } = require('../../../ts/types/message/initializeAttachmentMetadata'); +const MessageTS = require('../../../ts/types/Message'); const GROUP = 'group'; const PRIVATE = 'private'; @@ -334,3 +335,5 @@ exports.createAttachmentDataWriter = writeExistingAttachmentData => { return messageWithoutAttachmentData; }; }; + +exports.hasExpiration = MessageTS.hasExpiration; diff --git a/js/notifications.js b/js/notifications.js index 95029fe50..650730d87 100644 --- a/js/notifications.js +++ b/js/notifications.js @@ -1,7 +1,9 @@ +/* global Signal:false */ + (function() { 'use strict'; window.Whisper = window.Whisper || {}; - const { Settings } = window.Signal.Types; + const { Settings } = Signal.Types; var SETTINGS = { OFF: 'off', @@ -15,6 +17,8 @@ this.isEnabled = false; this.on('add', this.update); this.on('remove', this.onRemove); + + this.lastNotification = null; }, onClick: function(conversationId) { var conversation = ConversationController.get(conversationId); @@ -26,10 +30,12 @@ const isAudioNotificationEnabled = storage.get('audio-notification') || false; const isAudioNotificationSupported = Settings.isAudioNotificationSupported(); + const userSetting = this.getSetting(); const shouldPlayNotificationSound = isAudioNotificationSupported && isAudioNotificationEnabled; const numNotifications = this.length; console.log('Update notifications:', { + userSetting, isFocused, isEnabled, numNotifications, @@ -51,8 +57,7 @@ return; } - var setting = storage.get('notification-setting') || 'message'; - if (setting === SETTINGS.OFF) { + if (userSetting === SETTINGS.OFF) { return; } @@ -66,33 +71,49 @@ // distinguishing between zero (0) and other (non-zero), // e.g. Russian: // http://docs.translatehouse.org/projects/localization-guide/en/latest/l10n/pluralforms.html - var newMessageCount = [ - numNotifications, - numNotifications === 1 ? i18n('newMessage') : i18n('newMessages'), - ].join(' '); + var newMessageCountLabel = `${numNotifications} ${ + numNotifications === 1 ? i18n('newMessage') : i18n('newMessages') + }`; - var last = this.last(); - switch (this.getSetting()) { + const last = this.last(); + const lastJSON = last.toJSON(); + switch (userSetting) { case SETTINGS.COUNT: title = 'Signal'; - message = newMessageCount; + message = newMessageCountLabel; break; case SETTINGS.NAME: - title = newMessageCount; - message = 'Most recent from ' + last.get('title'); + const lastMessageTitle = last.get('title'); + title = newMessageCountLabel; iconUrl = last.get('iconUrl'); + if (numNotifications === 1) { + message = `${i18n('notificationFrom')} ${lastMessageTitle}`; + } else { + message = `${i18n( + 'notificationMostRecentFrom' + )} ${lastMessageTitle}`; + } break; case SETTINGS.MESSAGE: if (numNotifications === 1) { title = last.get('title'); + message = last.get('message'); } else { - title = newMessageCount; + title = newMessageCountLabel; + message = `${i18n('notificationMostRecent')} ${last.get( + 'message' + )}`; } - message = last.get('message'); iconUrl = last.get('iconUrl'); break; } + const shouldHideExpiringMessageBody = + lastJSON.isExpiringMessage && Signal.OS.isMacOS(); + if (shouldHideExpiringMessageBody) { + message = i18n('newMessage'); + } + if (window.config.polyfillNotifications) { window.nodeNotifier.notify({ title: title, @@ -103,30 +124,44 @@ last.get('conversationId'); }); } else { - var notification = new Notification(title, { + if (this.lastNotification) { + this.lastNotification.close(); + } + const notification = new Notification(title, { body: message, icon: iconUrl, tag: 'signal', silent: !shouldPlayNotificationSound, }); - notification.onclick = this.onClick.bind( this, last.get('conversationId') ); + this.lastNotification = notification; } - // We don't want to notify the user about these same messages again - this.clear(); + // We continue to build up more and more messages for our notifications until + // the user comes back to our app or closes the app. Then we'll clear everything + // out. The good news is that we'll have a maximum of 1 notification in the + // Notification area (something like '10 new messages') assuming that close() does + // its job. }, getSetting: function() { return storage.get('notification-setting') || SETTINGS.MESSAGE; }, onRemove: function() { - console.log('remove notification'); + console.log('Remove notification'); + if (this.length === 0) { + this.clear(); + } else { + this.update(); + } }, clear: function() { - console.log('remove all notifications'); + console.log('Remove all notifications'); + if (this.lastNotification) { + this.lastNotification.close(); + } this.reset([]); }, enable: function() { diff --git a/js/read_syncs.js b/js/read_syncs.js index 9f13125c4..5f123505a 100644 --- a/js/read_syncs.js +++ b/js/read_syncs.js @@ -25,6 +25,7 @@ ); }); if (message) { + Whisper.Notifications.remove(message); return message.markRead(receipt.get('read_at')).then( function() { this.notifyConversation(message); diff --git a/ts/types/Message.ts b/ts/types/Message.ts index 91c4e98b1..602827c72 100644 --- a/ts/types/Message.ts +++ b/ts/types/Message.ts @@ -16,6 +16,7 @@ export type IncomingMessage = Readonly< body?: string; decrypted_at?: number; errors?: Array; + expireTimer?: number; flags?: number; source?: string; sourceDevice?: number; @@ -81,3 +82,15 @@ type MessageSchemaVersion5 = Partial< hasFileAttachments: IndexablePresence; }> >; + +export const isUserMessage = (message: Message): message is UserMessage => + message.type === 'incoming' || message.type === 'outgoing'; + +export const hasExpiration = (message: Message): boolean => { + if (!isUserMessage(message)) { + return false; + } + + const { expireTimer } = message; + return typeof expireTimer === 'number' && expireTimer > 0; +};