Improvements to SafetyNumberChangeDialog

2022-11-01 17:10:27 -07:00 · 2022-11-01 17:10:27 -07:00 · 4fc1b6388c
commit 4fc1b6388c
parent 6700f6fa15
6 changed files with 329 additions and 114 deletions
--- a/_locales/en/messages.json
+++ b/_locales/en/messages.json
@ -433,22 +433,38 @@
  },
  "changedVerificationWarning": {
    "message": "The following people may have reinstalled or changed devices. Verify your safety number with them to ensure privacy.",
+    "description": "(deleted 2022/11/26) Shown on confirmation dialog when user attempts to send a message"
+  },
+  "safetyNumberChangeDialog__message": {
+    "message": "The following people may have reinstalled Signal or changed devices. Click a recipient to confirm the new safety number. This is optional.",
    "description": "Shown on confirmation dialog when user attempts to send a message"
  },
  "safetyNumberChangeDialog__pending-messages": {
    "message": "Send pending messages",
    "description": "Shown on confirmation dialog when user attempts to send a message in the outbox"
  },
+  "safetyNumberChangeDialog__review": {
+    "message": "Review",
+    "description": "Shown to enter 'review' mode if more than five contacts have changed safety numbers"
+  },
+  "icu:safetyNumberChangeDialog__many-contacts": {
+    "messageformat": "{count, plural, other {You have # connections who may have reinstalled Signal or changed devices. You can optionally review their safety numbers before sending.}}",
+    "description": "Shown during an attempted send when more than five contacts have changed their safety numbers"
+  },
  "identityKeyErrorOnSend": {
    "message": "Your safety number with $name1$ has changed. This could either mean that someone is trying to intercept your communication or that $name2$ has simply reinstalled Signal. You may wish to verify your safety number with this contact.",
    "description": "Shown when user clicks on a failed recipient in the message detail view after an identity key change"
  },
  "sendAnyway": {
-    "message": "Send Anyway",
+    "message": "Send anyway",
+    "description": "Used on a warning dialog to make it clear that it might be risky to send the message."
+  },
+  "safetyNumberChangeDialog_send": {
+    "message": "Send",
    "description": "Used on a warning dialog to make it clear that it might be risky to send the message."
  },
  "callAnyway": {
-    "message": "Call Anyway",
+    "message": "Call anyway",
    "description": "Used on a warning dialog to make it clear that it might be risky to call the conversation."
  },
  "continueCall": {
--- a/stylesheets/components/SafetyNumberChangeDialog.scss
+++ b/stylesheets/components/SafetyNumberChangeDialog.scss
@ -2,9 +2,61 @@
 // SPDX-License-Identifier: AGPL-3.0-only

 .module-SafetyNumberChangeDialog {
+  &__confirm-dialog__header {
+    padding-bottom: 0px;
+
+    // We've got no title, but we want the X button from ConfirmationDialog, so
+    //   we need to bump the dialog contents up into the header area just a bit.
+    margin-bottom: -5px;
+  }
+
+  // Used to ensure that a set of spans reverse order under RTL
+  &__rtl-span {
+    display: inline-block;
+  }
+
+  &__shield-icon {
+    margin-left: auto;
+    margin-right: auto;
+
+    height: 24px;
+    width: 24px;
+    @include light-theme {
+      @include color-svg(
+        '../images/icons/v2/safety-number-outline-24.svg',
+        $color-gray-90
+      );
+    }
+    @include dark-theme {
+      @include color-svg(
+        '../images/icons/v2/safety-number-outline-24.svg',
+        $color-white
+      );
+    }
+  }
+
+  &__title {
+    @include font-body-1-bold;
+
+    text-align: center;
+    margin-top: 8px;
+
+    @include light-theme {
+      color: $color-gray-90;
+    }
+    @include dark-theme {
+      color: $color-white;
+    }
+  }
+
  &__message {
    @include font-body-2;
+
    text-align: center;
+    margin-top: 8px;
+    margin-bottom: 24px;
+    padding-left: 4px;
+    padding-right: 4px;

    @include light-theme {
      color: $color-gray-60;
@ -18,11 +70,12 @@
  &__contacts {
    list-style-type: none;
    max-height: 300px;
-    overflow-y: scroll;
    padding: 0;
  }

  &__contact {
+    $contact: &;
+
    align-items: center;
    display: flex;
    flex-direction: row;
@ -34,14 +87,16 @@
    }

    &--name {
-      @include font-body-1-bold;
+      @include font-body-1;

      @include dark-theme {
        color: $color-white;
      }
    }

-    &--number {
+    &--subtitle {
+      @include font-subtitle;
+
      @include light-theme {
        color: $color-gray-60;
      }
@ -52,27 +107,22 @@
    }

    &--view {
-      @include font-body-1-bold;
-      background: inherit;
-      border: none;
-      cursor: pointer;
-      margin-right: 2px;
-      outline: none;
+      @include button-reset;
+      @include button-secondary-blue-text;
+
+      opacity: 0;
+      transition: opacity 150ms cubic-bezier(0.17, 0.17, 0, 1);
+
+      // Using keyboard/mouse classes directly; mixins were doing weird things
+      .mouse-mode #{$contact}:hover & {
+        opacity: 1;
+      }
+      .keyboard-mode #{$contact}:focus-within & {
+        opacity: 1;
+      }
+
+      border-radius: 4px;
      padding: 8px 14px;
-
-      @include keyboard-mode {
-        &:focus {
-          box-shadow: 0px 0px 0px 2px $color-ultramarine;
-        }
-      }
-
-      @include light-theme {
-        color: $color-ultramarine;
-      }
-
-      @include dark-theme {
-        color: $color-gray-05;
-      }
    }
  }
 }
--- a/ts/components/ConfirmationDialog.stories.tsx
+++ b/ts/components/ConfirmationDialog.stories.tsx
@ -3,7 +3,6 @@

 import * as React from 'react';
 import { action } from '@storybook/addon-actions';
-import { text } from '@storybook/addon-knobs';

 import { ConfirmationDialog } from './ConfirmationDialog';
 import { setupI18n } from '../util/setupI18n';
@ -21,7 +20,7 @@ export const _ConfirmationDialog = (): JSX.Element => {
      dialogName="test"
      i18n={i18n}
      onClose={action('onClose')}
-      title={text('Title', 'Foo bar banana baz?')}
+      title="Foo bar banana baz?"
      actions={[
        {
          text: 'Negate',
@ -35,7 +34,7 @@ export const _ConfirmationDialog = (): JSX.Element => {
        },
      ]}
    >
-      {text('Child text', 'asdf blip')}
+      asdf blip
    </ConfirmationDialog>
  );
 };
@ -51,7 +50,7 @@ export const CustomCancelText = (): JSX.Element => {
      cancelText="Nah"
      i18n={i18n}
      onClose={action('onClose')}
-      title={text('Title', 'Foo bar banana baz?')}
+      title="Maybs?"
      actions={[
        {
          text: 'Maybe',
@ -60,7 +59,7 @@ export const CustomCancelText = (): JSX.Element => {
        },
      ]}
    >
-      {text('Child text', 'asdf blip')}
+      Because.
    </ConfirmationDialog>
  );
 };
@ -68,3 +67,24 @@ export const CustomCancelText = (): JSX.Element => {
 CustomCancelText.story = {
  name: 'Custom cancel text',
 };
+
+export const NoDefaultCancel = (): JSX.Element => {
+  return (
+    <ConfirmationDialog
+      dialogName="test"
+      noDefaultCancelButton
+      i18n={i18n}
+      onClose={action('onClose')}
+      title="Do you?"
+      actions={[
+        {
+          text: 'Yep',
+          style: 'affirmative',
+          action: action('affirmative'),
+        },
+      ]}
+    >
+      No default cancel!
+    </ConfirmationDialog>
+  );
+};
--- a/ts/components/ConfirmationDialog.tsx
+++ b/ts/components/ConfirmationDialog.tsx
@ -27,6 +27,7 @@ export type OwnProps = Readonly<{
  i18n: LocalizerType;
  moduleClassName?: string;
  noMouseClose?: boolean;
+  noDefaultCancelButton?: boolean;
  onCancel?: () => unknown;
  onClose: () => unknown;
  onTopOfEverything?: boolean;
@ -67,6 +68,7 @@ export const ConfirmationDialog = React.memo(
    i18n,
    moduleClassName,
    noMouseClose,
+    noDefaultCancelButton,
    onCancel,
    onClose,
    onTopOfEverything,
@ -98,16 +100,18 @@ export const ConfirmationDialog = React.memo(

    const footer = (
      <>
-        <Button
-          onClick={handleCancel}
-          ref={focusRef}
-          variant={
-            cancelButtonVariant ||
-            (hasActions ? ButtonVariant.Secondary : ButtonVariant.Primary)
-          }
-        >
-          {cancelText || i18n('confirmation-dialog--Cancel')}
-        </Button>
+        {!noDefaultCancelButton ? (
+          <Button
+            onClick={handleCancel}
+            ref={focusRef}
+            variant={
+              cancelButtonVariant ||
+              (hasActions ? ButtonVariant.Secondary : ButtonVariant.Primary)
+            }
+          >
+            {cancelText || i18n('confirmation-dialog--Cancel')}
+          </Button>
+        ) : null}
        {actions.map((action, i) => (
          <Button
            key={action.text}
--- a/ts/components/SafetyNumberChangeDialog.stories.tsx
+++ b/ts/components/SafetyNumberChangeDialog.stories.tsx
@ -22,22 +22,24 @@ const contactWithAllData = getDefaultConversation({
  phoneNumber: '(305) 123-4567',
 });

-const contactWithJustProfile = getDefaultConversation({
+const contactWithJustProfileVerified = getDefaultConversation({
  id: 'def',
  avatarPath: undefined,
  title: '-*Smartest Dude*-',
  profileName: '-*Smartest Dude*-',
  name: undefined,
  phoneNumber: '(305) 123-4567',
+  isVerified: true,
 });

-const contactWithJustNumber = getDefaultConversation({
+const contactWithJustNumberVerified = getDefaultConversation({
  id: 'xyz',
  avatarPath: undefined,
  profileName: undefined,
  name: undefined,
  title: '(305) 123-4567',
  phoneNumber: '(305) 123-4567',
+  isVerified: true,
 });

 const contactWithNothing = getDefaultConversation({
@ -98,8 +100,8 @@ export const MultiContactDialog = (): JSX.Element => {
    <SafetyNumberChangeDialog
      contacts={[
        contactWithAllData,
-        contactWithJustProfile,
-        contactWithJustNumber,
+        contactWithJustProfileVerified,
+        contactWithJustNumberVerified,
        contactWithNothing,
      ]}
      getPreferredBadge={() => undefined}
@ -115,14 +117,35 @@ export const MultiContactDialog = (): JSX.Element => {
  );
 };

+export const AllVerified = (): JSX.Element => {
+  const theme = useTheme();
+  return (
+    <SafetyNumberChangeDialog
+      contacts={[contactWithJustProfileVerified, contactWithJustNumberVerified]}
+      getPreferredBadge={() => undefined}
+      i18n={i18n}
+      onCancel={action('cancel')}
+      onConfirm={action('confirm')}
+      renderSafetyNumber={() => {
+        action('renderSafetyNumber');
+        return <div>This is a mock Safety Number View</div>;
+      }}
+      theme={theme}
+    />
+  );
+};
+AllVerified.story = {
+  name: 'All verified; Send button instead',
+};
+
 export const MultipleContactsAllWithBadges = (): JSX.Element => {
  const theme = useTheme();
  return (
    <SafetyNumberChangeDialog
      contacts={[
        contactWithAllData,
-        contactWithJustProfile,
-        contactWithJustNumber,
+        contactWithJustProfileVerified,
+        contactWithJustNumberVerified,
        contactWithNothing,
      ]}
      getPreferredBadge={() => getFakeBadge()}
@ -142,14 +165,14 @@ MultipleContactsAllWithBadges.story = {
  name: 'Multiple contacts, all with badges',
 };

-export const ScrollDialog = (): JSX.Element => {
+export const TenContacts = (): JSX.Element => {
  const theme = useTheme();
  return (
    <SafetyNumberChangeDialog
      contacts={[
        contactWithAllData,
-        contactWithJustProfile,
-        contactWithJustNumber,
+        contactWithJustProfileVerified,
+        contactWithJustNumberVerified,
        contactWithNothing,
        contactWithAllData,
        contactWithAllData,
@ -170,3 +193,7 @@ export const ScrollDialog = (): JSX.Element => {
    />
  );
 };
+
+TenContacts.story = {
+  name: 'Ten contacts; first isReviewing = false, then scrolling dialog',
+};
--- a/ts/components/SafetyNumberChangeDialog.tsx
+++ b/ts/components/SafetyNumberChangeDialog.tsx
@ -5,6 +5,7 @@ import * as React from 'react';
 import { noop } from 'lodash';

 import { Avatar } from './Avatar';
+import type { ActionSpec } from './ConfirmationDialog';
 import { ConfirmationDialog } from './ConfirmationDialog';
 import { InContactsIcon } from './InContactsIcon';
 import { Modal } from './Modal';
@ -46,6 +47,9 @@ export const SafetyNumberChangeDialog = ({
  renderSafetyNumber,
  theme,
 }: Props): JSX.Element => {
+  const [isReviewing, setIsReviewing] = React.useState<boolean>(
+    contacts.length <= 5
+  );
  const [selectedContact, setSelectedContact] = React.useState<
    ConversationType | undefined
  >(undefined);
@ -76,80 +80,174 @@ export const SafetyNumberChangeDialog = ({
    );
  }

+  const allVerified = contacts.every(contact => contact.isVerified);
+  const actions: Array<ActionSpec> = [
+    {
+      action: onConfirm,
+      text:
+        confirmText ||
+        (allVerified
+          ? i18n('safetyNumberChangeDialog_send')
+          : i18n('sendAnyway')),
+      style: 'affirmative',
+    },
+  ];
+
+  if (isReviewing) {
+    return (
+      <ConfirmationDialog
+        key="SafetyNumberChangeDialog.reviewing"
+        dialogName="SafetyNumberChangeDialog.reviewing"
+        actions={actions}
+        hasXButton
+        i18n={i18n}
+        moduleClassName="module-SafetyNumberChangeDialog__confirm-dialog"
+        noMouseClose
+        noDefaultCancelButton={!isReviewing}
+        onCancel={onClose}
+        onClose={noop}
+      >
+        <div className="module-SafetyNumberChangeDialog__shield-icon" />
+        <div className="module-SafetyNumberChangeDialog__title">
+          {i18n('safetyNumberChanges')}
+        </div>
+        <div className="module-SafetyNumberChangeDialog__message">
+          {i18n('safetyNumberChangeDialog__message')}
+        </div>
+        <ul className="module-SafetyNumberChangeDialog__contacts">
+          {contacts.map((contact: ConversationType) => {
+            const shouldShowNumber = Boolean(
+              contact.name || contact.profileName
+            );
+
+            return (
+              <ContactRow
+                contact={contact}
+                getPreferredBadge={getPreferredBadge}
+                i18n={i18n}
+                setSelectedContact={setSelectedContact}
+                shouldShowNumber={shouldShowNumber}
+                theme={theme}
+              />
+            );
+          })}
+        </ul>
+      </ConfirmationDialog>
+    );
+  }
+
+  actions.unshift({
+    action: () => setIsReviewing(true),
+    text: i18n('safetyNumberChangeDialog__review'),
+  });
+
  return (
    <ConfirmationDialog
-      dialogName="SafetyNumberChangeDialog.confirmSend"
-      actions={[
-        {
-          action: onConfirm,
-          text: confirmText || i18n('sendMessageToContact'),
-          style: 'affirmative',
-        },
-      ]}
+      key="SafetyNumberChangeDialog.manyContacts"
+      dialogName="SafetyNumberChangeDialog.manyContacts"
+      actions={actions}
+      hasXButton
      i18n={i18n}
+      moduleClassName="module-SafetyNumberChangeDialog__confirm-dialog"
      noMouseClose
+      noDefaultCancelButton={!isReviewing}
      onCancel={onClose}
      onClose={noop}
-      title={i18n('safetyNumberChanges')}
    >
-      <div className="module-SafetyNumberChangeDialog__message">
-        {i18n('changedVerificationWarning')}
+      <div className="module-SafetyNumberChangeDialog__shield-icon" />
+      <div className="module-SafetyNumberChangeDialog__title">
+        {i18n('safetyNumberChanges')}
      </div>
-      <ul className="module-SafetyNumberChangeDialog__contacts">
-        {contacts.map((contact: ConversationType) => {
-          const shouldShowNumber = Boolean(contact.name || contact.profileName);
-
-          return (
-            <li
-              className="module-SafetyNumberChangeDialog__contact"
-              key={contact.id}
-            >
-              <Avatar
-                acceptedMessageRequest={contact.acceptedMessageRequest}
-                avatarPath={contact.avatarPath}
-                badge={getPreferredBadge(contact.badges)}
-                color={contact.color}
-                conversationType="direct"
-                i18n={i18n}
-                isMe={contact.isMe}
-                phoneNumber={contact.phoneNumber}
-                profileName={contact.profileName}
-                theme={theme}
-                title={contact.title}
-                sharedGroupNames={contact.sharedGroupNames}
-                size={52}
-                unblurredAvatarPath={contact.unblurredAvatarPath}
-              />
-              <div className="module-SafetyNumberChangeDialog__contact--wrapper">
-                <div className="module-SafetyNumberChangeDialog__contact--name">
-                  {contact.title}
-                  {isInSystemContacts(contact) ? (
-                    <span>
-                      {' '}
-                      <InContactsIcon i18n={i18n} />
-                    </span>
-                  ) : null}
-                </div>
-                {shouldShowNumber ? (
-                  <div className="module-SafetyNumberChangeDialog__contact--number">
-                    {contact.phoneNumber}
-                  </div>
-                ) : null}
-              </div>
-              <button
-                className="module-SafetyNumberChangeDialog__contact--view"
-                onClick={() => {
-                  setSelectedContact(contact);
-                }}
-                tabIndex={0}
-                type="button"
-              >
-                {i18n('view')}
-              </button>
-            </li>
-          );
+      <div className="module-SafetyNumberChangeDialog__message">
+        {i18n('icu:safetyNumberChangeDialog__many-contacts', {
+          count: contacts.length,
        })}
-      </ul>
+      </div>
    </ConfirmationDialog>
  );
 };
+
+type ContactRowProps = Readonly<{
+  contact: ConversationType;
+  getPreferredBadge: PreferredBadgeSelectorType;
+  i18n: LocalizerType;
+  setSelectedContact: (contact: ConversationType) => void;
+  shouldShowNumber: boolean;
+  theme: ThemeType;
+}>;
+
+function ContactRow({
+  contact,
+  getPreferredBadge,
+  i18n,
+  setSelectedContact,
+  shouldShowNumber,
+  theme,
+}: ContactRowProps) {
+  return (
+    <li className="module-SafetyNumberChangeDialog__contact" key={contact.id}>
+      <Avatar
+        acceptedMessageRequest={contact.acceptedMessageRequest}
+        avatarPath={contact.avatarPath}
+        badge={getPreferredBadge(contact.badges)}
+        color={contact.color}
+        conversationType="direct"
+        i18n={i18n}
+        isMe={contact.isMe}
+        phoneNumber={contact.phoneNumber}
+        profileName={contact.profileName}
+        theme={theme}
+        title={contact.title}
+        sharedGroupNames={contact.sharedGroupNames}
+        size={36}
+        unblurredAvatarPath={contact.unblurredAvatarPath}
+      />
+      <div className="module-SafetyNumberChangeDialog__contact--wrapper">
+        <div className="module-SafetyNumberChangeDialog__contact--name">
+          {contact.title}
+          {isInSystemContacts(contact) ? (
+            <span>
+              {' '}
+              <InContactsIcon i18n={i18n} />
+            </span>
+          ) : null}
+        </div>
+        {shouldShowNumber || contact.isVerified ? (
+          <div className="module-SafetyNumberChangeDialog__contact--subtitle">
+            {shouldShowNumber ? (
+              <span className="module-SafetyNumberChangeDialog__rtl-span">
+                {contact.phoneNumber}
+              </span>
+            ) : (
+              ''
+            )}
+            {shouldShowNumber && contact.isVerified ? (
+              <span className="module-SafetyNumberChangeDialog__rtl-span">
+                &nbsp;&middot;&nbsp;
+              </span>
+            ) : (
+              ''
+            )}
+            {contact.isVerified ? (
+              <span className="module-SafetyNumberChangeDialog__rtl-span">
+                {i18n('verified')}
+              </span>
+            ) : (
+              ''
+            )}
+          </div>
+        ) : null}
+      </div>
+      <button
+        className="module-SafetyNumberChangeDialog__contact--view"
+        onClick={() => {
+          setSelectedContact(contact);
+        }}
+        tabIndex={0}
+        type="button"
+      >
+        {i18n('view')}
+      </button>
+    </li>
+  );
+}