From f675e7992a9ca0dc5888370920e0f29fa0e50d56 Mon Sep 17 00:00:00 2001
From: Alex Gleason <alex@alexgleason.me>
Date: Wed, 12 Jan 2022 12:03:24 -0600
Subject: [PATCH] MfaForm: improve styles and usability

---
 app/soapbox/components/button.js          |  2 +
 app/soapbox/features/security/mfa_form.js | 74 +++++++++++++++--------
 app/soapbox/locales/en.json               |  8 +--
 app/styles/components/mfa_form.scss       | 27 +++++++--
 app/styles/forms.scss                     | 14 -----
 5 files changed, 75 insertions(+), 50 deletions(-)

diff --git a/app/soapbox/components/button.js b/app/soapbox/components/button.js
index 43ca98588..25e116154 100644
--- a/app/soapbox/components/button.js
+++ b/app/soapbox/components/button.js
@@ -8,6 +8,7 @@ import Icon from './icon';
 export default class Button extends React.PureComponent {
 
   static propTypes = {
+    type: PropTypes.string,
     text: PropTypes.node,
     onClick: PropTypes.func,
     to: PropTypes.string,
@@ -54,6 +55,7 @@ export default class Button extends React.PureComponent {
 
     const btn = (
       <button
+        type={this.props.type}
         className={className}
         disabled={this.props.disabled}
         onClick={this.handleClick}
diff --git a/app/soapbox/features/security/mfa_form.js b/app/soapbox/features/security/mfa_form.js
index 5e5adb04e..072703b1a 100644
--- a/app/soapbox/features/security/mfa_form.js
+++ b/app/soapbox/features/security/mfa_form.js
@@ -47,6 +47,8 @@ const messages = defineMessages({
   disableFail: { id: 'security.disable.fail', defaultMessage: 'Incorrect password. Try again.' },
   mfaDisableSuccess: { id: 'mfa.disable.success_message', defaultMessage: 'MFA disabled' },
   mfaConfirmSuccess: { id: 'mfa.confirm.success_message', defaultMessage: 'MFA confirmed' },
+  codePlaceholder: { id: 'mfa.mfa_setup.code_placeholder', defaultMessage: 'Code' },
+  passwordPlaceholder: { id: 'mfa.mfa_setup.password_placeholder', defaultMessage: 'Password' },
 });
 
 const mapStateToProps = state => ({
@@ -147,26 +149,33 @@ class DisableOtpForm extends ImmutablePureComponent {
     const { isLoading, password } = this.state;
 
     return (
-      <SimpleForm onSubmit={this.handleSubmit} disabled={isLoading}>
-        <div className='security-settings-panel'>
+      <div className='security-settings-panel'>
+        <SimpleForm onSubmit={this.handleSubmit} disabled={isLoading}>
           <h1 className='security-settings-panel__setup-otp'>
             <FormattedMessage id='mfa.otp_enabled_title' defaultMessage='OTP Enabled' />
           </h1>
-          <div><FormattedMessage id='mfa.otp_enabled_description' defaultMessage='You have enabled two-factor authentication via OTP.' /></div>
-          <div><FormattedMessage id='mfa.mfa_disable_enter_password' defaultMessage='Enter your current password to disable two-factor auth:' /></div>
+          <h2 className='security-settings-panel__setup-otp'>
+            <FormattedMessage id='mfa.otp_enabled_description' defaultMessage='You have enabled two-factor authentication via OTP.' />
+          </h2>
           <ShowablePassword
+            label={intl.formatMessage(messages.passwordPlaceholder)}
+            placeholder={intl.formatMessage(messages.passwordPlaceholder)}
+            hint={<FormattedMessage id='mfa.mfa_disable_enter_password' defaultMessage='Enter your current password to disable two-factor auth.' />}
             disabled={isLoading}
             name='password'
             onChange={this.handleInputChange}
             value={password}
+            required
           />
-          <Button
-            disabled={isLoading}
-            className='button button-primary disable'
-            text={intl.formatMessage(messages.mfa_setup_disable_button)}
-          />
-        </div>
-      </SimpleForm>
+          <div className='security-settings-panel__setup-otp__buttons'>
+            <Button
+              disabled={isLoading}
+              className='button button-primary disable'
+              text={intl.formatMessage(messages.mfa_setup_disable_button)}
+            />
+          </div>
+        </SimpleForm>
+      </div>
     );
   }
 
@@ -202,6 +211,7 @@ class EnableOtpForm extends ImmutablePureComponent {
 
   handleCancelClick = e => {
     this.context.router.history.push('../auth/edit');
+    e.preventDefault();
   }
 
   render() {
@@ -209,8 +219,8 @@ class EnableOtpForm extends ImmutablePureComponent {
     const { backupCodes, displayOtpForm } = this.state;
 
     return (
-      <SimpleForm>
-        <div className='security-settings-panel'>
+      <div className='security-settings-panel'>
+        <SimpleForm>
           <h1 className='security-settings-panel__setup-otp'>
             <FormattedMessage id='mfa.setup_otp_title' defaultMessage='OTP Disabled' />
           </h1>
@@ -244,8 +254,8 @@ class EnableOtpForm extends ImmutablePureComponent {
               )}
             </div>
           )}
-        </div>
-      </SimpleForm>
+        </SimpleForm>
+      </div>
     );
   }
 
@@ -286,6 +296,11 @@ class OtpConfirmForm extends ImmutablePureComponent {
     this.setState({ [e.target.name]: e.target.value });
   }
 
+  handleCancelClick = e => {
+    this.context.router.history.push('../auth/edit');
+    e.preventDefault();
+  }
+
   handleSubmit = e => {
     const { dispatch, intl } = this.props;
     const { code, password } = this.state;
@@ -308,41 +323,48 @@ class OtpConfirmForm extends ImmutablePureComponent {
     const { isLoading, qrCodeURI, confirm_key, password, code } = this.state;
 
     return (
-      <SimpleForm onSubmit={this.handleSubmit} disabled={isLoading}>
-        <div className='security-settings-panel'>
+      <div className='security-settings-panel'>
+        <SimpleForm onSubmit={this.handleSubmit} disabled={isLoading}>
 
           <fieldset disabled={false}>
             <FieldsGroup>
               <div className='security-settings-panel__section-container'>
                 <h2><FormattedMessage id='mfa.mfa_setup_scan_title' defaultMessage='Scan' /></h2>
 
-                <div><FormattedMessage id='mfa.mfa_setup_scan_description' defaultMessage='Using your two-factor app, scan this QR code or enter text key:' /></div>
+                <div><FormattedMessage id='mfa.mfa_setup_scan_description' defaultMessage='Using your two-factor app, scan this QR code or enter the text key.' /></div>
 
-                <span className='security-settings-panel qr-code'>
+                <div className='security-settings-panel__qr-code'>
                   <QRCode value={qrCodeURI} />
-                </span>
+                  <div className='security-settings-panel__confirm-key'>
+                    {confirm_key}
+                  </div>
+                </div>
 
-                <div className='security-settings-panel confirm-key'><FormattedMessage id='mfa.mfa_setup_scan_key' defaultMessage='Key:' /> {confirm_key}</div>
               </div>
 
               <div className='security-settings-panel__section-container'>
                 <h2><FormattedMessage id='mfa.mfa_setup_verify_title' defaultMessage='Verify' /></h2>
 
-                <div><FormattedMessage id='mfa.mfa_setup_verify_description' defaultMessage='To enable two-factor authentication, enter the code from your two-factor app:' /></div>
                 <TextInput
                   name='code'
+                  label={intl.formatMessage(messages.codePlaceholder)}
+                  hint={<FormattedMessage id='mfa.mfa_setup.code_hint' defaultMessage='Enter the code from your two-factor app.' />}
+                  placeholder={intl.formatMessage(messages.codePlaceholder)}
                   onChange={this.handleInputChange}
                   autoComplete='off'
                   value={code}
                   disabled={isLoading}
+                  required
                 />
-
-                <div><FormattedMessage id='mfa.mfa_setup_enter_password' defaultMessage='Enter your current password to confirm your identity:' /></div>
                 <ShowablePassword
                   name='password'
+                  label={intl.formatMessage(messages.passwordPlaceholder)}
+                  hint={<FormattedMessage id='mfa.mfa_setup.password_hint' defaultMessage='Enter your current password to confirm your identity.' />}
+                  placeholder={intl.formatMessage(messages.passwordPlaceholder)}
                   onChange={this.handleInputChange}
                   value={password}
                   disabled={isLoading}
+                  required
                 />
               </div>
             </FieldsGroup>
@@ -362,8 +384,8 @@ class OtpConfirmForm extends ImmutablePureComponent {
               disabled={isLoading}
             />
           </div>
-        </div>
-      </SimpleForm>
+        </SimpleForm>
+      </div>
     );
   }
 
diff --git a/app/soapbox/locales/en.json b/app/soapbox/locales/en.json
index c43dd7793..f90434132 100644
--- a/app/soapbox/locales/en.json
+++ b/app/soapbox/locales/en.json
@@ -580,12 +580,12 @@
   "media_gallery.toggle_visible": "Toggle visibility",
   "media_panel.empty_message": "No media found.",
   "media_panel.title": "Media",
-  "mfa.mfa_disable_enter_password": "Enter your current password to disable two-factor auth:",
-  "mfa.mfa_setup_enter_password": "Enter your current password to confirm your identity:",
-  "mfa.mfa_setup_scan_description": "Using your two-factor app, scan this QR code or enter text key:",
+  "mfa.mfa_disable_enter_password": "Enter your current password to disable two-factor auth.",
+  "mfa.mfa_setup_enter_password": "Enter your current password to confirm your identity",
+  "mfa.mfa_setup_scan_description": "Using your two-factor app, scan this QR code or enter the text key.",
   "mfa.mfa_setup_scan_key": "Key:",
   "mfa.mfa_setup_scan_title": "Scan",
-  "mfa.mfa_setup_verify_description": "To enable two-factor authentication, enter the code from your two-factor app:",
+  "mfa.mfa_setup_verify_description": "To enable two-factor authentication, enter the code from your two-factor app",
   "mfa.mfa_setup_verify_title": "Verify",
   "mfa.otp_enabled_description": "You have enabled two-factor authentication via OTP.",
   "mfa.otp_enabled_title": "OTP Enabled",
diff --git a/app/styles/components/mfa_form.scss b/app/styles/components/mfa_form.scss
index 1e45f20c2..f16f782e3 100644
--- a/app/styles/components/mfa_form.scss
+++ b/app/styles/components/mfa_form.scss
@@ -1,6 +1,10 @@
 .security-settings-panel {
   margin: 20px;
 
+  .simple_form {
+    padding: 0 !important;
+  }
+
   h1.security-settings-panel__setup-otp {
     font-size: 20px;
     line-height: 1.25;
@@ -26,7 +30,7 @@
   }
 
   .backup_codes {
-    margin: 20px;
+    margin: 10px 0;
     font-weight: bold;
     padding: 15px 20px;
     font-size: 14px;
@@ -35,6 +39,9 @@
     text-align: center;
     position: relative;
     min-height: 125px;
+    display: flex;
+    justify-content: center;
+    align-items: center;
 
     .backup_code {
       margin: 5px auto;
@@ -42,22 +49,30 @@
   }
 
   .security-settings-panel__setup-otp__buttons {
-    margin: 20px;
+    margin: 15px 0;
     display: flex;
     justify-content: space-between;
 
     .button {
-      min-width: 182px;
+      flex: 1;
     }
   }
 
-  div.confirm-key {
+  &__qr-code {
+    margin: 20px 0;
+    display: flex;
+    flex-direction: column;
+    justify-content: center;
+    align-items: center;
+  }
+
+  &__confirm-key {
     display: block;
-    font-size: 16px;
+    font-size: 14px;
     line-height: 1.5;
     color: var(--primary-text-color--faint);
     font-weight: 400;
-    margin: 0 0 20px 20px;
+    margin-top: 10px;
   }
 }
 
diff --git a/app/styles/forms.scss b/app/styles/forms.scss
index e62ec0b11..9fc4ad170 100644
--- a/app/styles/forms.scss
+++ b/app/styles/forms.scss
@@ -643,20 +643,6 @@ code {
   font-size: 24px;
 }
 
-.qr-code {
-  flex: 0 0 auto;
-  background: var(--foreground-color);
-  padding: 4px;
-  margin: 0 10px 20px 0;
-  box-shadow: 0 0 15px rgba($base-shadow-color, 0.2);
-  display: inline-block;
-
-  svg {
-    display: block;
-    margin: 0;
-  }
-}
-
 .simple_form {
   .warning {
     box-sizing: border-box;