Skip to content

Commit

Permalink
fix: Bug 28347 - Privacy mode tweaks (#28367) (#28372)
Browse files Browse the repository at this point in the history
## **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.
  • Loading branch information
gambinish authored Nov 8, 2024
1 parent 7c12d69 commit a40cb29
Show file tree
Hide file tree
Showing 10 changed files with 33 additions and 11 deletions.
5 changes: 3 additions & 2 deletions ui/components/app/assets/token-cell/token-cell.tsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import React from 'react';
import { useSelector } from 'react-redux';
import { getTokenList, getPreferences } from '../../../../selectors';
import { getTokenList } from '../../../../selectors';
import { useTokenFiatAmount } from '../../../../hooks/useTokenFiatAmount';
import { TokenListItem } from '../../../multichain';
import { isEqualCaseInsensitive } from '../../../../../shared/modules/string-utils';
Expand All @@ -12,6 +12,7 @@ type TokenCellProps = {
symbol: string;
string?: string;
image: string;
privacyMode?: boolean;
onClick?: (arg: string) => void;
};

Expand All @@ -20,10 +21,10 @@ export default function TokenCell({
image,
symbol,
string,
privacyMode = false,
onClick,
}: TokenCellProps) {
const tokenList = useSelector(getTokenList);
const { privacyMode } = useSelector(getPreferences);
const tokenData = Object.values(tokenList).find(
(token) =>
isEqualCaseInsensitive(token.symbol, symbol) &&
Expand Down
4 changes: 3 additions & 1 deletion ui/components/app/assets/token-list/token-list.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,8 @@ export default function TokenList({
nativeToken,
}: TokenListProps) {
const t = useI18nContext();
const { tokenSortConfig, tokenNetworkFilter } = useSelector(getPreferences);
const { tokenSortConfig, tokenNetworkFilter, privacyMode } =
useSelector(getPreferences);
const selectedAccount = useSelector(getSelectedAccount);
const conversionRate = useSelector(getConversionRate);
const nativeTokenWithBalance = useNativeTokenBalance();
Expand Down Expand Up @@ -88,6 +89,7 @@ export default function TokenList({
<TokenCell
key={`${tokenData.symbol}-${tokenData.address}`}
{...tokenData}
privacyMode={privacyMode}
onClick={onTokenClick}
/>
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ export type UserPrefrencedCurrencyDisplayProps = OverridingUnion<
showCurrencySuffix?: boolean;
shouldCheckShowNativeToken?: boolean;
isAggregatedFiatOverviewBalance?: boolean;
privacyMode?: boolean;
}
>;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ export default function UserPreferencedCurrencyDisplay({
showNative,
showCurrencySuffix,
shouldCheckShowNativeToken,
privacyMode = false,
...restProps
}) {
// NOTE: When displaying currencies, we need the actual account to detect whether we're in a
Expand Down Expand Up @@ -83,6 +84,7 @@ export default function UserPreferencedCurrencyDisplay({
numberOfDecimals={numberOfDecimals}
prefixComponent={prefixComponent}
suffix={showCurrencySuffix && !showEthLogo && currency}
privacyMode={privacyMode}
/>
);
}
Expand Down Expand Up @@ -126,6 +128,7 @@ const UserPreferencedCurrencyDisplayPropTypes = {
textProps: PropTypes.object,
suffixProps: PropTypes.object,
shouldCheckShowNativeToken: PropTypes.bool,
privacyMode: PropTypes.bool,
};

UserPreferencedCurrencyDisplay.propTypes =
Expand Down
6 changes: 3 additions & 3 deletions ui/components/app/wallet-overview/coin-overview.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,8 @@ export const CoinOverview = ({

const shouldShowPopover = useSelector(getShouldShowAggregatedBalancePopover);
const isTestnet = useSelector(getIsTestnet);
const { showFiatInTestnets, privacyMode } = useSelector(getPreferences);
const { showFiatInTestnets, privacyMode, showNativeTokenAsMainBalance } =
useSelector(getPreferences);

const selectedAccount = useSelector(getSelectedAccount);
const shouldHideZeroBalanceTokens = useSelector(
Expand All @@ -143,8 +144,6 @@ export const CoinOverview = ({
shouldHideZeroBalanceTokens,
);

const { showNativeTokenAsMainBalance } = useSelector(getPreferences);

const isEvm = useSelector(getMultichainIsEvm);
const isNotAggregatedFiatBalance =
showNativeTokenAsMainBalance || isTestnet || !isEvm;
Expand Down Expand Up @@ -281,6 +280,7 @@ export const CoinOverview = ({
isAggregatedFiatOverviewBalance={
!showNativeTokenAsMainBalance && !isTestnet
}
privacyMode={privacyMode}
/>
<ButtonIcon
color={IconColor.iconAlternative}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,7 @@ const AccountListItem = ({
startAccessory,
onActionClick,
shouldScrollToWhenSelected = true,
privacyMode = false,
}) => {
const t = useI18nContext();
const [accountOptionsMenuOpen, setAccountOptionsMenuOpen] = useState(false);
Expand Down Expand Up @@ -313,6 +314,7 @@ const AccountListItem = ({
type={PRIMARY}
showFiat={showFiat}
data-testid="first-currency-display"
privacyMode={privacyMode}
/>
</Text>
</Box>
Expand Down Expand Up @@ -360,6 +362,7 @@ const AccountListItem = ({
type={SECONDARY}
showNative
data-testid="second-currency-display"
privacyMode={privacyMode}
/>
</Text>
</Box>
Expand Down Expand Up @@ -507,6 +510,10 @@ AccountListItem.propTypes = {
* Determines if list item should be scrolled to when selected
*/
shouldScrollToWhenSelected: PropTypes.bool,
/**
* Determines if list balance should be obfuscated
*/
privacyMode: PropTypes.bool,
};

AccountListItem.displayName = 'AccountListItem';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -188,13 +188,15 @@ export const mergeAccounts = (

type AccountListMenuProps = {
onClose: () => void;
privacyMode?: boolean;
showAccountCreation?: boolean;
accountListItemProps?: object;
allowedAccountTypes?: KeyringAccountType[];
};

export const AccountListMenu = ({
onClose,
privacyMode = false,
showAccountCreation = true,
accountListItemProps,
allowedAccountTypes = [
Expand Down Expand Up @@ -644,6 +646,7 @@ export const AccountListMenu = ({
isHidden={Boolean(account.hidden)}
currentTabOrigin={currentTabOrigin}
isActive={Boolean(account.active)}
privacyMode={privacyMode}
{...accountListItemProps}
/>
</Box>
Expand Down
Original file line number Diff line number Diff line change
@@ -1,10 +1,8 @@
import React from 'react';
import { useSelector } from 'react-redux';
import PropTypes from 'prop-types';
import classnames from 'classnames';
import { useCurrencyDisplay } from '../../../hooks/useCurrencyDisplay';
import { EtherDenomination } from '../../../../shared/constants/common';
import { getPreferences } from '../../../selectors';
import { SensitiveText, Box } from '../../component-library';
import {
AlignItems,
Expand Down Expand Up @@ -35,9 +33,9 @@ export default function CurrencyDisplay({
textProps = {},
suffixProps = {},
isAggregatedFiatOverviewBalance = false,
privacyMode = false,
...props
}) {
const { privacyMode } = useSelector(getPreferences);
const [title, parts] = useCurrencyDisplay(value, {
account,
displayValue,
Expand Down Expand Up @@ -125,6 +123,7 @@ const CurrencyDisplayPropTypes = {
textProps: PropTypes.object,
suffixProps: PropTypes.object,
isAggregatedFiatOverviewBalance: PropTypes.bool,
privacyMode: PropTypes.bool,
};

CurrencyDisplay.propTypes = CurrencyDisplayPropTypes;
7 changes: 6 additions & 1 deletion ui/pages/routes/routes.component.js
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,7 @@ export default class Routes extends Component {
history: PropTypes.object,
location: PropTypes.object,
autoLockTimeLimit: PropTypes.number,
privacyMode: PropTypes.bool,
pageChanged: PropTypes.func.isRequired,
browserEnvironmentOs: PropTypes.string,
browserEnvironmentBrowser: PropTypes.string,
Expand Down Expand Up @@ -417,6 +418,7 @@ export default class Routes extends Component {
switchedNetworkDetails,
clearSwitchedNetworkDetails,
clearEditedNetwork,
privacyMode,
///: BEGIN:ONLY_INCLUDE_IF(keyring-snaps)
isShowKeyringSnapRemovalResultModal,
hideShowKeyringSnapRemovalResultModal,
Expand Down Expand Up @@ -494,7 +496,10 @@ export default class Routes extends Component {
///: END:ONLY_INCLUDE_IF
}
{isAccountMenuOpen ? (
<AccountListMenu onClose={() => toggleAccountMenu()} />
<AccountListMenu
onClose={() => toggleAccountMenu()}
privacyMode={privacyMode}
/>
) : null}
{isNetworkMenuOpen ? (
<NetworkListMenu
Expand Down
3 changes: 2 additions & 1 deletion ui/pages/routes/routes.container.js
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ import Routes from './routes.component';
function mapStateToProps(state) {
const { activeTab, appState } = state;
const { alertOpen, alertMessage, isLoading, loadingMessage } = appState;
const { autoLockTimeLimit = DEFAULT_AUTO_LOCK_TIME_LIMIT } =
const { autoLockTimeLimit = DEFAULT_AUTO_LOCK_TIME_LIMIT, privacyMode } =
getPreferences(state);
const { completedOnboarding } = state.metamask;

Expand All @@ -81,6 +81,7 @@ function mapStateToProps(state) {
isNetworkLoading: isNetworkLoading(state),
currentCurrency: state.metamask.currentCurrency,
autoLockTimeLimit,
privacyMode,
browserEnvironmentOs: state.metamask.browserEnvironment?.os,
browserEnvironmentContainter: state.metamask.browserEnvironment?.browser,
providerId: getNetworkIdentifier(state),
Expand Down

0 comments on commit a40cb29

Please sign in to comment.