Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

feat: add privacy mode #28021

Open
wants to merge 22 commits into
base: develop
Choose a base branch
from
Open

feat: add privacy mode #28021

wants to merge 22 commits into from

Conversation

jonybur
Copy link
Contributor

@jonybur jonybur commented Oct 22, 2024 •
edited by vinnyhoward
Loading

Description

Adds a privacy mode toggle (an eye icon next to the main balance) that hides all sensitive information/token balances

UPDATE
Here is feedback from @amandaye0h and has been currently implemented in this PR
Figma

Open in GitHub Codespaces

Related issues

Fixes:
https://github.com/MetaMask/MetaMask-planning/issues/3416
https://github.com/MetaMask/MetaMask-planning/issues/3418
https://github.com/MetaMask/MetaMask-planning/issues/3419

Manual testing steps

  1. Go to the Wallet page
  2. Click on the new Eye icon next to the balance
  3. All balances should be hidden
  4. Click on the Eye icon once again
  5. All balances should be shown

Screenshots/Recordings

Before

After

nocontent2.mp4
demo2.mp4
Screenshot 2024-10-22 at 18 43 19

Pre-merge author checklist

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.

@jonybur jonybur requested review from a team as code owners October 22, 2024 18:38
@github-actions GitHub Actions
Copy link
Contributor

CLA Signature Action: All authors have signed the CLA. You may need to manually re-run the blocking PR check if it doesn't pass in a few minutes.

gambinish
gambinish reviewed Oct 22, 2024
View reviewed changes
ui/store/actions.ts Outdated Show resolved Hide resolved
@metamaskbot
Copy link
Collaborator

