Message details: group by send status, including viewed state

_locales/en/messages.json

@@ -5647,6 +5647,30 @@
     "message": "Slower, more data",
     "description": "Description of high quality selector"
   },
+  "MessageDetailsHeader--Failed": {
+    "message": "Not sent",
+    "description": "In the message details screen, shown above contacts where the message failed to deliver"
+  },
+  "MessageDetailsHeader--Pending": {
+    "message": "Pending",
+    "description": "In the message details screen, shown above contacts where the message is still sending"
+  },
+  "MessageDetailsHeader--Sent": {
+    "message": "Sent to",
+    "description": "In the message details screen, shown above contacts where the message has been sent (but not delivered, read, or viewed)"
+  },
+  "MessageDetailsHeader--Delivered": {
+    "message": "Delivered to",
+    "description": "In the message details screen, shown above contacts who have received your message"
+  },
+  "MessageDetailsHeader--Read": {
+    "message": "Read by",
+    "description": "In the message details screen, shown above contacts who have read this message"
+  },
+  "MessageDetailsHeader--Viewed": {
+    "message": "Viewed by",
+    "description": "In the message details screen, shown above contacts who have viewed this message"
+  },
   "ProfileEditor--about": {
     "message": "About",
     "description": "Default text for about field"

stylesheets/_modules.scss

@@ -3142,184 +3142,6 @@ button.module-conversation-details__action-button {
   }
 }
 
