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.

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

[Bug]: Privacy Mode (Balances) - Native token Balance is not hidden while having privacy mode, if I go to the Native Assets page (ie ETH) #28347

Closed
seaona opened this issue Nov 7, 2024 · 4 comments · Fixed by #28367
Labels
regression-RC-12.7.0 release-12.8.0 Issue or pull request that will be included in release 12.8.0 release-blocker This bug is blocking the next release Sev2-normal Normal severity; minor loss of service or inconvenience. team-wallet-ux type-bug

Comments

@seaona
Copy link
Contributor

seaona commented Nov 7, 2024

Describe the bug

Whenever I have the privacy mode enabled, I see that the balance is not hidden in the native token page.
Ie if I click on ETH or Sepolia ETH, I can see my balance

Expected behavior

No response

Screenshots/Recordings

prviacy-balance-not-hidden.mp4

Steps to reproduce

  1. Click the eye for hiding the balance
  2. Click on ETH or SepoliaETH asset
  3. See how you can still see your balance

Error messages or log output

No response

Detection stage

During release testing

Version

12.7.0

Build type

None

Browser

Chrome

Operating system

Linux

Hardware wallet

No response

Additional context

No response

Severity

No response

@seaona seaona added type-bug Sev2-normal Normal severity; minor loss of service or inconvenience. release-blocker This bug is blocking the next release team-wallet-ux labels Nov 7, 2024
@darkwing
Copy link
Contributor

darkwing commented Nov 7, 2024

Should be fixed by #28248

@alfeng6
Copy link

alfeng6 commented Nov 7, 2024

@seaona when testing, why did you expect that the token balance in the asset page would be hidden too? Given the setting is a local setting (only able to toggle in the main wallet view), originally the intention was to have the hidden values only occur in the main wallet view (main token list + account picker).

@seaona
Copy link
Contributor Author

seaona commented Nov 7, 2024

@seaona when testing, why did you expect that the token balance in the asset page would be hidden too?

hi there @alfeng6 👋 I expect it to be hidden because I see this behaviour in all the token pages, except the native token (ie ETH).

privacy-tokens-native.mp4

the intention was to have the hidden values only occur in the main wallet view (main token list + account picker).

if that's the case, then we shouldn't hide the values in any asset page? (currently balances are hidden in the assets pages, except in the native asset page).

@alfeng6
Copy link

alfeng6 commented Nov 7, 2024

ah ok let me make sure the team knows not to hide the token values for any of the asset pages :) thank you for your QA!

github-merge-queue bot pushed a commit that referenced this issue Nov 7, 2024
## **Description**

Privacy Mode should only effect PortfolioView and main account picker
popover. It should not impact other areas of the App like Send/Swap/Gas
because the toggle only exists on PortfolioView.

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

## **Related issues**

Fixes: #28347

## **Manual testing steps**

You can toggle privacyMode with eyeball on main PortfolioView

Should respect privacyMode:
1. Go to PortfolioView, toggling eyeball should show/hide balances for
tokens as well as main balance
2. Go to AccountPicker from main Portfolio View, balances should
hide/show
3. Go to AccountPicker from asset detail view, balances should hide/show

Should _not_ respect privacyMode:
1. Go to AssetDetails, token balance should show on main page
2. Should not be respected on send/swap/gas screens
3. Balances should not be impacted elsewhere in the app. Please try to
verify this while reviewing.

## **Screenshots/Recordings**

https://github.com/user-attachments/assets/695fa68a-c9bb-4871-b03c-8c41c88b1344

## **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**

- [ ] 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.
@metamaskbot metamaskbot added the release-12.8.0 Issue or pull request that will be included in release 12.8.0 label Nov 7, 2024
gambinish added a commit that referenced this issue Nov 7, 2024
## **Description**

Privacy Mode should only effect PortfolioView and main account picker
popover. It should not impact other areas of the App like Send/Swap/Gas
because the toggle only exists on PortfolioView.

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

## **Related issues**

Fixes: #28347

## **Manual testing steps**

You can toggle privacyMode with eyeball on main PortfolioView

Should respect privacyMode:
1. Go to PortfolioView, toggling eyeball should show/hide balances for
tokens as well as main balance
2. Go to AccountPicker from main Portfolio View, balances should
hide/show
3. Go to AccountPicker from asset detail view, balances should hide/show

Should _not_ respect privacyMode:
1. Go to AssetDetails, token balance should show on main page
2. Should not be respected on send/swap/gas screens
3. Balances should not be impacted elsewhere in the app. Please try to
verify this while reviewing.

## **Screenshots/Recordings**

https://github.com/user-attachments/assets/695fa68a-c9bb-4871-b03c-8c41c88b1344

## **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**

- [ ] 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.
gambinish added a commit that referenced this issue Nov 8, 2024
## **Description**

Cherry pick of
82fdd64

Original PR: #28367

Privacy Mode should only effect PortfolioView and main account picker
popover. It should not impact other areas of the App like Send/Swap/Gas
because the toggle only exists on PortfolioView.

[![Open in GitHub

Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/28367?quickstart=1)

## **Related issues**

Fixes: #28347

## **Manual testing steps**

You can toggle privacyMode with eyeball on main PortfolioView

Should respect privacyMode:
1. Go to PortfolioView, toggling eyeball should show/hide balances for
tokens as well as main balance
2. Go to AccountPicker from main Portfolio View, balances should
hide/show
3. Go to AccountPicker from asset detail view, balances should hide/show

Should _not_ respect privacyMode:
1. Go to AssetDetails, token balance should show on main page
2. Should not be respected on send/swap/gas screens
3. Balances should not be impacted elsewhere in the app. Please try to
verify this while reviewing.

## **Screenshots/Recordings**


https://github.com/user-attachments/assets/695fa68a-c9bb-4871-b03c-8c41c88b1344

## **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.


<!--
Please submit this PR as a draft initially.
Do not mark it as "Ready for review" until the template has been
completely filled out, and PR status checks have passed at least once.
-->

## **Description**

<!--
Write a short description of the changes included in this pull request,
also include relevant motivation and context. Have in mind the following
questions:
1. What is the reason for the change?
2. What is the improvement/solution?
-->

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

## **Related issues**

Fixes:

## **Manual testing steps**

1. Go to this page...
2.
3.

## **Screenshots/Recordings**

<!-- If applicable, add screenshots and/or recordings to visualize the
before and after of your change. -->

### **Before**

<!-- [screenshots/recordings] -->

### **After**

<!-- [screenshots/recordings] -->

## **Pre-merge author checklist**

- [ ] 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).
- [ ] I've completed the PR template to the best of my ability
- [ ] I’ve included tests if applicable
- [ ] I’ve documented my code using [JSDoc](https://jsdoc.app/) format
if applicable
- [ ] 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**

- [ ] 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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
regression-RC-12.7.0 release-12.8.0 Issue or pull request that will be included in release 12.8.0 release-blocker This bug is blocking the next release Sev2-normal Normal severity; minor loss of service or inconvenience. team-wallet-ux type-bug
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

4 participants