Builds ready [02b49f1]
Page Load Metrics (2013 ± 72 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint18092557201216881
domContentLoaded17932394196114168
load18582495201315172
domInteractive22166523115
backgroundConnect11115483014
firstReactRender513031056531
getState493342613
initialActions01000
loadScripts12741822145413264
setupStore1390312110
uiStartup205429042281241116
Bundle size diffs [🚨 Warning! Bundle size has increased!]
  • background: 45 Bytes (0.00%)
  • ui: 823 Bytes (0.01%)
  • common: 2.81 KiB (0.04%)

andreahaku
andreahaku reviewed Oct 23, 2024
View reviewed changes
Copy link
Member

@andreahaku andreahaku left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

a few comments on the SensitiveText component

ui/components/component-library/sensitive-text/sensitive-text.tsx Outdated Show resolved Hide resolved
ui/components/component-library/sensitive-text/sensitive-text.tsx Outdated Show resolved Hide resolved
ui/components/component-library/sensitive-text/sensitive-text.tsx Outdated Show resolved Hide resolved
georgewrmarshall
georgewrmarshall reviewed Oct 23, 2024
View reviewed changes
Copy link
Contributor

@georgewrmarshall georgewrmarshall left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @jonybur, this is looking great! Could we pull out the SensitiveText component into its own PR for review? This way, we can ensure the docs, tests, and Storybook stories accurately reflect the component, making it more valuable for the rest of the org. Let me know if you have any questions or need support!

@metamaskbot
Copy link
Collaborator

Builds ready [8fc378a]
Page Load Metrics (2052 ± 128 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint41325781854492236
domContentLoaded170126311995242116
load171426542052267128
domInteractive1895492010
backgroundConnect9265495527
firstReactRender501981093517
getState5106262813
initialActions01000
loadScripts123321221494215103
setupStore1392292211
uiStartup191333022308361173
Bundle size diffs [🚨 Warning! Bundle size has increased!]
  • background: 45 Bytes (0.00%)
  • ui: 823 Bytes (0.01%)
  • common: 3.33 KiB (0.04%)

@vinnyhoward vinnyhoward mentioned this pull request Oct 23, 2024
Merged
7 tasks
@vinnyhoward
Copy link
Contributor

Hey @jonybur, this is looking great! Could we pull out the SensitiveText component into its own PR for review? This way, we can ensure the docs, tests, and Storybook stories accurately reflect the component, making it more valuable for the rest of the org. Let me know if you have any questions or need support!

Here is a separate PR for just the sensitive text component @georgewrmarshall
#28056

@metamaskbot
Copy link
Collaborator

Builds ready [6e27a5c]
Page Load Metrics (2055 ± 66 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint33624641885510245
domContentLoaded17432381200413364
load17692470205513766
domInteractive15234534421
backgroundConnect15100522713
firstReactRender632051093517
getState492242512
initialActions01000
loadScripts12461746146410349
setupStore12104282311
uiStartup19462736229218890
Bundle size diffs [🚨 Warning! Bundle size has increased!]
  • background: 45 Bytes (0.00%)
  • ui: 1.03 KiB (0.01%)
  • common: 3.64 KiB (0.05%)

@metamaskbot
Copy link
Collaborator

Builds ready [7431efc]
Page Load Metrics (2054 ± 69 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint18222362205914670
domContentLoaded17862263201613665
load18202310205414469
domInteractive1693522210
backgroundConnect12112433215
firstReactRender552821034421
getState663242110
initialActions01000
loadScripts13301722149510852
setupStore1274332311
uiStartup20222611229617282
Bundle size diffs [🚨 Warning! Bundle size has increased!]
  • background: 45 Bytes (0.00%)
  • ui: 1.03 KiB (0.01%)
  • common: 3.64 KiB (0.05%)

github-merge-queue bot pushed a commit that referenced this pull request Oct 24, 2024
@vinnyhoward
feat(3419): sensitive text component (#28056)
7d75226 
## **Description**

### Overview
This PR introduces a new `SensitiveText` component to our component
library. The `SensitiveText` component extends our existing `Text`
component to handle sensitive information, providing the ability to hide
or show text content as needed.

### Features
- Extends the existing `Text` component functionality
- Allows toggling between visible and hidden states for sensitive
information
- Supports different lengths of hidden text (Short, Medium, Long,
ExtraLong)
- Maintains all styling capabilities of the `Text` component (variants,
colors, etc)

This is dependent on this open PR
[#28021](#28021)

[![Open in GitHub
Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/28056?quickstart=1)

## **Related issues**

Fixes:
[#3419](MetaMask/MetaMask-planning#3419)

## **Manual testing steps**

1. Go to Storybook to play with new component 👯 

## **Screenshots/Recordings**

NA

### **Before**

NA

### **After**
![Screenshot 2024-10-24 at 11 56
13 AM](https://github.com/user-attachments/assets/58ae3fc8-882d-47f5-8c18-4d69bb033d94)

The screenshots below are from this [pull
request](#28021) and
NOT apart of this PR. This is just to show how it looks like in
extension


https://github.com/user-attachments/assets/2950ac0c-593d-4daa-aa5d-3e6c3a2d5598


https://github.com/user-attachments/assets/6371c2a2-04fa-48a3-8744-991a1540d5f2

<img width="496" alt="Screenshot 2024-10-22 at 18 43 19"
src="https://github.com/user-attachments/assets/d7c2f681-75c7-4be0-921b-f3c5186f8d4a">

## **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/develop/.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/develop/.github/guidelines/LABELING_GUIDELINES.md)).
Not required for external contributors.

## **Pre-merge reviewer checklist**

- [x] I've manually tested the PR (e.g. pull and build branch, run the
app, test code being changed).
- [x] 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.
@vinnyhoward vinnyhoward changed the title feat: add sensitive text component feat: add privacy mode Oct 24, 2024
@georgewrmarshall georgewrmarshall mentioned this pull request Oct 24, 2024
Open
7 tasks
@metamaskbot
Copy link
Collaborator

Builds ready [5ac2d24]
Page Load Metrics (1992 ± 88 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint17432500199717886
domContentLoaded16742378194215775
load16892507199218488
domInteractive28237594421
backgroundConnect10129403417
firstReactRender48288994823
getState56122199
initialActions01000
loadScripts12451737144111857
setupStore1197302412
uiStartup188828352222246118
Bundle size diffs [🚨 Warning! Bundle size has increased!]
  • background: 45 Bytes (0.00%)
  • ui: 1.03 KiB (0.01%)
  • common: 90 Bytes (0.00%)

cryptotavares pushed a commit that referenced this pull request Oct 25, 2024
@vinnyhoward @cryptotavares
feat(3419): sensitive text component (#28056)
27d5e68 
## **Description**

### Overview
This PR introduces a new `SensitiveText` component to our component
library. The `SensitiveText` component extends our existing `Text`
component to handle sensitive information, providing the ability to hide
or show text content as needed.

### Features
- Extends the existing `Text` component functionality
- Allows toggling between visible and hidden states for sensitive
information
- Supports different lengths of hidden text (Short, Medium, Long,
ExtraLong)
- Maintains all styling capabilities of the `Text` component (variants,
colors, etc)

This is dependent on this open PR
[#28021](#28021)

[![Open in GitHub
Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/28056?quickstart=1)

## **Related issues**

Fixes:
[#3419](MetaMask/MetaMask-planning#3419)

## **Manual testing steps**

1. Go to Storybook to play with new component 👯 

## **Screenshots/Recordings**

NA

### **Before**

NA

### **After**
![Screenshot 2024-10-24 at 11 56
13 AM](https://github.com/user-attachments/assets/58ae3fc8-882d-47f5-8c18-4d69bb033d94)

The screenshots below are from this [pull
request](#28021) and
NOT apart of this PR. This is just to show how it looks like in
extension


https://github.com/user-attachments/assets/2950ac0c-593d-4daa-aa5d-3e6c3a2d5598


https://github.com/user-attachments/assets/6371c2a2-04fa-48a3-8744-991a1540d5f2

<img width="496" alt="Screenshot 2024-10-22 at 18 43 19"
src="https://github.com/user-attachments/assets/d7c2f681-75c7-4be0-921b-f3c5186f8d4a">

## **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/develop/.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/develop/.github/guidelines/LABELING_GUIDELINES.md)).
Not required for external contributors.

## **Pre-merge reviewer checklist**

- [x] I've manually tested the PR (e.g. pull and build branch, run the
app, test code being changed).
- [x] 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.
@vinnyhoward
fix: refactor on how privacy mode works with token list and added e2e…
731199a 
… test for account selection modal and privacy mode
@metamaskbot
Copy link
Collaborator

Builds ready [731199a]
Page Load Metrics (1865 ± 51 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint40220961794338162
domContentLoaded16762011183710048
load17202095186510651
domInteractive198343209
backgroundConnect888312210
firstReactRender562941206129
getState56917189
initialActions01000
loadScripts1245152713629144
setupStore1269332512
uiStartup19342609213318689
Bundle size diffs [🚨 Warning! Bundle size has increased!]
  • background: 45 Bytes (0.00%)
  • ui: 1.27 KiB (0.02%)
  • common: 90 Bytes (0.00%)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@georgewrmarshall georgewrmarshall georgewrmarshall left review comments

@andreahaku andreahaku andreahaku left review comments

@vinnyhoward vinnyhoward vinnyhoward left review comments

@gambinish gambinish gambinish left review comments

@darkwing darkwing darkwing requested changes

Requested changes must be addressed to merge this pull request.

Assignees
No one assigned
Labels
team-wallet-ux
Projects
PR review queue
Status: Needs dev review
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@jonybur @metamaskbot @vinnyhoward @darkwing @georgewrmarshall @andreahaku @gambinish