From 7b039fa526acb41de94630e3b296b33816e9d66b Mon Sep 17 00:00:00 2001 From: trevor-signal <131492920+trevor-signal@users.noreply.github.com> Date: Tue, 9 May 2023 11:33:39 -0400 Subject: [PATCH] Update contact spoofing banner & dialog --- _locales/en/messages.json | 12 +- .../ContactSpoofingReviewDialog.scss | 10 +- .../ContactSpoofingReviewDialogPerson.scss | 2 +- .../ContactSpoofingReviewDialog.tsx | 224 ++++++++++-------- ts/components/conversation/Timeline.tsx | 56 +++-- 5 files changed, 178 insertions(+), 126 deletions(-) diff --git a/_locales/en/messages.json b/_locales/en/messages.json index 14e73570bbeb..19da046d8524 100644 --- a/_locales/en/messages.json +++ b/_locales/en/messages.json @@ -9455,9 +9455,13 @@ "description": "(deleted 04/04/2023) Shown in the timeline warning when you multiple group members have the same name" }, "icu:ContactSpoofing__same-name-in-group--link": { - "messageformat": "{count, plural, one {# group member has} other {# group members have}} the same name. Review request", + "messageformat": "{count, plural, one {# group member has} other {# group members have}} the same name. Review members", "description": "Shown in the timeline warning when you multiple group members have the same name" }, + "icu:ContactSpoofing__same-names-in-group--link": { + "messageformat": "{count, plural, one {# name conflict was} other {# name conflicts were}} found in this group. Review members", + "description": "Shown in the timeline warning when multiple names are shared by members of a group." + }, "ContactSpoofing__same-name__link": { "message": "Review request", "description": "(deleted 03/29/2023) Shown in the timeline warning when you have a message request from someone with the same name as someone else" @@ -9472,7 +9476,7 @@ }, "icu:ContactSpoofing__same-name-in-group__link": { "messageformat": "Click to review", - "description": "Shown in the timeline warning when you multiple group members have the same name" + "description": "(deleted 05/04/2023) Shown in the timeline warning when you multiple group members have the same name" }, "ContactSpoofingReviewDialog__title": { "message": "Review request", @@ -9522,6 +9526,10 @@ "messageformat": "{count, plural, one {# group member} other {# group members}} have similar names. Review the members below or choose to take action.", "description": "Description for the group contact spoofing review dialog" }, + "icu:ContactSpoofingReviewDialog__group__multiple-conflicts__description": { + "messageformat": "{count, plural, one {# name conflict was} other {# name conflicts were}} found in this group. Review the members below or choose to take action.", + "description": "Description for the group contact spoofing review dialog when there are multiple shared names" + }, "ContactSpoofingReviewDialog__group__members-header": { "message": "Members", "description": "(deleted 03/29/2023) Header in the group contact spoofing review dialog. After this header, there will be a list of members" diff --git a/stylesheets/components/ContactSpoofingReviewDialog.scss b/stylesheets/components/ContactSpoofingReviewDialog.scss index 139026220fed..2f24f24db83a 100644 --- a/stylesheets/components/ContactSpoofingReviewDialog.scss +++ b/stylesheets/components/ContactSpoofingReviewDialog.scss @@ -16,14 +16,20 @@ } } + &__description { + margin-top: 4px; + } + h2 { @include font-body-1-bold; + margin-top: 28px; + margin-bottom: 20px; } hr { border: 0; height: 1px; - margin-block: 16px; + margin-block: 20px; margin-inline: 0; @include light-theme { @@ -35,7 +41,7 @@ } &__buttons { - margin-top: 8px; + margin-top: 12px; .module-Button:not(:last-child) { margin-inline-end: 12px; diff --git a/stylesheets/components/ContactSpoofingReviewDialogPerson.scss b/stylesheets/components/ContactSpoofingReviewDialogPerson.scss index 87a6164ea387..df486215220e 100644 --- a/stylesheets/components/ContactSpoofingReviewDialogPerson.scss +++ b/stylesheets/components/ContactSpoofingReviewDialogPerson.scss @@ -12,7 +12,7 @@ margin-inline-start: 12px; &__contact-name { - @include font-body-1; + @include font-body-1-bold; } &__property { diff --git a/ts/components/conversation/ContactSpoofingReviewDialog.tsx b/ts/components/conversation/ContactSpoofingReviewDialog.tsx index f262dfb64f21..c3e45d5f51b1 100644 --- a/ts/components/conversation/ContactSpoofingReviewDialog.tsx +++ b/ts/components/conversation/ContactSpoofingReviewDialog.tsx @@ -3,7 +3,6 @@ import type { ReactChild, ReactNode } from 'react'; import React, { useState } from 'react'; -import { concat, orderBy } from 'lodash'; import type { LocalizerType, ThemeType } from '../../types/Util'; import type { ConversationType } from '../../state/ducks/conversations'; @@ -235,115 +234,140 @@ export function ContactSpoofingReviewDialog(props: PropsType): JSX.Element { } case ContactSpoofingType.MultipleGroupMembersWithSameTitle: { const { group, collisionInfoByTitle } = props; - - const unsortedConversationInfos = concat( - // This empty array exists to appease Lodash's type definitions. - [], - ...Object.values(collisionInfoByTitle) + const sharedTitles = Object.keys(collisionInfoByTitle); + const numSharedTitles = sharedTitles.length; + const totalConversations = Object.values(collisionInfoByTitle).reduce( + (sum, conversationInfos) => sum + conversationInfos.length, + 0 ); - const conversationInfos = orderBy(unsortedConversationInfos, [ - // We normally use an `Intl.Collator` to sort by title. We do this instead, as - // we only really care about stability (not perfect ordering). - 'title', - 'id', - ]); title = i18n('icu:ContactSpoofingReviewDialog__group__title'); contents = ( <> -

- {i18n('icu:ContactSpoofingReviewDialog__group__description', { - count: conversationInfos.length, - })} +

+ {numSharedTitles > 1 + ? i18n( + 'icu:ContactSpoofingReviewDialog__group__multiple-conflicts__description', + { + count: numSharedTitles, + } + ) + : i18n('icu:ContactSpoofingReviewDialog__group__description', { + count: totalConversations, + })}

-

- {i18n('icu:ContactSpoofingReviewDialog__group__members-header')} -

- {conversationInfos.map((conversationInfo, index) => { - let button: ReactNode; - if (group.areWeAdmin) { - button = ( - - ); - } else if (conversationInfo.conversation.isBlocked) { - button = ( - - ); - } else if (!isInSystemContacts(conversationInfo.conversation)) { - button = ( - - ); - } - const { oldName } = conversationInfo; - const newName = - conversationInfo.conversation.profileName || - conversationInfo.conversation.title; + {Object.values(collisionInfoByTitle).map( + (conversationInfos, titleIdx) => { + return ( + <> +

+ {i18n( + 'icu:ContactSpoofingReviewDialog__group__members-header' + )} +

+ {conversationInfos.map( + (conversationInfo, conversationIdx) => { + let button: ReactNode; + if (group.areWeAdmin) { + button = ( + + ); + } else if (conversationInfo.conversation.isBlocked) { + button = ( + + ); + } else if ( + !isInSystemContacts(conversationInfo.conversation) + ) { + button = ( + + ); + } - let callout: JSX.Element | undefined; - if (oldName && oldName !== newName) { - callout = ( -
- , - newName: , - }} - /> -
- ); - } + const { oldName } = conversationInfo; + const newName = + conversationInfo.conversation.profileName || + conversationInfo.conversation.title; - return ( - <> - {index !== 0 &&
} - - {callout} - {button && ( -
- {button} -
+ let callout: JSX.Element | undefined; + if (oldName && oldName !== newName) { + callout = ( +
+ , + newName: , + }} + /> +
+ ); + } + + return ( + <> + + {callout} + {button && ( +
+ {button} +
+ )} + + {titleIdx < sharedTitles.length - 1 || + conversationIdx < conversationInfos.length - 1 ? ( +
+ ) : null} + + ); + } )} - - - ); - })} + + ); + } + )} ); break; diff --git a/ts/components/conversation/Timeline.tsx b/ts/components/conversation/Timeline.tsx index f9f98268c538..215394f09ca4 100644 --- a/ts/components/conversation/Timeline.tsx +++ b/ts/components/conversation/Timeline.tsx @@ -19,6 +19,7 @@ import { clearTimeoutIfNecessary } from '../../util/clearTimeoutIfNecessary'; import { WidthBreakpoint } from '../_util'; import { ErrorBoundary } from './ErrorBoundary'; +import type { FullJSXType } from '../Intl'; import { Intl } from '../Intl'; import { TimelineWarning } from './TimelineWarning'; import { TimelineWarnings } from './TimelineWarnings'; @@ -941,29 +942,42 @@ export class Timeline extends React.Component< break; case ContactSpoofingType.MultipleGroupMembersWithSameTitle: { const { groupNameCollisions } = warning; - text = ( - result + conversations.length, - 0 - ), - // This is a render props, not a component - // eslint-disable-next-line react/no-unstable-nested-components - reviewRequestLink: parts => ( - { - reviewGroupMemberNameCollision(id); - }} - > - {parts} - - ), + const numberOfSharedNames = Object.keys(groupNameCollisions).length; + const reviewRequestLink: FullJSXType = parts => ( + { + reviewGroupMemberNameCollision(id); }} - /> + > + {parts} + ); + if (numberOfSharedNames === 1) { + text = ( + result + conversations.length, + 0 + ), + reviewRequestLink, + }} + /> + ); + } else { + text = ( + + ); + } onClose = () => { acknowledgeGroupMemberNameCollisions(id, groupNameCollisions); };