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

feat(4057): replace asset avatars with network avatars #13910

Draft
wants to merge 7 commits into
base: main
Choose a base branch
from
+1,268 −171
Draft

feat(4057): replace asset avatars with network avatars #13910

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
d5508fa
feat: replaced asset avatars with network avatars in account selectio…
vinnyhoward Mar 7, 2025
f2ac4e4
fix: update comments
vinnyhoward Mar 7, 2025
5641532
fix: update and refined account selector unit tests
vinnyhoward Mar 8, 2025
3e47871
fix: add test coverage as well as fixed more unit tests, aggregated b…
vinnyhoward Mar 11, 2025
675ab11
Merge branch 'main' of github.com:MetaMask/metamask-mobile into feat-…
vinnyhoward Mar 11, 2025
01a4d95
update: comments
vinnyhoward Mar 12, 2025
f741507
Merge branch 'main' into feat-4057-replace-asset-avatars-with-network…
vinnyhoward Mar 12, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions app/component-library/components/Avatars/AvatarGroup/AvatarGroup.styles.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,12 @@ const styleSheet = (params: {
textStyle: {
marginLeft: OVERFLOWTEXTMARGIN_BY_AVATARSIZE[size],
},
avatarGroup: {
borderRadius: 4,
borderWidth: 2,
height: 16,
width: 16,
},
Comment on lines +37 to +42
Copy link
Contributor

Choose a reason for hiding this comment

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

Will these styles be applied to all AvatarGroup styles throughout the app?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Theres only one other instance of it being used which is network permission summary when interacting with dapps. I believe this change is fine for that scenario but I'll double check with Hester

});
};

Expand Down
78 changes: 78 additions & 0 deletions app/component-library/components/Avatars/AvatarGroup/AvatarGroup.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,4 +72,82 @@ describe('AvatarGroup', () => {
spaceBetweenAvatars,
);
});
it('should render the last maxStackedAvatars avatars when renderOverflowCounter is false', () => {
const avatarPropsList = [
...SAMPLE_AVATARGROUP_PROPS.avatarPropsList,

{
variant: 'Account',
type: 'JazzIcon',
accountAddress: '0xLastAddress1',
},
{
variant: 'Account',
type: 'JazzIcon',
accountAddress: '0xLastAddress2',
},
{
variant: 'Account',
type: 'JazzIcon',
accountAddress: '0xLastAddress3',
},
];

const maxStackedAvatars = 4;

const { getAllByTestId, queryByTestId } = renderComponent({
avatarPropsList,
maxStackedAvatars,
renderOverflowCounter: false,
});

const overflowCounter = queryByTestId(AVATARGROUP_OVERFLOWCOUNTER_TESTID);
expect(overflowCounter).toBeNull();

const avatars = getAllByTestId(AVATARGROUP_AVATAR_TESTID);

expect(avatars.length).toBe(maxStackedAvatars);
const lastAvatarProps = avatarPropsList.slice(-maxStackedAvatars);

expect(avatars.length).toBe(lastAvatarProps.length);
});

