From f4e336836f1c3910bd0dbac68eeb81eeaa47cb9c Mon Sep 17 00:00:00 2001 From: Evan Hahn <69474926+EvanHahn-Signal@users.noreply.github.com> Date: Mon, 15 Nov 2021 14:01:58 -0600 Subject: [PATCH] Add user badges to typing bubbles, refactor typing logic --- .../AnnouncementsOnlyGroupBanner.tsx | 1 - ts/components/ConversationList.stories.tsx | 6 +- ts/components/ConversationList.tsx | 2 +- .../conversation/Timeline.stories.tsx | 13 +- ts/components/conversation/Timeline.tsx | 24 ++-- .../conversation/TypingBubble.stories.tsx | 13 ++ ts/components/conversation/TypingBubble.tsx | 125 +++++++++--------- .../conversationList/ConversationListItem.tsx | 6 +- ts/models/conversations.ts | 17 +-- ts/state/ducks/conversations.ts | 12 +- ts/state/smart/Timeline.tsx | 2 +- ts/state/smart/TypingBubble.tsx | 15 ++- .../state/selectors/conversations_test.ts | 78 ++--------- 13 files changed, 125 insertions(+), 189 deletions(-) diff --git a/ts/components/AnnouncementsOnlyGroupBanner.tsx b/ts/components/AnnouncementsOnlyGroupBanner.tsx index 976f9b5d6..d042e35ee 100644 --- a/ts/components/AnnouncementsOnlyGroupBanner.tsx +++ b/ts/components/AnnouncementsOnlyGroupBanner.tsx @@ -41,7 +41,6 @@ export const AnnouncementsOnlyGroupBanner = ({ draftPreview="" lastMessage={undefined} lastUpdated={undefined} - typingContact={undefined} theme={theme} /> ))} diff --git a/ts/components/ConversationList.stories.tsx b/ts/components/ConversationList.stories.tsx index cf02dfdeb..07f3a55cb 100644 --- a/ts/components/ConversationList.stories.tsx +++ b/ts/components/ConversationList.stories.tsx @@ -18,6 +18,7 @@ import { getDefaultConversation } from '../test-both/helpers/getDefaultConversat import { setupI18n } from '../util/setupI18n'; import enMessages from '../../_locales/en/messages.json'; import { StorybookThemeContext } from '../../.storybook/StorybookThemeContext'; +import { UUID } from '../types/UUID'; const i18n = setupI18n('en', enMessages); @@ -295,10 +296,7 @@ story.add('Contact checkboxes: disabled', () => ( story.add('Conversation: Typing Status', () => renderConversation({ - typingContact: { - ...getDefaultConversation(), - name: 'Someone Here', - }, + typingContactId: UUID.generate().toString(), }) ); diff --git a/ts/components/ConversationList.tsx b/ts/components/ConversationList.tsx index 7a48bc751..a4ac7cbcb 100644 --- a/ts/components/ConversationList.tsx +++ b/ts/components/ConversationList.tsx @@ -269,7 +269,7 @@ export const ConversationList: React.FC = ({ 'shouldShowDraft', 'title', 'type', - 'typingContact', + 'typingContactId', 'unblurredAvatarPath', 'unreadCount', ]); diff --git a/ts/components/conversation/Timeline.stories.tsx b/ts/components/conversation/Timeline.stories.tsx index 3aff52acd..bbb5bd995 100644 --- a/ts/components/conversation/Timeline.stories.tsx +++ b/ts/components/conversation/Timeline.stories.tsx @@ -3,7 +3,7 @@ import * as React from 'react'; import * as moment from 'moment'; -import { isBoolean, times } from 'lodash'; +import { times } from 'lodash'; import { v4 as uuid } from 'uuid'; import { storiesOf } from '@storybook/react'; import { text, boolean, number } from '@storybook/addon-knobs'; @@ -25,6 +25,8 @@ import { TypingBubble } from './TypingBubble'; import { ContactSpoofingType } from '../../util/contactSpoofing'; import { ReadStatus } from '../../messages/MessageReadStatus'; import type { WidthBreakpoint } from '../_util'; +import { ThemeType } from '../../types/Util'; +import { UUID } from '../../types/UUID'; const i18n = setupI18n('en', enMessages); @@ -441,12 +443,14 @@ const renderLoadingRow = () => ; const renderTypingBubble = () => ( ); @@ -486,10 +490,7 @@ const createProps = (overrideProps: Partial = {}): PropsType => ({ renderHeroRow, renderLoadingRow, renderTypingBubble, - typingContact: boolean( - 'typingContact', - isBoolean(overrideProps.typingContact) ? overrideProps.typingContact : false - ), + typingContactId: overrideProps.typingContactId, ...actions(), }); @@ -561,7 +562,7 @@ story.add('Target Index to Top', () => { story.add('Typing Indicator', () => { const props = createProps({ - typingContact: true, + typingContactId: UUID.generate().toString(), }); return ; diff --git a/ts/components/conversation/Timeline.tsx b/ts/components/conversation/Timeline.tsx index 2cb5be8fb..b021b0751 100644 --- a/ts/components/conversation/Timeline.tsx +++ b/ts/components/conversation/Timeline.tsx @@ -94,7 +94,7 @@ type PropsHousekeepingType = { areWeAdmin?: boolean; isGroupV1AndDisabled?: boolean; isIncomingMessageRequest: boolean; - typingContact?: unknown; + typingContactId?: string; unreadCount?: number; selectedMessageId?: string; @@ -859,7 +859,7 @@ export class Timeline extends React.PureComponent { } public getRowCount(): number { - const { oldestUnreadIndex, typingContact } = this.props; + const { oldestUnreadIndex, typingContactId } = this.props; const { items } = this.props; const itemsCount = items && items.length ? items.length : 0; @@ -870,7 +870,7 @@ export class Timeline extends React.PureComponent { extraRows += 1; } - if (typingContact) { + if (typingContactId) { extraRows += 1; } @@ -1033,7 +1033,7 @@ export class Timeline extends React.PureComponent { resetCounter, scrollToBottomCounter, scrollToIndex, - typingContact, + typingContactId, } = this.props; // We recompute the hero row's height if: @@ -1097,7 +1097,7 @@ export class Timeline extends React.PureComponent { if ( items !== prevProps.items || oldestUnreadIndex !== prevProps.oldestUnreadIndex || - Boolean(typingContact) !== Boolean(prevProps.typingContact) + Boolean(typingContactId) !== Boolean(prevProps.typingContactId) ) { const { atTop } = this.state; @@ -1135,13 +1135,13 @@ export class Timeline extends React.PureComponent { const rowsIterator = Timeline.getEphemeralRows({ items, oldestUnreadIndex, - typingContact: Boolean(typingContact), + hasTypingContact: Boolean(typingContactId), haveOldest, }); const prevRowsIterator = Timeline.getEphemeralRows({ items: prevProps.items, oldestUnreadIndex: prevProps.oldestUnreadIndex, - typingContact: Boolean(prevProps.typingContact), + hasTypingContact: Boolean(prevProps.typingContactId), haveOldest: prevProps.haveOldest, }); @@ -1578,13 +1578,13 @@ export class Timeline extends React.PureComponent { } private static *getEphemeralRows({ - items, - typingContact, - oldestUnreadIndex, + hasTypingContact, haveOldest, + items, + oldestUnreadIndex, }: { items: ReadonlyArray; - typingContact: boolean; + hasTypingContact: boolean; oldestUnreadIndex?: number; haveOldest: boolean; }): Iterator { @@ -1597,7 +1597,7 @@ export class Timeline extends React.PureComponent { yield `item:${items[i]}`; } - if (typingContact) { + if (hasTypingContact) { yield 'typing-contact'; } } diff --git a/ts/components/conversation/TypingBubble.stories.tsx b/ts/components/conversation/TypingBubble.stories.tsx index e89ec8fb5..887da8ec0 100644 --- a/ts/components/conversation/TypingBubble.stories.tsx +++ b/ts/components/conversation/TypingBubble.stories.tsx @@ -10,6 +10,8 @@ import enMessages from '../../../_locales/en/messages.json'; import type { Props } from './TypingBubble'; import { TypingBubble } from './TypingBubble'; import { AvatarColors } from '../../types/Colors'; +import { getFakeBadge } from '../../test-both/helpers/getFakeBadge'; +import { ThemeType } from '../../types/Util'; const i18n = setupI18n('en', enMessages); @@ -17,6 +19,7 @@ const story = storiesOf('Components/Conversation/TypingBubble', module); const createProps = (overrideProps: Partial = {}): Props => ({ acceptedMessageRequest: true, + badge: overrideProps.badge, isMe: false, i18n, color: select( @@ -33,6 +36,7 @@ const createProps = (overrideProps: Partial = {}): Props => ({ overrideProps.conversationType || 'direct' ), sharedGroupNames: [], + theme: ThemeType.light, }); story.add('Direct', () => { @@ -46,3 +50,12 @@ story.add('Group', () => { return ; }); + +story.add('Group (with badge)', () => { + const props = createProps({ + badge: getFakeBadge(), + conversationType: 'group', + }); + + return ; +}); diff --git a/ts/components/conversation/TypingBubble.tsx b/ts/components/conversation/TypingBubble.tsx index fd28e0032..ed15fa82f 100644 --- a/ts/components/conversation/TypingBubble.tsx +++ b/ts/components/conversation/TypingBubble.tsx @@ -1,14 +1,16 @@ // Copyright 2018-2021 Signal Messenger, LLC // SPDX-License-Identifier: AGPL-3.0-only +import type { ReactElement } from 'react'; import React from 'react'; import classNames from 'classnames'; import { TypingAnimation } from './TypingAnimation'; import { Avatar } from '../Avatar'; -import type { LocalizerType } from '../../types/Util'; +import type { LocalizerType, ThemeType } from '../../types/Util'; import type { ConversationType } from '../../state/ducks/conversations'; +import type { BadgeType } from '../../badges/types'; export type Props = Pick< ConversationType, @@ -22,76 +24,69 @@ export type Props = Pick< | 'sharedGroupNames' | 'title' > & { + badge: undefined | BadgeType; conversationType: 'group' | 'direct'; i18n: LocalizerType; + theme: ThemeType; }; -export class TypingBubble extends React.PureComponent { - public renderAvatar(): JSX.Element | null { - const { - acceptedMessageRequest, - avatarPath, - color, - conversationType, - i18n, - isMe, - name, - phoneNumber, - profileName, - sharedGroupNames, - title, - } = this.props; +export function TypingBubble({ + acceptedMessageRequest, + avatarPath, + badge, + color, + conversationType, + i18n, + isMe, + name, + phoneNumber, + profileName, + sharedGroupNames, + theme, + title, +}: Props): ReactElement { + const isGroup = conversationType === 'group'; - if (conversationType !== 'group') { - return null; - } - - return ( -
- -
- ); - } - - public override render(): JSX.Element { - const { i18n, conversationType } = this.props; - const isGroup = conversationType === 'group'; - - return ( -
- {this.renderAvatar()} -
-
-
- -
+ return ( +
+ {isGroup && ( +
+ +
+ )} +
+
+
+
- ); - } +
+ ); } diff --git a/ts/components/conversationList/ConversationListItem.tsx b/ts/components/conversationList/ConversationListItem.tsx index 135e9bb7c..464d63e29 100644 --- a/ts/components/conversationList/ConversationListItem.tsx +++ b/ts/components/conversationList/ConversationListItem.tsx @@ -55,7 +55,7 @@ export type PropsData = Pick< | 'shouldShowDraft' | 'title' | 'type' - | 'typingContact' + | 'typingContactId' | 'unblurredAvatarPath' | 'unreadCount' > & { @@ -94,7 +94,7 @@ export const ConversationListItem: FunctionComponent = React.memo( theme, title, type, - typingContact, + typingContactId, unblurredAvatarPath, unreadCount, }) { @@ -121,7 +121,7 @@ export const ConversationListItem: FunctionComponent = React.memo( {i18n('ConversationListItem--message-request')} ); - } else if (typingContact) { + } else if (typingContactId) { messageText = ; } else if (shouldShowDraft && draftPreview) { messageText = ( diff --git a/ts/models/conversations.ts b/ts/models/conversations.ts index 0ef05e7fb..48df5166c 100644 --- a/ts/models/conversations.ts +++ b/ts/models/conversations.ts @@ -1400,9 +1400,6 @@ export class ConversationModel extends window.Backbone const typingMostRecent = window._.first( window._.sortBy(typingValues, 'timestamp') ); - const typingContact = typingMostRecent - ? window.ConversationController.get(typingMostRecent.senderId) - : null; // eslint-disable-next-line @typescript-eslint/no-non-null-assertion const timestamp = this.get('timestamp')!; @@ -1440,7 +1437,7 @@ export class ConversationModel extends window.Backbone // TODO: DESKTOP-720 /* eslint-disable @typescript-eslint/no-non-null-assertion */ - const result: ConversationType = { + return { id: this.id, uuid: this.get('uuid'), e164: this.get('e164'), @@ -1521,6 +1518,7 @@ export class ConversationModel extends window.Backbone sortedGroupMembers, timestamp, title: this.getTitle()!, + typingContactId: typingMostRecent?.senderId, searchableTitle: isMe(this.attributes) ? window.i18n('noteToSelf') : this.getTitle(), @@ -1537,18 +1535,7 @@ export class ConversationModel extends window.Backbone sharedGroupNames: [], }), }; - - if (typingContact) { - // We don't want to call .format() on our own conversation - if (typingContact.id === this.id) { - result.typingContact = result; - } else { - result.typingContact = typingContact.format(); - } - } /* eslint-enable @typescript-eslint/no-non-null-assertion */ - - return result; } updateE164(e164?: string | null): void { diff --git a/ts/state/ducks/conversations.ts b/ts/state/ducks/conversations.ts index 8afbc16c5..fad8fa11c 100644 --- a/ts/state/ducks/conversations.ts +++ b/ts/state/ducks/conversations.ts @@ -181,17 +181,7 @@ export type ConversationType = { unreadCount?: number; isSelected?: boolean; isFetchingUUID?: boolean; - typingContact?: { - acceptedMessageRequest: boolean; - avatarPath?: string; - color?: AvatarColorType; - isMe: boolean; - name?: string; - phoneNumber?: string; - profileName?: string; - sharedGroupNames: Array; - title: string; - } | null; + typingContactId?: string; recentMediaItems?: Array; profileSharing?: boolean; diff --git a/ts/state/smart/Timeline.tsx b/ts/state/smart/Timeline.tsx index d0b5464c6..7e2978e36 100644 --- a/ts/state/smart/Timeline.tsx +++ b/ts/state/smart/Timeline.tsx @@ -298,7 +298,7 @@ const mapStateToProps = (state: StateType, props: ExternalProps) => { ...pick(conversation, [ 'areWeAdmin', 'unreadCount', - 'typingContact', + 'typingContactId', 'isGroupV1AndDisabled', ]), isIncomingMessageRequest: Boolean( diff --git a/ts/state/smart/TypingBubble.tsx b/ts/state/smart/TypingBubble.tsx index 5ac2d0f70..bc5d6ad3a 100644 --- a/ts/state/smart/TypingBubble.tsx +++ b/ts/state/smart/TypingBubble.tsx @@ -1,4 +1,4 @@ -// Copyright 2019-2020 Signal Messenger, LLC +// Copyright 2019-2021 Signal Messenger, LLC // SPDX-License-Identifier: AGPL-3.0-only import { connect } from 'react-redux'; @@ -7,8 +7,9 @@ import { TypingBubble } from '../../components/conversation/TypingBubble'; import { strictAssert } from '../../util/assert'; import type { StateType } from '../reducer'; -import { getIntl } from '../selectors/user'; +import { getIntl, getTheme } from '../selectors/user'; import { getConversationSelector } from '../selectors/conversations'; +import { getPreferredBadgeSelector } from '../selectors/badges'; type ExternalProps = { id: string; @@ -17,17 +18,21 @@ type ExternalProps = { const mapStateToProps = (state: StateType, props: ExternalProps) => { const { id } = props; - const conversation = getConversationSelector(state)(id); + const conversationSelector = getConversationSelector(state); + const conversation = conversationSelector(id); if (!conversation) { throw new Error(`Did not find conversation ${id} in state!`); } - strictAssert(conversation.typingContact, 'Missing typingContact'); + strictAssert(conversation.typingContactId, 'Missing typing contact ID'); + const typingContact = conversationSelector(conversation.typingContactId); return { - ...conversation.typingContact, + ...typingContact, + badge: getPreferredBadgeSelector(state)(typingContact.badges), conversationType: conversation.type, i18n: getIntl(state), + theme: getTheme(state), }; }; diff --git a/ts/test-both/state/selectors/conversations_test.ts b/ts/test-both/state/selectors/conversations_test.ts index 7162b3372..4471fb9b8 100644 --- a/ts/test-both/state/selectors/conversations_test.ts +++ b/ts/test-both/state/selectors/conversations_test.ts @@ -1254,11 +1254,7 @@ describe('both/state/selectors/conversations', () => { title: 'No timestamp', unreadCount: 1, isSelected: false, - typingContact: { - ...getDefaultConversation(), - name: 'Someone There', - phoneNumber: '+18005551111', - }, + typingContactId: UUID.generate().toString(), acceptedMessageRequest: true, }), @@ -1279,11 +1275,7 @@ describe('both/state/selectors/conversations', () => { title: 'B', unreadCount: 1, isSelected: false, - typingContact: { - ...getDefaultConversation(), - name: 'Someone There', - phoneNumber: '+18005551111', - }, + typingContactId: UUID.generate().toString(), acceptedMessageRequest: true, }), @@ -1304,11 +1296,7 @@ describe('both/state/selectors/conversations', () => { title: 'C', unreadCount: 1, isSelected: false, - typingContact: { - ...getDefaultConversation(), - name: 'Someone There', - phoneNumber: '+18005551111', - }, + typingContactId: UUID.generate().toString(), acceptedMessageRequest: true, }), @@ -1329,11 +1317,7 @@ describe('both/state/selectors/conversations', () => { title: 'A', unreadCount: 1, isSelected: false, - typingContact: { - ...getDefaultConversation(), - name: 'Someone There', - phoneNumber: '+18005551111', - }, + typingContactId: UUID.generate().toString(), acceptedMessageRequest: true, }), @@ -1354,11 +1338,7 @@ describe('both/state/selectors/conversations', () => { title: 'First!', unreadCount: 1, isSelected: false, - typingContact: { - ...getDefaultConversation(), - name: 'Someone There', - phoneNumber: '+18005551111', - }, + typingContactId: UUID.generate().toString(), acceptedMessageRequest: true, }), @@ -1400,11 +1380,7 @@ describe('both/state/selectors/conversations', () => { title: 'Pin Two', unreadCount: 1, isSelected: false, - typingContact: { - ...getDefaultConversation(), - name: 'Someone There', - phoneNumber: '+18005551111', - }, + typingContactId: UUID.generate().toString(), acceptedMessageRequest: true, }), @@ -1426,11 +1402,7 @@ describe('both/state/selectors/conversations', () => { title: 'Pin Three', unreadCount: 1, isSelected: false, - typingContact: { - ...getDefaultConversation(), - name: 'Someone There', - phoneNumber: '+18005551111', - }, + typingContactId: UUID.generate().toString(), acceptedMessageRequest: true, }), @@ -1452,11 +1424,7 @@ describe('both/state/selectors/conversations', () => { title: 'Pin One', unreadCount: 1, isSelected: false, - typingContact: { - ...getDefaultConversation(), - name: 'Someone There', - phoneNumber: '+18005551111', - }, + typingContactId: UUID.generate().toString(), acceptedMessageRequest: true, }), @@ -1495,11 +1463,7 @@ describe('both/state/selectors/conversations', () => { title: 'Pin Two', unreadCount: 1, isSelected: false, - typingContact: { - ...getDefaultConversation(), - name: 'Someone There', - phoneNumber: '+18005551111', - }, + typingContactId: UUID.generate().toString(), acceptedMessageRequest: true, }), @@ -1520,11 +1484,7 @@ describe('both/state/selectors/conversations', () => { title: 'Pin Three', unreadCount: 1, isSelected: false, - typingContact: { - ...getDefaultConversation(), - name: 'Someone There', - phoneNumber: '+18005551111', - }, + typingContactId: UUID.generate().toString(), acceptedMessageRequest: true, }), @@ -1545,11 +1505,7 @@ describe('both/state/selectors/conversations', () => { title: 'Pin One', unreadCount: 1, isSelected: false, - typingContact: { - ...getDefaultConversation(), - name: 'Someone There', - phoneNumber: '+18005551111', - }, + typingContactId: UUID.generate().toString(), acceptedMessageRequest: true, }), @@ -1571,11 +1527,7 @@ describe('both/state/selectors/conversations', () => { title: 'Pin One', unreadCount: 1, isSelected: false, - typingContact: { - ...getDefaultConversation(), - name: 'Someone There', - phoneNumber: '+18005551111', - }, + typingContactId: UUID.generate().toString(), acceptedMessageRequest: true, }), @@ -1596,11 +1548,7 @@ describe('both/state/selectors/conversations', () => { title: 'Pin One', unreadCount: 1, isSelected: false, - typingContact: { - ...getDefaultConversation(), - name: 'Someone There', - phoneNumber: '+18005551111', - }, + typingContactId: UUID.generate().toString(), acceptedMessageRequest: true, }),