-// Module: Message Detail
-
-.module-message-detail {
-  max-width: 650px;
-  margin-left: auto;
-  margin-right: auto;
-  padding: 20px;
-  outline: none;
-}
-
-.module-message-detail__message-container {
-  padding-top: 20px;
-  padding-bottom: 20px;
-
-  &::after {
-    content: '.';
-    visibility: hidden;
-    display: block;
-    height: 0;
-    clear: both;
-  }
-}
-
-.module-message-detail__label {
-  @include font-body-1-bold;
-}
-
-.module-message-detail__unix-timestamp {
-  @include light-theme {
-    color: $color-gray-05;
-  }
-  @include dark-theme {
-    color: $color-gray-45;
-  }
-}
-
-.module-message-detail__contact-container {
-  margin: 20px;
-}
-
-.module-message-detail__contact {
-  margin-bottom: 8px;
-  display: flex;
-  flex-direction: row;
-  align-items: center;
-}
-
-.module-message-detail__contact__text {
-  margin-left: 10px;
-  flex-grow: 1;
-}
-
-.module-message-detail__contact__error {
-  color: $color-accent-red;
-  font-weight: bold;
-}
-
-.module-message-detail__contact__status-icon {
-  width: 12px;
-  height: 12px;
-  display: inline-block;
-  margin-bottom: 2px;
-}
-
-.module-message-detail__contact__status-icon--Pending {
-  animation: module-message-detail__contact__status-icon--spinning 4s linear
-    infinite;
-
-  @include light-theme {
-    @include color-svg('../images/sending.svg', $color-gray-60);
-  }
-  @include dark-theme {
-    @include color-svg('../images/sending.svg', $color-gray-25);
-  }
-}
-
-@keyframes module-message-detail__contact__status-icon--spinning {
-  100% {
-    -webkit-transform: rotate(360deg);
-    transform: rotate(360deg);
-  }
-}
-
-.module-message-detail__contact__status-icon--Sent {
-  @include light-theme {
-    @include color-svg('../images/check-circle-outline.svg', $color-gray-60);
-  }
-  @include dark-theme {
-    @include color-svg('../images/check-circle-outline.svg', $color-gray-25);
-  }
-}
-.module-message-detail__contact__status-icon--Delivered {
-  width: 18px;
-
-  @include light-theme {
-    @include color-svg('../images/double-check.svg', $color-gray-60);
-  }
-  @include dark-theme {
-    @include color-svg('../images/double-check.svg', $color-gray-25);
-  }
-}
-.module-message-detail__contact__status-icon--Read,
-.module-message-detail__contact__status-icon--Viewed {
-  width: 18px;
-
-  @include light-theme {
-    @include color-svg('../images/read.svg', $color-gray-60);
-  }
-  @include dark-theme {
-    @include color-svg('../images/read.svg', $color-gray-25);
-  }
-}
-.module-message-detail__contact__status-icon--Failed {
-  @include light-theme {
-    @include color-svg(
-      '../images/icons/v2/error-outline-12.svg',
-      $color-accent-red
-    );
-  }
-  @include dark-theme {
-    @include color-svg(
-      '../images/icons/v2/error-solid-12.svg',
-      $color-accent-red
-    );
-  }
-}
-
-.module-message-detail__contact__unidentified-delivery-icon {
-  margin-left: 6px;
-  margin-right: 10px;
-
-  width: 20px;
-  height: 20px;
-  display: inline-block;
-
-  @include light-theme {
-    @include color-svg(
-      '../images/icons/v2/unidentified-delivery-solid-20.svg',
-      $color-gray-60
-    );
-  }
-  @include dark-theme {
-    @include color-svg(
-      '../images/icons/v2/unidentified-delivery-solid-20.svg',
-      $color-gray-25
-    );
-  }
-}
-
-.module-message-detail__contact__error-buttons {
-  text-align: right;
-}
-
-.module-message-detail__contact__show-safety-number {
-  @include button-reset;
-  padding: 4px;
-  border-radius: 4px;
-
-  color: $color-white;
-
-  @include light-theme {
-    background-color: $color-gray-45;
-  }
-  @include dark-theme {
-    background-color: $color-gray-25;
-  }
-}
-.module-message-detail__contact__send-anyway {
-  @include button-reset;
-  margin-left: 5px;
-  margin-top: 5px;
-  padding: 4px;
-  border-radius: 4px;
-
-  color: $color-white;
-  background-color: $color-accent-red;
-}
-
 // Module: Media Gallery
 
 .module-media-gallery {

stylesheets/components/MessageDetail.scss

@@ -0,0 +1,206 @@
+// Copyright 2018-2021 Signal Messenger, LLC
+// SPDX-License-Identifier: AGPL-3.0-only
+
+.module-message-detail {
+  max-width: 650px;
+  margin-left: auto;
+  margin-right: auto;
+  padding: 20px;
+  outline: none;
+}
+
+.module-message-detail__message-container {
+  padding-top: 20px;
+  padding-bottom: 20px;
+
+  &::after {
+    content: '.';
+    visibility: hidden;
+    display: block;
+    height: 0;
+    clear: both;
+  }
+}
+
+.module-message-detail__label {
+  @include font-body-1-bold;
+}
+
+.module-message-detail__unix-timestamp {
+  @include light-theme {
+    color: $color-gray-05;
+  }
+  @include dark-theme {
+    color: $color-gray-45;
+  }
+}
+
+.module-message-detail__contact-container {
+  border-top: 1px solid $color-gray-15;
+  margin-top: 36px;
+
+  @include light-theme {
+    border-top-color: $color-gray-15;
+  }
+  @include dark-theme {
+    border-top-color: $color-gray-75;
+  }
+}
+
+.module-message-detail__contact-group__header {
+  @include font-body-1-bold;
+  align-items: center;
+  display: flex;
+  justify-content: space-between;
+  margin-top: 24px;
+  padding: 10px 0;
+  user-select: none;
+
+  &:first-child {
+    margin-top: 36px;
+  }
+
+  &--Failed,
+  &--Viewed,
+  &--Read,
+  &--Delivered,
+  &--Sent,
+  &--Pending {
+    &:after {
+      content: '';
+      display: block;
+      flex-shrink: 0;
+      height: 12px;
+      margin-left: 10px;
+    }
+  }
+
+  &--Failed:after {
+    width: 12px;
+    @include light-theme {
+      @include color-svg(
+        '../images/icons/v2/error-outline-12.svg',
+        $color-accent-red
+      );
+    }
+    @include dark-theme {
+      @include color-svg(
+        '../images/icons/v2/error-solid-12.svg',
+        $color-accent-red
+      );
+    }
+  }
+
+  @mixin normal-icon($icon) {
+    @include light-theme {
+      @include color-svg($icon, $color-gray-60);
+    }
+    @include dark-theme {
+      @include color-svg($icon, $color-gray-25);
+    }
+  }
+
+  &--Viewed:after,
+  &--Read:after {
+    // Viewed and read deliberately have the same icon.
+    width: 18px;
+    @include normal-icon('../images/read.svg');
+  }
+
+  &--Delivered:after {
+    width: 18px;
+    @include normal-icon('../images/double-check.svg');
+  }
+
+  &--Sent:after {
+    width: 12px;
+    @include normal-icon('../images/check-circle-outline.svg');
+  }
+
+  &--Pending:after {
+    width: 12px;
+    animation: module-message-detail__contact-group__header--Pending 4s linear
+      infinite;
+    @include normal-icon('../images/sending.svg');
+  }
+}
+
+@keyframes module-message-detail__contact-group__header--Pending {
+  100% {
+    -webkit-transform: rotate(360deg);
+    transform: rotate(360deg);
+  }
+}
+
+.module-message-detail__contact {
+  margin-bottom: 8px;
+  padding: 8px 0;
+  display: flex;
+  flex-direction: row;
+  align-items: center;
+
+  &:last-child {
+    margin-bottom: 0;
+  }
+}
+
+.module-message-detail__contact__text {
+  @include font-body-1;
+  flex-grow: 1;
+  margin-left: 10px;
+}
+
+.module-message-detail__contact__error {
+  color: $color-accent-red;
+  font-weight: bold;
+}
+
+.module-message-detail__contact__unidentified-delivery-icon {
+  margin-left: 6px;
+
+  width: 18px;
+  height: 18px;
+  display: inline-block;
+
+  @include light-theme {
+    @include color-svg(
+      '../images/icons/v2/unidentified-delivery-solid-20.svg',
+      $color-gray-60
+    );
+  }
+  @include dark-theme {
+    @include color-svg(
+      '../images/icons/v2/unidentified-delivery-solid-20.svg',
+      $color-gray-25
+    );
+  }
+}
+
+.module-message-detail__contact__error-buttons {
+  text-align: right;
+}
+
+.module-message-detail__contact__show-safety-number {
+  @include button-reset;
+  padding: 4px;
+  border-radius: 4px;
+
+  color: $color-white;
+
+  @include light-theme {
+    background-color: $color-gray-45;
+  }
+  @include dark-theme {
+    background-color: $color-gray-25;
+  }
+}
+.module-message-detail__contact__send-anyway {
+  @include button-reset;
+  margin-left: 5px;
+  margin-top: 5px;
+  padding: 4px;
+  border-radius: 4px;
+
+  color: $color-white;
+  background-color: $color-accent-red;
+}

stylesheets/manifest.scss

@@ -53,6 +53,7 @@
 @import './components/Input.scss';
 @import './components/MediaQualitySelector.scss';
 @import './components/MessageAudio.scss';
+@import './components/MessageDetail.scss';
 @import './components/Modal.scss';
 @import './components/ProfileEditor.scss';
 @import './components/SafetyNumberChangeDialog.scss';

ts/components/Avatar.tsx

@@ -28,6 +28,7 @@ export enum AvatarBlur {
 export enum AvatarSize {
   TWENTY_EIGHT = 28,
   THIRTY_TWO = 32,
+  THIRTY_SIX = 36,
   FIFTY_TWO = 52,
   EIGHTY = 80,
   NINETY_SIX = 96,

ts/components/conversation/MessageDetail.stories.tsx

@@ -92,7 +92,19 @@ const createProps = (overrideProps: Partial<Props> = {}): Props => ({
 });
 
 story.add('Delivered Incoming', () => {
-  const props = createProps({});
+  const props = createProps({
+    contacts: [
+      {
+        ...getDefaultConversation({
+          color: 'forest',
+          title: 'Max',
+        }),
+        status: undefined,
+        isOutgoingKeyError: false,
+        isUnidentifiedDelivery: false,
+      },
+    ],
+  });
   return <MessageDetail {...props} />;
 });
 

ts/components/conversation/MessageDetail.tsx

@@ -1,12 +1,12 @@
 // Copyright 2018-2021 Signal Messenger, LLC
 // SPDX-License-Identifier: AGPL-3.0-only
 
-import React from 'react';
+import React, { ReactChild, ReactNode } from 'react';
 import classNames from 'classnames';
 import moment from 'moment';
 import { noop } from 'lodash';
 
-import { Avatar } from '../Avatar';
+import { Avatar, AvatarSize } from '../Avatar';
 import { ContactName } from './ContactName';
 import {
   Message,
@@ -16,6 +16,7 @@ import {
 import { LocalizerType } from '../../types/Util';
 import { ConversationType } from '../../state/ducks/conversations';
 import { assert } from '../../util/assert';
+import { groupBy } from '../../util/mapUtil';
 import { ContactNameColorType } from '../../types/Colors';
 import { SendStatus } from '../../messages/MessageSendState';
 
@@ -33,7 +34,7 @@ export type Contact = Pick<
   | 'title'
   | 'unblurredAvatarPath'
 > & {
-  status: SendStatus | null;
+  status?: SendStatus;
 
   isOutgoingKeyError: boolean;
   isUnidentifiedDelivery: boolean;
@@ -42,7 +43,11 @@ export type Contact = Pick<
 };
 
 export type Props = {
-  contacts: Array<Contact>;
+  // An undefined status means they were the sender and it's an incoming message. If
+  //   `undefined` is a status, there should be no other items in the array; if there are
+  //   any defined statuses, `undefined` shouldn't be present.
+  contacts: ReadonlyArray<Contact>;
+
   contactNameColor?: ContactNameColorType;
   errors: Array<Error>;
   message: Omit<MessagePropsDataType, 'renderingContext'>;
@@ -79,6 +84,8 @@ export type Props = {
   | 'showVisualAttachment'
 >;
 
+const contactSortCollator = new Intl.Collator();
+
 const _keyForError = (error: Error): string => {
   return `${error.name}-${error.message}`;
 };
@@ -124,7 +131,7 @@ export class MessageDetail extends React.Component<Props> {
         profileName={profileName}
         title={title}
         sharedGroupNames={sharedGroupNames}
-        size={52}
+        size={AvatarSize.THIRTY_SIX}
         unblurredAvatarPath={unblurredAvatarPath}
       />
     );
@@ -152,22 +159,12 @@ export class MessageDetail extends React.Component<Props> {
         </button>
       </div>
     ) : null;
-    const statusComponent = !contact.isOutgoingKeyError ? (
-      <div
-        className={classNames(
-          'module-message-detail__contact__status-icon',
-          contact.status
-            ? `module-message-detail__contact__status-icon--${contact.status}`
-            : undefined
-        )}
-      />
-    ) : null;
     const unidentifiedDeliveryComponent = contact.isUnidentifiedDelivery ? (
       <div className="module-message-detail__contact__unidentified-delivery-icon" />
     ) : null;
 
     return (
-      <div key={contact.phoneNumber} className="module-message-detail__contact">
+      <div key={contact.id} className="module-message-detail__contact">
         {this.renderAvatar(contact)}
         <div className="module-message-detail__contact__text">
           <div className="module-message-detail__contact__name">
@@ -190,21 +187,65 @@ export class MessageDetail extends React.Component<Props> {
         </div>
         {errorComponent}
         {unidentifiedDeliveryComponent}
-        {statusComponent}
       </div>
     );
   }
 
-  public renderContacts(): JSX.Element | null {
+  private renderContactGroup(
-    const { contacts } = this.props;
+    sendStatus: undefined | SendStatus,
-
+    contacts: undefined | ReadonlyArray<Contact>
+  ): ReactNode {
+    const { i18n } = this.props;
     if (!contacts || !contacts.length) {
       return null;
     }
 
+    const i18nKey =
+      sendStatus === undefined ? 'from' : `MessageDetailsHeader--${sendStatus}`;
+
+    const sortedContacts = [...contacts].sort((a, b) =>
+      contactSortCollator.compare(a.title, b.title)
+    );
+
+    return (
+      <div key={i18nKey} className="module-message-detail__contact-group">
+        <div
+          className={classNames(
+            'module-message-detail__contact-group__header',
+            sendStatus &&
+              `module-message-detail__contact-group__header--${sendStatus}`
+          )}
+        >
+          {i18n(i18nKey)}
+        </div>
+        {sortedContacts.map(contact => this.renderContact(contact))}
+      </div>
+    );
+  }
+
+  private renderContacts(): ReactChild {
+    // This assumes that the list either contains one sender (a status of `undefined`) or
+    //   1+ contacts with `SendStatus`es, but it doesn't check that assumption.
+    const { contacts } = this.props;
+
+    const contactsBySendStatus = groupBy(contacts, contact => contact.status);
+
     return (
       <div className="module-message-detail__contact-container">
-        {contacts.map(contact => this.renderContact(contact))}
+        {[
+          undefined,
+          SendStatus.Failed,
+          SendStatus.Viewed,
+          SendStatus.Read,
+          SendStatus.Delivered,
+          SendStatus.Sent,
+          SendStatus.Pending,
+        ].map(sendStatus =>
+          this.renderContactGroup(
+            sendStatus,
+            contactsBySendStatus.get(sendStatus)
+          )
+        )}
       </div>
     );
   }
@@ -331,11 +372,6 @@ export class MessageDetail extends React.Component<Props> {
                 </td>
               </tr>
             ) : null}
-            <tr>
-              <td className="module-message-detail__label">
-                {message.direction === 'incoming' ? i18n('from') : i18n('to')}
-              </td>
-            </tr>
           </tbody>
         </table>
         {this.renderContacts()}

ts/models/messages.ts

@@ -325,17 +325,18 @@ export class MessageModel extends window.Backbone.Model<MessageAttributesType> {
 
       return window.ConversationController.getConversationId(identifier);
     });
-    const finalContacts: Array<SmartMessageDetailContact> = conversationIds.map(
+
-      (id: string): SmartMessageDetailContact => {
+    const contacts: ReadonlyArray<SmartMessageDetailContact> = conversationIds.map(
-        const errorsForContact = errorsGroupedById[id];
+      id => {
+        const errorsForContact = getOwn(errorsGroupedById, id);
         const isOutgoingKeyError = Boolean(
-          _.find(errorsForContact, error => error.name === OUTGOING_KEY_ERROR)
+          errorsForContact?.some(error => error.name === OUTGOING_KEY_ERROR)
         );
         const isUnidentifiedDelivery =
           window.storage.get('unidentifiedDeliveryIndicators', false) &&
           this.isUnidentifiedDelivery(id, unidentifiedDeliveriesSet);
 
-        let status = getOwn(sendStateByConversationId, id)?.status || null;
+        let status = getOwn(sendStateByConversationId, id)?.status;
 
         // If a message was only sent to yourself (Note to Self or a lonely group), it
         //   is shown read.
@@ -352,27 +353,6 @@ export class MessageModel extends window.Backbone.Model<MessageAttributesType> {
         };
       }
     );
-    // The prefix created here ensures that contacts with errors are listed
-    //   first; otherwise it's alphabetical
-    const collator = new Intl.Collator();
-    const sortedContacts: Array<SmartMessageDetailContact> = finalContacts.sort(
-      (
-        left: SmartMessageDetailContact,
-        right: SmartMessageDetailContact
-      ): number => {
-        const leftErrors = Boolean(left.errors && left.errors.length);
-        const rightErrors = Boolean(right.errors && right.errors.length);
-
-        if (leftErrors && !rightErrors) {
-          return -1;
-        }
-        if (!leftErrors && rightErrors) {
-          return 1;
-        }
-
-        return collator.compare(left.title, right.title);
-      }
-    );
 
     return {
       sentAt: this.get('sent_at'),
@@ -394,7 +374,7 @@ export class MessageModel extends window.Backbone.Model<MessageAttributesType> {
         }
       ),
       errors,
-      contacts: sortedContacts,
+      contacts,
     };
   }
 

ts/state/smart/MessageDetail.tsx

@@ -4,13 +4,9 @@
 import { ComponentProps } from 'react';
 import { connect } from 'react-redux';
 
-import {
+import { MessageDetail } from '../../components/conversation/MessageDetail';
-  MessageDetail,
-  Contact,
-} from '../../components/conversation/MessageDetail';
-import { PropsData as MessagePropsDataType } from '../../components/conversation/Message';
-import { mapDispatchToProps } from '../actions';
 
+import { mapDispatchToProps } from '../actions';
 import { StateType } from '../reducer';
 import { getIntl, getInteractionMode } from '../selectors/user';
 import { renderAudioAttachment } from './renderAudioAttachment';
@@ -21,36 +17,34 @@ type MessageDetailProps = ComponentProps<typeof MessageDetail>;
 
 export { Contact } from '../../components/conversation/MessageDetail';
 
-export type OwnProps = {
+export type OwnProps = Pick<
-  contacts: Array<Contact>;
-  errors: Array<Error>;
-  message: Omit<MessagePropsDataType, 'renderingContext'>;
-  receivedAt: number;
-  sentAt: number;
-
-  sendAnyway: (contactId: string, messageId: string) => unknown;
-  showSafetyNumber: (contactId: string) => void;
-} & Pick<
   MessageDetailProps,
   | 'clearSelectedMessage'
   | 'checkForAccount'
+  | 'contacts'
   | 'deleteMessage'
   | 'deleteMessageForEveryone'
   | 'displayTapToViewMessage'
   | 'downloadAttachment'
   | 'doubleCheckMissingQuoteReference'
+  | 'errors'
   | 'kickOffAttachmentDownload'
   | 'markAttachmentAsCorrupted'
+  | 'message'
   | 'openConversation'
   | 'openLink'
   | 'reactToMessage'
+  | 'receivedAt'
   | 'replyToMessage'
   | 'retrySend'
+  | 'sendAnyway'
+  | 'sentAt'
   | 'showContactDetail'
   | 'showContactModal'
   | 'showExpiredIncomingTapToViewToast'
   | 'showExpiredOutgoingTapToViewToast'
   | 'showForwardMessageModal'
+  | 'showSafetyNumber'
   | 'showVisualAttachment'
 >;
 

ts/test-both/util/mapUtil_test.ts

@@ -0,0 +1,30 @@
+// Copyright 2021 Signal Messenger, LLC
+// SPDX-License-Identifier: AGPL-3.0-only
+
+import { assert } from 'chai';
+import * as sinon from 'sinon';
+
+import { groupBy } from '../../util/mapUtil';
+
+describe('map utilities', () => {
+  describe('groupBy', () => {
+    it('returns an empty map when passed an empty iterable', () => {
+      const fn = sinon.fake();
+
+      assert.isEmpty(groupBy([], fn));
+
+      sinon.assert.notCalled(fn);
+    });
+
+    it('groups the iterable', () => {
+      assert.deepEqual(
+        groupBy([2.3, 1.3, 2.9, 1.1, 3.4], Math.floor),
+        new Map([
+          [1, [1.3, 1.1]],
+          [2, [2.3, 2.9]],
+          [3, [3.4]],
+        ])
+      );
+    });
+  });
+});

ts/util/mapUtil.ts

@@ -0,0 +1,26 @@
+// Copyright 2021 Signal Messenger, LLC
+// SPDX-License-Identifier: AGPL-3.0-only
+
+import { reduce } from './iterables';
+
+/**
+ * Like Lodash's `groupBy`, but returns a `Map`.
+ */
+export const groupBy = <T, ResultT>(
+  iterable: Iterable<T>,
+  fn: (value: T) => ResultT
+): Map<ResultT, Array<T>> =>
+  reduce(
+    iterable,
+    (result: Map<ResultT, Array<T>>, value: T) => {
+      const key = fn(value);
+      const existingGroup = result.get(key);
+      if (existingGroup) {
+        existingGroup.push(value);
+      } else {
+        result.set(key, [value]);
+      }
+      return result;
+    },
+    new Map<ResultT, Array<T>>()
+  );