Improvements to SafetyNumberChangeDialog

Co-authored-by: Scott Nonnenberg <scott@signal.org>
signalapp · Nov 2, 2022 · 9d2938a · 9d2938a
1 parent f5d7d97
commit 9d2938a
Show file tree

Hide file tree

Showing 6 changed files with 326 additions and 111 deletions.
diff --git 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": {

diff --git 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;
-      padding: 8px 14px;
+      @include button-reset;
+      @include button-secondary-blue-text;
 
-      @include keyboard-mode {
-        &:focus {
-          box-shadow: 0px 0px 0px 2px $color-ultramarine;
-        }
-      }
+      opacity: 0;
+      transition: opacity 150ms cubic-bezier(0.17, 0.17, 0, 1);
 
-      @include light-theme {
-        color: $color-ultramarine;
+      // Using keyboard/mouse classes directly; mixins were doing weird things
+      .mouse-mode #{$contact}:hover & {
+        opacity: 1;
       }
-
-      @include dark-theme {
-        color: $color-gray-05;
+      .keyboard-mode #{$contact}:focus-within & {
+        opacity: 1;
       }
+
+      border-radius: 4px;
+      padding: 8px 14px;
     }
   }
 }
diff --git 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,11 +59,32 @@ export const CustomCancelText = (): JSX.Element => {
         },
       ]}
     >
-      {text('Child text', 'asdf blip')}
+      Because.
     </ConfirmationDialog>
   );
 };
 
 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>
+  );
+};
diff --git 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}

diff --git 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',
+};