From fa2d3007149a70e8df74ee8208f69d766901ffc6 Mon Sep 17 00:00:00 2001 From: Scott Nonnenberg Date: Mon, 26 Oct 2020 07:39:45 -0700 Subject: [PATCH] Show 'accept invite UI' for re-invite, calm progress spinner --- ts/ConversationController.ts | 6 ------ ts/background.ts | 12 ++++++++---- ts/components/CompositionArea.tsx | 7 ++++++- ts/models/conversations.ts | 31 ++++++++++--------------------- ts/state/ducks/conversations.ts | 1 + ts/util/index.ts | 2 ++ ts/util/lint/exceptions.json | 4 ++-- ts/views/conversation_view.ts | 12 +++++++++++- 8 files changed, 40 insertions(+), 35 deletions(-) diff --git a/ts/ConversationController.ts b/ts/ConversationController.ts index 70c3ddef29..117c7377d9 100644 --- a/ts/ConversationController.ts +++ b/ts/ConversationController.ts @@ -692,12 +692,6 @@ export class ConversationController { await Promise.all( this._conversations.map(async conversation => { try { - // This call is important to allow Conversation models not to generate their - // cached props on initial construction if we're in the middle of the load - // from the database. Then we come back to the models when it is safe and - // generate those props. - conversation.generateProps(); - if (!conversation.get('lastMessage')) { await conversation.updateLastMessage(); } diff --git a/ts/background.ts b/ts/background.ts index d0e0834dca..0796e5b903 100644 --- a/ts/background.ts +++ b/ts/background.ts @@ -749,12 +749,16 @@ type WhatIsThis = typeof window.WhatIsThis; conversationRemoved(id); }); convoCollection.on('add', conversation => { - const { id, cachedProps } = conversation || {}; - conversationAdded(id, cachedProps); + if (!conversation) { + return; + } + conversationAdded(conversation.id, conversation.format()); }); convoCollection.on('change', conversation => { - const { id, cachedProps } = conversation || {}; - conversationChanged(id, cachedProps); + if (!conversation) { + return; + } + conversationChanged(conversation.id, conversation.format()); }); convoCollection.on('reset', removeAllConversations); diff --git a/ts/components/CompositionArea.tsx b/ts/components/CompositionArea.tsx index d4a9a16713..1fc29b8943 100644 --- a/ts/components/CompositionArea.tsx +++ b/ts/components/CompositionArea.tsx @@ -23,6 +23,7 @@ import { EmojiPickDataType } from './emoji/EmojiPicker'; export type OwnProps = { readonly i18n: LocalizerType; + readonly areWePending?: boolean; readonly groupVersion?: 1 | 2; readonly isMissingMandatoryProfileSharing?: boolean; readonly messageRequestsEnabled?: boolean; @@ -115,6 +116,7 @@ export const CompositionArea = ({ clearShowPickerHint, // Message Requests acceptedMessageRequest, + areWePending, conversationType, groupVersion, isBlocked, @@ -331,7 +333,10 @@ export const CompositionArea = ({ }; }, [setLarge]); - if (messageRequestsEnabled && (!acceptedMessageRequest || isBlocked)) { + if ( + messageRequestsEnabled && + (!acceptedMessageRequest || isBlocked || areWePending) + ) { return ( void; - // backbone ensures this exists in initialize() - // eslint-disable-next-line @typescript-eslint/ban-ts-comment - // @ts-ignore - generateProps: () => void; - // backbone ensures this exists // eslint-disable-next-line @typescript-eslint/ban-ts-comment // @ts-ignore @@ -237,19 +232,11 @@ export class ConversationModel extends window.Backbone.Model< this.typingRefreshTimer = null; this.typingPauseTimer = null; - // Keep props ready - this.generateProps = () => { - // This is to prevent race conditions on startup; Conversation models are created - // but the full window.ConversationController.load() sequence isn't complete. - if (!window.ConversationController.isFetchComplete()) { - return; - } - - this.cachedProps = this.getProps(); - }; - this.on('change', this.generateProps); - - this.generateProps(); + // We clear our cached props whenever we change so that the next call to format() will + // result in refresh via a getProps() call. See format() below. + this.on('change', () => { + this.cachedProps = null; + }); } isMe(): boolean { @@ -282,9 +269,7 @@ export class ConversationModel extends window.Backbone.Model< isMemberPending(conversationId: string): boolean { if (!this.isGroupV2()) { - throw new Error( - `isPendingMember: Called for non-GroupV2 conversation ${this.idForLogging()}` - ); + return false; } const pendingMembersV2 = this.get('pendingMembersV2'); @@ -1064,6 +1049,7 @@ export class ConversationModel extends window.Backbone.Model< const messageRequestsEnabled = window.Signal.RemoteConfig.isEnabled( 'desktop.messageRequests' ); + const ourConversationId = window.ConversationController.getOurConversationId(); let groupVersion: undefined | 1 | 2; if (this.isGroupV1()) { @@ -1081,6 +1067,9 @@ export class ConversationModel extends window.Backbone.Model< acceptedMessageRequest: this.getAccepted(), activeAt: this.get('active_at')!, + areWePending: Boolean( + ourConversationId && this.isMemberPending(ourConversationId) + ), avatarPath: this.getAvatarPath()!, color, draftPreview, diff --git a/ts/state/ducks/conversations.ts b/ts/state/ducks/conversations.ts index fafd493cf7..23e50c58da 100644 --- a/ts/state/ducks/conversations.ts +++ b/ts/state/ducks/conversations.ts @@ -42,6 +42,7 @@ export type ConversationType = { firstName?: string; profileName?: string; avatarPath?: string; + areWePending?: boolean; color?: ColorType; isArchived?: boolean; isBlocked?: boolean; diff --git a/ts/util/index.ts b/ts/util/index.ts index 0020fe9b69..37f9f7962b 100644 --- a/ts/util/index.ts +++ b/ts/util/index.ts @@ -19,6 +19,7 @@ import { makeLookup } from './makeLookup'; import { migrateColor } from './migrateColor'; import { missingCaseError } from './missingCaseError'; import { parseRemoteClientExpiration } from './parseRemoteClientExpiration'; +import { sleep } from './sleep'; import * as zkgroup from './zkgroup'; export { @@ -41,5 +42,6 @@ export { missingCaseError, parseRemoteClientExpiration, Registration, + sleep, zkgroup, }; diff --git a/ts/util/lint/exceptions.json b/ts/util/lint/exceptions.json index 4fb52ec0c9..b7f34b44d6 100644 --- a/ts/util/lint/exceptions.json +++ b/ts/util/lint/exceptions.json @@ -13129,7 +13129,7 @@ "rule": "DOM-innerHTML", "path": "ts/components/CompositionArea.tsx", "line": " el.innerHTML = '';", - "lineNumber": 81, + "lineNumber": 82, "reasonCategory": "usageTrusted", "updated": "2020-06-03T19:23:21.195Z", "reasonDetail": "Our code, no user input, only clearing out the dom" @@ -13548,4 +13548,4 @@ "reasonCategory": "falseMatch", "updated": "2020-09-08T23:07:22.682Z" } -] +] \ No newline at end of file diff --git a/ts/views/conversation_view.ts b/ts/views/conversation_view.ts index 77ac8cdf3f..f33ce73dc1 100644 --- a/ts/views/conversation_view.ts +++ b/ts/views/conversation_view.ts @@ -660,8 +660,10 @@ Whisper.ConversationView = Whisper.View.extend({ }): Promise { const idLog = `${name}/${this.model.idForLogging()}`; const ONE_SECOND = 1000; + const TWO_SECONDS = 2000; let progressView: any | undefined; + let spinnerStart; let progressTimeout: NodeJS.Timeout | undefined = setTimeout(() => { window.log.info(`longRunningTaskWrapper/${idLog}: Creating spinner`); @@ -671,7 +673,8 @@ Whisper.ConversationView = Whisper.View.extend({ className: 'progress-modal-wrapper', Component: window.Signal.Components.ProgressModal, }); - }, ONE_SECOND); + spinnerStart = Date.now(); + }, TWO_SECONDS); // Note: any task we put here needs to have its own safety valve; this function will // show a spinner until it's done @@ -687,6 +690,13 @@ Whisper.ConversationView = Whisper.View.extend({ progressTimeout = undefined; } if (progressView) { + const now = Date.now(); + if (spinnerStart && now - spinnerStart < ONE_SECOND) { + window.log.info( + `longRunningTaskWrapper/${idLog}: Spinner shown for less than second, showing for another second` + ); + await window.Signal.Util.sleep(ONE_SECOND); + } progressView.remove(); progressView = undefined; }