chore: fix security settings layout (#29258)

georgewrmarshall · web-flow · commit 22bcc760ab13 · 2024-12-18T16:04:16.000Z
## **Description** This PR improves the layout of the basic security section by aligning the toggle button with the heading text. The changes include: 1. Restructuring the layout using Box components with proper alignment 2. Moving the toggle button to be inline with the heading 3. Improving the visual hierarchy by using proper Typography components 4. Maintaining the description text below with appropriate spacing [![Open in GitHub Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/29258?quickstart=1) ## **Related issues** Fixes: #26667 ## **Manual testing steps** 1. Go to Settings > Security & Privacy 2. Observe the Basic Configuration section at the top 3. Verify the toggle button is properly aligned with the heading 4. Verify the description text appears below with proper spacing ## **Screenshots/Recordings** ### **Before** https://github.com/user-attachments/assets/bab9a14c-8f10-4865-b159-5343537a2785 ### **After** https://github.com/user-attachments/assets/b9d02fd8-8e35-4dfa-a00d-b4e6952625df ## **Pre-merge author checklist** - [x] I've followed [MetaMask Contributor Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Extension Coding Standards](https://github.com/MetaMask/metamask-extension/blob/main/.github/guidelines/CODING_GUIDELINES.md). - [x] I've completed the PR template to the best of my ability - [x] I've included tests if applicable - [x] I've documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [x] I've applied the right labels on the PR (see [labeling guidelines](https://github.com/MetaMask/metamask-extension/blob/main/.github/guidelines/LABELING_GUIDELINES.md)). Not required for external contributors. ## **Pre-merge reviewer checklist** - [ ] I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed). - [ ] I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots.
diff --git a/ui/pages/settings/security-tab/__snapshots__/security-tab.test.js.snap b/ui/pages/settings/security-tab/__snapshots__/security-tab.test.js.snap
@@ -12,11 +12,62 @@ exports[`Security Tab should match snapshot 1`] = `
       <div
         class="settings-page__content-item"
       >
-        <span>
-          Basic functionality
-        </span>
         <div
-          class="settings-page__content-description"
+          class="mm-box mm-box--margin-bottom-2 mm-box--display-flex mm-box--justify-content-space-between mm-box--align-items-center"
+        >
+          <h4
+            class="mm-box mm-text mm-text--heading-sm mm-box--color-text-default"
+          >
+            Basic functionality
+          </h4>
+          <label
+            class="toggle-button toggle-button--off"
+            tabindex="0"
+          >
+            <div
+              style="display: flex; width: 52px; align-items: center; justify-content: flex-start; position: relative; cursor: pointer; background-color: transparent; border: 0px; padding: 0px; user-select: none;"
+            >
+              <div
+                style="width: 40px; height: 24px; padding: 0px; border-radius: 26px; display: flex; align-items: center; justify-content: center; background-color: rgb(106, 115, 125);"
+              >
+                <div
+                  style="font-size: 11px; display: flex; align-items: center; justify-content: center; font-family: 'Helvetica Neue', Helvetica, sans-serif; position: relative; color: rgb(250, 250, 250); margin-top: auto; margin-bottom: auto; line-height: 0; opacity: 0; width: 26px; height: 20px; left: 4px;"
+                />
+                <div
+                  style="font-size: 11px; display: flex; align-items: center; justify-content: center; font-family: 'Helvetica Neue', Helvetica, sans-serif; position: relative; color: rgba(255, 255, 255, 0.6); bottom: 0px; margin-top: auto; margin-bottom: auto; padding-right: 5px; line-height: 0; width: 26px; height: 20px; opacity: 1;"
+                />
+              </div>
+              <div
+                style="position: absolute; height: 100%; top: 0px; left: 0px; display: flex; flex: 1; align-self: stretch; align-items: center; justify-content: flex-start;"
+              >
+                <div
+                  style="width: 18px; height: 18px; display: flex; align-self: center; box-shadow: var(--shadow-size-xs) var(--color-shadow-default); border-radius: 50%; box-sizing: border-box; position: relative; background-color: rgb(255, 255, 255); left: 3px;"
+                />
+              </div>
+              <input
+                style="border: 0px; height: 1px; margin: -1px; overflow: hidden; padding: 0px; position: absolute; width: 1px;"
+                type="checkbox"
+                value="false"
+              />
+            </div>
+            <div
+              class="toggle-button__status"
+            >
+              <span
+                class="toggle-button__label-off"
+              >
+                Off
+              </span>
+              <span
+                class="toggle-button__label-on"
+              >
+                On
+              </span>
+            </div>
+          </label>
+        </div>
+        <p
+          class="mm-box mm-text mm-text--body-md mm-box--margin-bottom-2 mm-box--color-text-alternative"
         >
           <span>
              
@@ -31,57 +82,11 @@ exports[`Security Tab should match snapshot 1`] = `
             .
              
           </span>
-        </div>
+        </p>
       </div>
       <div
         class="settings-page__content-item-col"
-      >
-        <label
-          class="toggle-button toggle-button--off"
-          tabindex="0"
-        >
-          <div
-            style="display: flex; width: 52px; align-items: center; justify-content: flex-start; position: relative; cursor: pointer; background-color: transparent; border: 0px; padding: 0px; user-select: none;"
-          >
-            <div
-              style="width: 40px; height: 24px; padding: 0px; border-radius: 26px; display: flex; align-items: center; justify-content: center; background-color: rgb(106, 115, 125);"
-            >
-              <div
-                style="font-size: 11px; display: flex; align-items: center; justify-content: center; font-family: 'Helvetica Neue', Helvetica, sans-serif; position: relative; color: rgb(250, 250, 250); margin-top: auto; margin-bottom: auto; line-height: 0; opacity: 0; width: 26px; height: 20px; left: 4px;"
-              />
-              <div
-                style="font-size: 11px; display: flex; align-items: center; justify-content: center; font-family: 'Helvetica Neue', Helvetica, sans-serif; position: relative; color: rgba(255, 255, 255, 0.6); bottom: 0px; margin-top: auto; margin-bottom: auto; padding-right: 5px; line-height: 0; width: 26px; height: 20px; opacity: 1;"
-              />
-            </div>
-            <div
-              style="position: absolute; height: 100%; top: 0px; left: 0px; display: flex; flex: 1; align-self: stretch; align-items: center; justify-content: flex-start;"
-            >
-              <div
-                style="width: 18px; height: 18px; display: flex; align-self: center; box-shadow: var(--shadow-size-xs) var(--color-shadow-default); border-radius: 50%; box-sizing: border-box; position: relative; background-color: rgb(255, 255, 255); left: 3px;"
-              />
-            </div>
-            <input
-              style="border: 0px; height: 1px; margin: -1px; overflow: hidden; padding: 0px; position: absolute; width: 1px;"
-              type="checkbox"
-              value="false"
-            />
-          </div>
-          <div
-            class="toggle-button__status"
-          >
-            <span
-              class="toggle-button__label-off"
-            >
-              Off
-            </span>
-            <span
-              class="toggle-button__label-on"
-            >
-              On
-            </span>
-          </div>
-        </label>
-      </div>
+      />
     </div>
     <span
       class="settings-page__security-tab-sub-header__bold"
diff --git a/ui/pages/settings/security-tab/security-tab.component.js b/ui/pages/settings/security-tab/security-tab.component.js
@@ -26,7 +26,7 @@ import {
 import SRPQuiz from '../../../components/app/srp-quiz-modal/SRPQuiz';
 import {
   Button,
-  BUTTON_SIZES,
+  ButtonSize,
   Icon,
   IconSize,
   IconName,
@@ -44,6 +44,7 @@ import {
   TextColor,
   TextVariant,
   IconColor,
+  AlignItems,
 } from '../../../helpers/constants/design-system';
 import { ADD_POPULAR_CUSTOM_NETWORK } from '../../../helpers/constants/routes';
 import {
@@ -177,7 +178,7 @@ export default class SecurityTab extends PureComponent {
           <Button
             data-testid="reveal-seed-words"
             type="danger"
-            size={BUTTON_SIZES.LG}
+            size={ButtonSize.Lg}
             onClick={(event) => {
               event.preventDefault();
               this.context.trackEvent({
@@ -1042,8 +1043,44 @@ export default class SecurityTab extends PureComponent {
         data-testid="advanced-setting-show-testnet-conversion"
       >
         <div className="settings-page__content-item">
-          <span>{t('basicConfigurationLabel')}</span>
-          <div className="settings-page__content-description">
+          <Box
+            display={Display.Flex}
+            justifyContent={JustifyContent.spaceBetween}
+            alignItems={AlignItems.center}
+            marginBottom={2}
+          >
+            <Text variant={TextVariant.headingSm}>
+              {t('basicConfigurationLabel')}
+            </Text>
+            <ToggleButton
+              value={useExternalServices}
+              onToggle={() => {
+                if (useExternalServices) {
+                  // If we are going to be disabling external services, then we want to show the "turn off" warning modal
+                  setBasicFunctionalityModalOpen();
+                } else {
+                  toggleExternalServices(true);
+                  this.context.trackEvent({
+                    category: MetaMetricsEventCategory.Settings,
+                    event: MetaMetricsEventName.SettingsUpdated,
+                    properties: {
+                      settings_group: 'security_privacy',
+                      settings_type: 'basic_functionality',
+                      old_value: false,
+                      new_value: true,
+                      // these values will always be set to false
+                      // when basic functionality is re-enabled
+                      was_notifications_on: false,
+                      was_profile_syncing_on: false,
+                    },
+                  });
+                }
+              }}
+              offLabel={t('off')}
+              onLabel={t('on')}
+            />
+          </Box>
+          <Text marginBottom={2} color={TextColor.textAlternative}>
             {t('basicConfigurationDescription', [
               <a
                 href="https://consensys.io/privacy-policy"
@@ -1054,38 +1091,10 @@ export default class SecurityTab extends PureComponent {
                 {t('privacyMsg')}
               </a>,
             ])}
-          </div>
+          </Text>
         </div>
 
-        <div className="settings-page__content-item-col">
-          <ToggleButton
-            value={useExternalServices}
-            onToggle={() => {
-              if (useExternalServices) {
-                // If we are going to be disabling external services, then we want to show the "turn off" warning modal
-                setBasicFunctionalityModalOpen();
-              } else {
-                toggleExternalServices(true);
-                this.context.trackEvent({
-                  category: MetaMetricsEventCategory.Settings,
-                  event: MetaMetricsEventName.SettingsUpdated,
-                  properties: {
-                    settings_group: 'security_privacy',
-                    settings_type: 'basic_functionality',
-                    old_value: false,
-                    new_value: true,
-                    // these values will always be set to false
-                    // when basic functionality is re-enabled
-                    was_notifications_on: false,
-                    was_profile_syncing_on: false,
-                  },
-                });
-              }
-            }}
-            offLabel={t('off')}
-            onLabel={t('on')}
-          />
-        </div>
+        <div className="settings-page__content-item-col"></div>
       </Box>
     );
   }
