Skip to content

Commit

Permalink
chore: fix security settings layout (#29258)
Browse files Browse the repository at this point in the history 
## **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.
  • Loading branch information
@georgewrmarshall
georgewrmarshall authored Dec 18, 2024
1 parent 41b46a0 commit 22bcc76
Show file tree
Hide file tree
Showing 2 changed files with 100 additions and 86 deletions.
109 changes: 57 additions & 52 deletions ui/pages/settings/security-tab/__snapshots__/security-tab.test.js.snap
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand All @@ -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"
Expand Down
77 changes: 43 additions & 34 deletions ui/pages/settings/security-tab/security-tab.component.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ import {
import SRPQuiz from '../../../components/app/srp-quiz-modal/SRPQuiz';
import {
Button,
BUTTON_SIZES,
ButtonSize,
Icon,
IconSize,
IconName,
Expand All @@ -44,6 +44,7 @@ import {
TextColor,
TextVariant,
IconColor,
AlignItems,
} from '../../../helpers/constants/design-system';
import { ADD_POPULAR_CUSTOM_NETWORK } from '../../../helpers/constants/routes';
import {
Expand Down Expand Up @@ -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({
Expand Down Expand Up @@ -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"
Expand All @@ -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>
);
}
Expand Down

0 comments on commit 22bcc76

Please sign in to comment.