it('should render the first maxStackedAvatars avatars when renderOverflowCounter is true', () => {
const avatarPropsList = [
{
variant: 'Account',
type: 'JazzIcon',
accountAddress: '0xFirstAddress1',
},
{
variant: 'Account',
type: 'JazzIcon',
accountAddress: '0xFirstAddress2',
},
{
variant: 'Account',
type: 'JazzIcon',
accountAddress: '0xFirstAddress3',
},
...SAMPLE_AVATARGROUP_PROPS.avatarPropsList,
];

const maxStackedAvatars = 3;

const { getAllByTestId, getByTestId } = renderComponent({
avatarPropsList,
maxStackedAvatars,
renderOverflowCounter: true,
});

const overflowCounter = getByTestId(AVATARGROUP_OVERFLOWCOUNTER_TESTID);
expect(overflowCounter).toBeDefined();

const avatars = getAllByTestId(AVATARGROUP_AVATAR_TESTID);

expect(avatars.length).toBe(maxStackedAvatars);

const overflowCounterNumber = avatarPropsList.length - maxStackedAvatars;
expect(overflowCounter.props.children).toBe(`+${overflowCounterNumber}`);
});
});
64 changes: 34 additions & 30 deletions app/component-library/components/Avatars/AvatarGroup/AvatarGroup.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,51 +27,55 @@ const AvatarGroup = ({
maxStackedAvatars = DEFAULT_AVATARGROUP_MAXSTACKEDAVATARS,
includesBorder = true,
spaceBetweenAvatars,
renderOverflowCounter = true,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is introduced to optionally avoid rendering the overflow counter to match the Figma

with prop without prop
with without

style,
}: AvatarGroupProps) => {
const overflowCounter = avatarPropsList.length - maxStackedAvatars;
const avatarNegativeSpacing =
spaceBetweenAvatars || SPACEBETWEENAVATARS_BY_AVATARSIZE[size];
const shouldRenderOverflowCounter = overflowCounter > 0;

const doesOverflowCountExist = overflowCounter > 0;
const { styles } = useStyles(styleSheet, {
size,
style,
});

const renderAvatarList = useCallback(
() =>
avatarPropsList.slice(0, maxStackedAvatars).map((avatarProps, index) => {
const marginLeft = index === 0 ? 0 : avatarNegativeSpacing;
const renderAvatarList = useCallback(() => {
const avatarsToRender = renderOverflowCounter
? avatarPropsList.slice(0, maxStackedAvatars)
: avatarPropsList.slice(-maxStackedAvatars);
return avatarsToRender.map((avatarProps, index) => {
const marginLeft = index === 0 ? 0 : avatarNegativeSpacing;

return (
<View
key={`avatar-${index}`}
testID={`${AVATARGROUP_AVATAR_CONTAINER_TESTID}-${index}`}
style={{ marginLeft }}
>
<Avatar
{...avatarProps}
size={size}
includesBorder={includesBorder}
testID={AVATARGROUP_AVATAR_TESTID}
/>
</View>
);
}),
[
avatarPropsList,
avatarNegativeSpacing,
includesBorder,
maxStackedAvatars,
size,
],
);
return (
<View
key={`avatar-${index}`}
testID={`${AVATARGROUP_AVATAR_CONTAINER_TESTID}-${index}`}
style={{ marginLeft }}
>
<Avatar
{...avatarProps}
size={size}
includesBorder={includesBorder}
testID={AVATARGROUP_AVATAR_TESTID}
style={styles.avatarGroup}
/>
</View>
);
});
}, [
avatarPropsList,
avatarNegativeSpacing,
includesBorder,
maxStackedAvatars,
renderOverflowCounter,
size,
styles.avatarGroup,
]);

return (
<View style={styles.base}>
{renderAvatarList()}
{shouldRenderOverflowCounter && (
{renderOverflowCounter && doesOverflowCountExist && (
<Text
variant={TEXTVARIANT_BY_AVATARSIZE[size]}
color={DEFAULT_AVATARGROUP_COUNTER_TEXTCOLOR}
Expand Down
5 changes: 5 additions & 0 deletions app/component-library/components/Avatars/AvatarGroup/AvatarGroup.types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,11 @@ export interface AvatarGroupProps extends ViewProps {
* - Please refer to the constants file for the mappings.
*/
spaceBetweenAvatars?: number;
/**
* Optional boolean to render the overflow counter.
* @default true
*/
renderOverflowCounter?: boolean;
}
export interface AvatarGroupStyleSheetVars
extends Pick<AvatarGroupProps, 'style'> {
Expand Down
12 changes: 8 additions & 4 deletions app/component-library/components/Avatars/AvatarGroup/__snapshots__/AvatarGroup.test.tsx.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,8 @@ exports[`AvatarGroup should render AvatarGroup component 1`] = `
{
"alignItems": "center",
"backgroundColor": "#ffffff",
"borderRadius": 8,
"borderRadius": 4,
"borderWidth": 2,
"height": 16,
"justifyContent": "center",
"overflow": "hidden",
Expand Down Expand Up @@ -57,7 +58,8 @@ exports[`AvatarGroup should render AvatarGroup component 1`] = `
style={
{
"backgroundColor": "#ffffff",
"borderRadius": 8,
"borderRadius": 4,
"borderWidth": 2,
"height": 16,
"overflow": "hidden",
"width": 16,
Expand Down Expand Up @@ -96,7 +98,8 @@ exports[`AvatarGroup should render AvatarGroup component 1`] = `
style={
{
"backgroundColor": "#ffffff",
"borderRadius": 8,
"borderRadius": 4,
"borderWidth": 2,
"height": 16,
"overflow": "hidden",
"width": 16,
Expand Down Expand Up @@ -135,7 +138,8 @@ exports[`AvatarGroup should render AvatarGroup component 1`] = `
style={
{
"backgroundColor": "#ffffff",
"borderRadius": 8,
"borderRadius": 4,
"borderWidth": 2,
"height": 16,
"overflow": "hidden",
"width": 16,
Expand Down
26 changes: 6 additions & 20 deletions app/components/UI/AccountSelectorList/AccountSelector.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import {
import { mockNetworkState } from '../../../util/test/network';
import { CHAIN_IDS } from '@metamask/transaction-controller';
import { AccountSelectorListProps } from './AccountSelectorList.types';
import { AVATARGROUP_AVATAR_CONTAINER_TESTID } from '../../../component-library/components/Avatars/AvatarGroup/AvatarGroup.constants';

// eslint-disable-next-line import/no-namespace
import * as Utils from '../../hooks/useAccounts/utils';
Expand Down Expand Up @@ -159,12 +160,10 @@ describe('AccountSelectorList', () => {
`${AccountListBottomSheetSelectorsIDs.ACCOUNT_BALANCE_BY_ADDRESS_TEST_ID}-${PERSONAL_ACCOUNT}`,
);

expect(within(businessAccountItem).getByText(regex.eth(1))).toBeDefined();
expect(
within(businessAccountItem).getByText(regex.usd(3200)),
).toBeDefined();

expect(within(personalAccountItem).getByText(regex.eth(2))).toBeDefined();
expect(
within(personalAccountItem).getByText(regex.usd(6400)),
).toBeDefined();
Expand Down Expand Up @@ -199,7 +198,6 @@ describe('AccountSelectorList', () => {
`${AccountListBottomSheetSelectorsIDs.ACCOUNT_BALANCE_BY_ADDRESS_TEST_ID}-${BUSINESS_ACCOUNT}`,
);

expect(within(businessAccountItem).getByText(regex.eth(1))).toBeDefined();
expect(
within(businessAccountItem).getByText(regex.usd(3200)),
).toBeDefined();
Expand Down Expand Up @@ -267,33 +265,21 @@ describe('AccountSelectorList', () => {
`${AccountListBottomSheetSelectorsIDs.ACCOUNT_BALANCE_BY_ADDRESS_TEST_ID}-${BUSINESS_ACCOUNT}`,
);

expect(within(businessAccountItem).getByText(regex.eth(1))).toBeDefined();
expect(
within(businessAccountItem).getByText(regex.usd(3200)),
).toBeDefined();

expect(within(businessAccountItem).queryByText('••••••')).toBeNull();
});
});
it('Text is hidden when privacy mode is on', async () => {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This test is removed because we are no longer showing the selected network token balance, instead it shows the network avatar icons. See the before and after screenshots

const state = {
...initialState,
privacyMode: true,
};

const { queryByTestId } = renderComponent(state);
it('should render AvatarGroup', async () => {
const { queryByTestId } = renderComponent(initialState);

await waitFor(() => {
const businessAccountItem = queryByTestId(
`${AccountListBottomSheetSelectorsIDs.ACCOUNT_BALANCE_BY_ADDRESS_TEST_ID}-${BUSINESS_ACCOUNT}`,
const avatarGroup = queryByTestId(
`${AVATARGROUP_AVATAR_CONTAINER_TESTID}-0`,
);

expect(within(businessAccountItem).queryByText(regex.eth(1))).toBeNull();
expect(
within(businessAccountItem).queryByText(regex.usd(3200)),
).toBeNull();

expect(within(businessAccountItem).getByText('••••••')).toBeDefined();
expect(avatarGroup).toBeDefined();
});
});
});
7 changes: 6 additions & 1 deletion app/components/UI/AccountSelectorList/AccountSelectorList.styles.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,12 @@ const styleSheet = () =>
alignItems: 'flex-end',
flexDirection: 'column',
},
balanceLabel: { textAlign: 'right' },
balanceLabel: {
textAlign: 'right',
},
networkTokensContainer: {
marginTop: 4,
},
});

export default styleSheet;
Loading
Loading