From 9b382de6da369b70126c8ec0db773e5823a35312 Mon Sep 17 00:00:00 2001 From: Mikunj Date: Thu, 17 Jan 2019 15:10:07 +1100 Subject: [PATCH 1/6] Added online indicator. Disable selection in contacts. --- js/models/conversations.js | 9 ++++++++ js/views/conversation_list_item_view.js | 1 + stylesheets/_modules.scss | 8 ++++--- ts/components/Avatar.tsx | 29 +++++++++++++++++++++++-- ts/components/ConversationListItem.tsx | 3 +++ 5 files changed, 45 insertions(+), 5 deletions(-) diff --git a/js/models/conversations.js b/js/models/conversations.js index f5e70fc22..198e75de8 100644 --- a/js/models/conversations.js +++ b/js/models/conversations.js @@ -77,6 +77,7 @@ unlockTimestamp: null, // Timestamp used for expiring friend requests. sessionResetStatus: SessionResetEnum.none, swarmNodes: new Set([]), + isOnline: false, }; }, @@ -252,6 +253,13 @@ ); }, + async setIsOnline(online) { + this.set({ isOnline: online }); + await window.Signal.Data.updateConversation(this.id, this.attributes, { + Conversation: Whisper.Conversation, + }); + }, + async cleanup() { await window.Signal.Types.Conversation.deleteExternalFiles( this.attributes, @@ -386,6 +394,7 @@ status: this.lastMessageStatus, text: this.lastMessage, }, + isOnline: this.get('isOnline'), onClick: () => this.trigger('select', this), }; diff --git a/js/views/conversation_list_item_view.js b/js/views/conversation_list_item_view.js index ff6eb7bf1..06459a19e 100644 --- a/js/views/conversation_list_item_view.js +++ b/js/views/conversation_list_item_view.js @@ -60,6 +60,7 @@ const props = this.model.getPropsForListItem(); delete props.lastMessage; delete props.lastUpdated; + delete props.isSelected; return props; }, diff --git a/stylesheets/_modules.scss b/stylesheets/_modules.scss index b1a9f4e69..92c0c4de7 100644 --- a/stylesheets/_modules.scss +++ b/stylesheets/_modules.scss @@ -1767,6 +1767,10 @@ .module-avatar { background-color: $color-dark-85; } + + .module-contact-name { + margin-right: 0px; + } } .module-conversation-list-item--has-unread { @@ -1822,12 +1826,10 @@ .module-conversation-list-item__content { flex-grow: 1; margin-left: 12px; - // parent - 48px (for avatar) - 16px (our right margin) - max-width: calc(100% - 64px); - display: flex; flex-direction: column; align-items: stretch; + overflow: hidden; } .module-conversation-list-item__header { diff --git a/ts/components/Avatar.tsx b/ts/components/Avatar.tsx index 9a921ac1d..7b34b123e 100644 --- a/ts/components/Avatar.tsx +++ b/ts/components/Avatar.tsx @@ -13,6 +13,7 @@ interface Props { phoneNumber?: string; profileName?: string; size: number; + borderColor?: string; } interface State { @@ -41,7 +42,14 @@ export class Avatar extends React.Component { } public renderImage() { - const { avatarPath, i18n, name, phoneNumber, profileName } = this.props; + const { + avatarPath, + i18n, + name, + phoneNumber, + profileName, + borderColor, + } = this.props; const { imageBroken } = this.state; const hasImage = avatarPath && !imageBroken; @@ -53,8 +61,16 @@ export class Avatar extends React.Component { !name && profileName ? ` ~${profileName}` : '' }`; + const borderStyle = borderColor + ? { + borderColor: borderColor, + borderStyle: 'solid', + } + : undefined; + return ( {i18n('contactAvatarAlt', { } public renderNoImage() { - const { conversationType, name, size } = this.props; + const { conversationType, name, size, borderColor } = this.props; const initials = getInitials(name); const isGroup = conversationType === 'group'; + const borderStyle = borderColor + ? { + borderColor: borderColor, + borderStyle: 'solid', + } + : undefined; + if (!isGroup && initials) { return (
{ 'module-avatar__label', `module-avatar__label--${size}` )} + style={borderStyle} > {initials}
@@ -88,6 +112,7 @@ export class Avatar extends React.Component { `module-avatar__icon--${conversationType}`, `module-avatar__icon--${size}` )} + style={borderStyle} /> ); } diff --git a/ts/components/ConversationListItem.tsx b/ts/components/ConversationListItem.tsx index a30359479..bacf476f0 100644 --- a/ts/components/ConversationListItem.tsx +++ b/ts/components/ConversationListItem.tsx @@ -28,6 +28,7 @@ interface Props { }; showFriendRequestIndicator?: boolean; isBlocked: boolean; + isOnline: boolean; i18n: Localizer; onClick?: () => void; @@ -43,6 +44,7 @@ export class ConversationListItem extends React.Component { name, phoneNumber, profileName, + isOnline, } = this.props; return ( @@ -56,6 +58,7 @@ export class ConversationListItem extends React.Component { phoneNumber={phoneNumber} profileName={profileName} size={48} + borderColor={isOnline ? '#1c8260' : '#3d3e44'} /> {this.renderUnread()} From f4e9bc655a9caac6da74173e5b83bedd772fed75 Mon Sep 17 00:00:00 2001 From: Mikunj Date: Fri, 1 Feb 2019 11:27:31 +1100 Subject: [PATCH 2/6] Hooked up LokiP2pAPI with online stuff. --- js/conversation_controller.js | 23 +++++++++++++++++++++-- js/models/conversations.js | 10 +++------- js/modules/data.js | 12 +++++++++--- js/modules/loki_p2p_api.js | 4 ++++ 4 files changed, 37 insertions(+), 12 deletions(-) diff --git a/js/conversation_controller.js b/js/conversation_controller.js index 8decd5151..dd123f2c2 100644 --- a/js/conversation_controller.js +++ b/js/conversation_controller.js @@ -1,4 +1,4 @@ -/* global _, Whisper, Backbone, storage */ +/* global _, Whisper, Backbone, storage, lokiP2pAPI */ /* eslint-disable more/no-then */ @@ -248,6 +248,13 @@ async load() { window.log.info('ConversationController: starting initial fetch'); + // We setup online and offline listeners here because we want + // to minimize the amount of listeners we have to avoid memory leaks + if (!this.p2pListenersSet) { + lokiP2pAPI.on('online', this._handleOnline.bind(this)); + lokiP2pAPI.on('offline', this._handleOffline.bind(this)); + } + if (conversations.length) { throw new Error('ConversationController: Already loaded!'); } @@ -263,12 +270,12 @@ this._initialFetchComplete = true; const promises = []; conversations.forEach(conversation => { + // TODO This needs to be synchronous (one after the other) promises.concat([ conversation.updateLastMessage(), conversation.updateProfile(), conversation.updateProfileAvatar(), conversation.resetPendingSend(), - conversation.updateProfile(), ]); }); await Promise.all(promises); @@ -292,5 +299,17 @@ return this._initialPromise; }, + _handleOnline(pubKey) { + try { + const conversation = this.get(pubKey); + conversation.set({ isOnline: true }); + } catch (e) {} // eslint-disable-line + }, + _handleOffline(pubKey) { + try { + const conversation = this.get(pubKey); + conversation.set({ isOnline: false }); + } catch (e) {} // eslint-disable-line + }, }; })(); diff --git a/js/models/conversations.js b/js/models/conversations.js index 198e75de8..fe291c98c 100644 --- a/js/models/conversations.js +++ b/js/models/conversations.js @@ -156,6 +156,9 @@ this.setFriendRequestExpiryTimeout(); this.typingRefreshTimer = null; this.typingPauseTimer = null; + + // Online status handling + this.set({ isOnline: lokiP2pAPI.isOnline(this.id) }); }, isMe() { @@ -253,13 +256,6 @@ ); }, - async setIsOnline(online) { - this.set({ isOnline: online }); - await window.Signal.Data.updateConversation(this.id, this.attributes, { - Conversation: Whisper.Conversation, - }); - }, - async cleanup() { await window.Signal.Types.Conversation.deleteExternalFiles( this.attributes, diff --git a/js/modules/data.js b/js/modules/data.js index d4e2fb58e..1a46ba568 100644 --- a/js/modules/data.js +++ b/js/modules/data.js @@ -11,6 +11,7 @@ const { map, merge, set, + omit, } = require('lodash'); const { base64ToArrayBuffer, arrayBufferToBase64 } = require('./crypto'); @@ -688,11 +689,13 @@ async function getConversationCount() { } async function saveConversation(data) { - await channels.saveConversation(data); + const cleaned = omit(data, 'isOnine'); + await channels.saveConversation(cleaned); } async function saveConversations(data) { - await channels.saveConversations(data); + const cleaned = data.map(d => omit(d, 'isOnline')); + await channels.saveConversations(cleaned); } async function getConversationById(id, { Conversation }) { @@ -712,7 +715,10 @@ async function updateConversation(id, data, { Conversation }) { if (merged.swarmNodes instanceof Set) { merged.swarmNodes = Array.from(merged.swarmNodes); } - await channels.updateConversation(merged); + + // Don't save the online status of the object + const cleaned = omit(merged, 'isOnline'); + await channels.updateConversation(cleaned); } async function removeConversation(id, { Conversation }) { diff --git a/js/modules/loki_p2p_api.js b/js/modules/loki_p2p_api.js index 598a7d0a4..ca8a13597 100644 --- a/js/modules/loki_p2p_api.js +++ b/js/modules/loki_p2p_api.js @@ -66,6 +66,10 @@ class LokiP2pAPI extends EventEmitter { ); } + isOnline(pubKey) { + return !!(this.contactP2pDetails[pubKey] && this.contactP2pDetails[pubKey].isOnline); + } + pingContact(pubKey) { if (!this.contactP2pDetails[pubKey]) { return; From 8526c6dd92ccf8fbcae0c0f384b1199b4fe3d3d6 Mon Sep 17 00:00:00 2001 From: Mikunj Date: Fri, 1 Feb 2019 11:30:44 +1100 Subject: [PATCH 3/6] Fix up possible database write issue. --- js/conversation_controller.js | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/js/conversation_controller.js b/js/conversation_controller.js index dd123f2c2..d52278fc2 100644 --- a/js/conversation_controller.js +++ b/js/conversation_controller.js @@ -270,13 +270,14 @@ this._initialFetchComplete = true; const promises = []; conversations.forEach(conversation => { - // TODO This needs to be synchronous (one after the other) - promises.concat([ - conversation.updateLastMessage(), - conversation.updateProfile(), - conversation.updateProfileAvatar(), - conversation.resetPendingSend(), - ]); + promises.push(async () => { + // We need to await each one as + // we don't want to simultaneously call writes on the sql database + await conversation.updateLastMessage(); + await conversation.updateProfile(); + await conversation.updateProfileAvatar(); + await conversation.resetPendingSend(); + }); }); await Promise.all(promises); From 4518e95619d48b8029ba4e010b636252c869b442 Mon Sep 17 00:00:00 2001 From: Mikunj Date: Fri, 1 Feb 2019 11:35:23 +1100 Subject: [PATCH 4/6] Linting --- js/models/conversations.js | 1 + js/modules/loki_p2p_api.js | 9 +++++++-- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/js/models/conversations.js b/js/models/conversations.js index fe291c98c..3191f3319 100644 --- a/js/models/conversations.js +++ b/js/models/conversations.js @@ -7,6 +7,7 @@ /* global storage: false */ /* global textsecure: false */ /* global Whisper: false */ +/* global lokiP2pAPI: false */ /* eslint-disable more/no-then */ diff --git a/js/modules/loki_p2p_api.js b/js/modules/loki_p2p_api.js index ca8a13597..5cbbbdacd 100644 --- a/js/modules/loki_p2p_api.js +++ b/js/modules/loki_p2p_api.js @@ -16,7 +16,10 @@ class LokiP2pAPI extends EventEmitter { ? 60 * 1000 // 1 minute : 2 * 60 * 1000; // 2 minutes - if (this.contactP2pDetails[pubKey] && this.contactP2pDetails[pubKey].pingTimer) { + if ( + this.contactP2pDetails[pubKey] && + this.contactP2pDetails[pubKey].pingTimer + ) { clearTimeout(this.contactP2pDetails[pubKey].pingTimer); } this.contactP2pDetails[pubKey] = { @@ -67,7 +70,9 @@ class LokiP2pAPI extends EventEmitter { } isOnline(pubKey) { - return !!(this.contactP2pDetails[pubKey] && this.contactP2pDetails[pubKey].isOnline); + return !!( + this.contactP2pDetails[pubKey] && this.contactP2pDetails[pubKey].isOnline + ); } pingContact(pubKey) { From 594a815069305570902d6f16ff29d62d1d255441 Mon Sep 17 00:00:00 2001 From: Mikunj Date: Fri, 1 Feb 2019 13:37:15 +1100 Subject: [PATCH 5/6] Fix test --- test/backup_test.js | 1 + 1 file changed, 1 insertion(+) diff --git a/test/backup_test.js b/test/backup_test.js index 806572839..8e832d41f 100644 --- a/test/backup_test.js +++ b/test/backup_test.js @@ -555,6 +555,7 @@ describe('Backup', () => { 'friendRequestStatus', 'unlockTimestamp', 'sessionResetStatus', + 'isOnline', ]; const conversationFromDB = conversationCollection.at(0).attributes; console.log({ conversationFromDB, conversation }); From 174f8747b62b1a75ac5e5597bc11c3cb82e6b850 Mon Sep 17 00:00:00 2001 From: Mikunj Date: Fri, 1 Feb 2019 14:44:46 +1100 Subject: [PATCH 6/6] Review fixes. Revert promises back to old style. --- js/conversation_controller.js | 15 +++++++-------- js/modules/data.js | 2 +- 2 files changed, 8 insertions(+), 9 deletions(-) diff --git a/js/conversation_controller.js b/js/conversation_controller.js index d52278fc2..7a15d689a 100644 --- a/js/conversation_controller.js +++ b/js/conversation_controller.js @@ -253,6 +253,7 @@ if (!this.p2pListenersSet) { lokiP2pAPI.on('online', this._handleOnline.bind(this)); lokiP2pAPI.on('offline', this._handleOffline.bind(this)); + this.p2pListenersSet = true; } if (conversations.length) { @@ -270,14 +271,12 @@ this._initialFetchComplete = true; const promises = []; conversations.forEach(conversation => { - promises.push(async () => { - // We need to await each one as - // we don't want to simultaneously call writes on the sql database - await conversation.updateLastMessage(); - await conversation.updateProfile(); - await conversation.updateProfileAvatar(); - await conversation.resetPendingSend(); - }); + promises.concat([ + conversation.updateLastMessage(), + conversation.updateProfile(), + conversation.updateProfileAvatar(), + conversation.resetPendingSend(), + ]); }); await Promise.all(promises); diff --git a/js/modules/data.js b/js/modules/data.js index 1a46ba568..e850c723d 100644 --- a/js/modules/data.js +++ b/js/modules/data.js @@ -689,7 +689,7 @@ async function getConversationCount() { } async function saveConversation(data) { - const cleaned = omit(data, 'isOnine'); + const cleaned = omit(data, 'isOnline'); await channels.saveConversation(cleaned); }