Skip to content

Commit

Permalink
feat: added duplicate contact warning icon in contact list
Browse files Browse the repository at this point in the history
  • Loading branch information
Matt561 committed Nov 15, 2024
1 parent f1351c6 commit 60d7635
Show file tree
Hide file tree
Showing 10 changed files with 220 additions and 20 deletions.
3 changes: 3 additions & 0 deletions app/_locales/en/messages.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

41 changes: 33 additions & 8 deletions ui/components/app/contact-list/contact-list.component.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import { sortBy } from 'lodash';
import Button from '../../ui/button';
import { BannerAlert, BannerAlertSeverity } from '../../component-library';
import RecipientGroup from './recipient-group/recipient-group.component';
import { hasDuplicateContacts } from './utils';
import { hasDuplicateContacts, buildDuplicateContactMap } from './utils';

export default class ContactList extends PureComponent {
static propTypes = {
Expand Down Expand Up @@ -62,15 +62,40 @@ export default class ContactList extends PureComponent {
}

renderAddressBook() {
const unsortedContactsByLetter = this.props
.searchForContacts()
.reduce((obj, contact) => {
const {
addressBook,
internalAccounts,
searchForContacts,
selectRecipient,
selectedAddress,
} = this.props;

const duplicateContactMap = buildDuplicateContactMap(
addressBook,
internalAccounts,
);

const unsortedContactsByLetter = searchForContacts().reduce(
(obj, contact) => {
const firstLetter = contact.name[0].toUpperCase();

const isDuplicate =
(duplicateContactMap.get(contact.name.trim().toLowerCase()) ?? [])
.length > 1;

return {
...obj,
[firstLetter]: [...(obj[firstLetter] || []), contact],
[firstLetter]: [
...(obj[firstLetter] || []),
{
...contact,
isDuplicate,
},
],
};
}, {});
},
{},
);

const letters = Object.keys(unsortedContactsByLetter).sort();

Expand All @@ -88,8 +113,8 @@ export default class ContactList extends PureComponent {
key={`${letter}-contact-group`}
label={letter}
items={groupItems}
onSelect={this.props.selectRecipient}
selectedAddress={this.props.selectedAddress}
onSelect={selectRecipient}
selectedAddress={selectedAddress}
/>
));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,13 @@ export default function RecipientGroup({ items, onSelect }) {
return null;
}

return items.map(({ address, name }) => (
return items.map(({ address, name, isDuplicate }) => (
<AddressListItem
address={address}
label={name}
onClick={() => onSelect(address, name)}
key={address}
isDuplicate={isDuplicate}
/>
));
}
Expand Down
27 changes: 26 additions & 1 deletion ui/components/app/contact-list/utils.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,31 @@
import { AddressBookEntry } from '@metamask/address-book-controller';
import { InternalAccount } from '@metamask/keyring-api';

export const buildDuplicateContactMap = (
addressBook: AddressBookEntry[],
internalAccounts: InternalAccount[],
) => {
const contactMap = new Map<string, string[]>(
internalAccounts.map((account) => [
account.metadata.name.trim().toLowerCase(),
[`account-id-${account.id}`],
]),
);

addressBook.forEach((entry) => {
const { name, address } = entry;

const sanitizedName = name.trim().toLowerCase();

const currentArray = contactMap.get(sanitizedName) ?? [];
currentArray.push(address);

contactMap.set(sanitizedName, currentArray);
});

return contactMap;
};

export const hasDuplicateContacts = (
addressBook: AddressBookEntry[],
internalAccounts: InternalAccount[],
Expand Down Expand Up @@ -32,5 +57,5 @@ export const isDuplicateContact = (
metadata.name.toLowerCase().trim() === newName.toLowerCase().trim(),
);

return !nameExistsInAddressBook && !nameExistsInAccountList;
return nameExistsInAddressBook || nameExistsInAccountList;
};
Original file line number Diff line number Diff line change
@@ -1,6 +1,106 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP

exports[`AddressListItem renders the address and label 1`] = `
exports[`AddressListItem displays duplicate contact warning icon 1`] = `
<div>
<button
class="mm-box address-list-item mm-box--padding-4 mm-box--display-flex mm-box--align-items-center mm-box--width-full mm-box--background-color-transparent"
>
<div
class="mm-box mm-text mm-avatar-base mm-avatar-base--size-md mm-avatar-account mm-text--body-sm mm-text--text-transform-uppercase mm-box--margin-inline-end-2 mm-box--display-flex mm-box--justify-content-center mm-box--align-items-center mm-box--color-text-default mm-box--background-color-background-alternative mm-box--rounded-full mm-box--border-color-transparent box--border-style-solid box--border-width-1"
>
<div
class="mm-avatar-account__jazzicon"
>
<div
style="border-radius: 50px; overflow: hidden; padding: 0px; margin: 0px; width: 32px; height: 32px; display: inline-block; background: rgb(3, 73, 94);"
>
<svg
height="32"
width="32"
x="0"
y="0"
>
<rect
fill="#F5D800"
height="32"
transform="translate(-2.259751642128839 9.986727856309313) rotate(238.2 16 16)"
width="32"
x="0"
y="0"
/>
<rect
fill="#1888F2"
height="32"
transform="translate(3.5983407434334205 13.963394795558013) rotate(211.2 16 16)"
width="32"
x="0"
y="0"
/>
<rect
fill="#017E8E"
height="32"
transform="translate(5.504819407037733 -22.91863393981918) rotate(404.9 16 16)"
width="32"
x="0"
y="0"
/>
</svg>
</div>
</div>
</div>
<div
class="mm-box mm-box--display-flex mm-box--flex-direction-column"
style="overflow: hidden;"
>
<p
class="mm-box mm-text address-list-item__label mm-text--body-md-medium mm-text--ellipsis mm-text--text-align-left mm-box--padding-0 mm-box--width-full mm-box--color-text-default"
data-testid="address-list-item-label"
style="overflow: hidden;"
>
metamask.eth
</p>
<div
class="mm-box mm-text mm-text--body-sm mm-text--ellipsis mm-box--display-flex mm-box--color-text-alternative"
data-testid="address-list-item-address"
>
<div>
<div
aria-describedby="tippy-tooltip-7"
class=""
data-original-title="0x0c54FcCd2e384b4BB6f2E405Bf5Cbc15a017AaFb"
data-tooltipped=""
style="display: inline;"
tabindex="0"
>
0x0c54F...7AaFb
</div>
</div>
</div>
</div>
<div
class="mm-box address-list-item__duplicate-contact-warning-icon"
>
<div>
<div
aria-describedby="tippy-tooltip-8"
class=""
data-original-title="This contact name collides with an existing account or contact"
data-tooltipped=""
style="display: inline;"
tabindex="0"
>
<span
class="mm-box mm-icon mm-icon--size-md mm-box--display-inline-block mm-box--color-warning-default"
style="mask-image: url('./images/icons/danger.svg');"
/>
</div>
</div>
</div>
</button>
</div>
`;

exports[`AddressListItem renders the address and label without duplicate contact warning icon 1`] = `
<div>
<button
class="mm-box address-list-item mm-box--padding-4 mm-box--display-flex mm-box--align-items-center mm-box--width-full mm-box--background-color-transparent"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,35 +11,50 @@ const SAMPLE_LABEL = 'metamask.eth';

const mockOnClick = jest.fn();

const render = (label = '', useConfusable = false) => {
type Options = {
label?: string;
useConfusable?: boolean;
isDuplicate?: boolean;
};

const render = (options?: Options) => {
return renderWithProvider(
<AddressListItem
address={SAMPLE_ADDRESS}
label={label || SAMPLE_LABEL}
useConfusable={useConfusable}
label={options?.label || SAMPLE_LABEL}
useConfusable={options?.useConfusable}
onClick={mockOnClick}
isDuplicate={options?.isDuplicate}
/>,
configureStore(mockState),
);
};

describe('AddressListItem', () => {
it('renders the address and label', () => {
it('renders the address and label without duplicate contact warning icon', () => {
const { getByText, container } = render();
expect(container).toMatchSnapshot();

expect(getByText(shortenAddress(SAMPLE_ADDRESS))).toBeInTheDocument();
expect(
document.querySelector(
'.address-list-item__duplicate-contact-warning-icon',
),
).not.toBeInTheDocument();
});

it('uses a confusable when it should', () => {
const { container } = render('metamask.eth', true);
const { container } = render({
label: 'metamask.eth',
useConfusable: true,
});
expect(container).toMatchSnapshot();

expect(document.querySelector('.confusable__point')).toBeInTheDocument();
});

it('does not force red text when unnecessary', () => {
render('metamask.eth');
render({ label: 'metamask.eth' });
expect(
document.querySelector('.confusable__point'),
).not.toBeInTheDocument();
Expand All @@ -52,4 +67,15 @@ describe('AddressListItem', () => {

expect(mockOnClick).toHaveBeenCalled();
});

it('displays duplicate contact warning icon', () => {
const { container } = render({ isDuplicate: true });

expect(container).toMatchSnapshot();
expect(
document.querySelector(
'.address-list-item__duplicate-contact-warning-icon',
),
).toBeInTheDocument();
});
});
17 changes: 16 additions & 1 deletion ui/components/multichain/address-list-item/address-list-item.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import React from 'react';
import React, { useContext } from 'react';
import { useSelector } from 'react-redux';
import Confusable from '../../ui/confusable';
import {
Expand All @@ -7,6 +7,8 @@ import {
AvatarAccountVariant,
Text,
AvatarAccountSize,
Icon,
IconName,
} from '../../component-library';
import {
TextAlign,
Expand All @@ -18,24 +20,30 @@ import {
BackgroundColor,
TextColor,
AlignItems,
IconColor,
} from '../../../helpers/constants/design-system';
import { getUseBlockie } from '../../../selectors';
import { shortenAddress } from '../../../helpers/utils/util';
import Tooltip from '../../ui/tooltip';
import { I18nContext } from '../../../contexts/i18n';

type AddressListItemProps = {
address: string;
label: string;
useConfusable?: boolean;
isDuplicate?: boolean;
onClick: () => void;
};

export const AddressListItem = ({
address,
label,
useConfusable = false,
isDuplicate = false,
onClick,
}: AddressListItemProps) => {
const t = useContext(I18nContext);

const useBlockie = useSelector(getUseBlockie);
let displayName: string | React.ReactNode = shortenAddress(address);
if (label) {
Expand Down Expand Up @@ -100,6 +108,13 @@ export const AddressListItem = ({
</Tooltip>
</Text>
</Box>
{isDuplicate && (
<Box className="address-list-item__duplicate-contact-warning-icon">
<Tooltip title={t('duplicateContactTooltip')} position="top">
<Icon name={IconName.Danger} color={IconColor.warningDefault} />
</Tooltip>
</Box>
)}
</Box>
);
};
5 changes: 5 additions & 0 deletions ui/components/multichain/address-list-item/index.scss
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,11 @@
font-size: 14px !important;
}

&__duplicate-contact-warning-icon {
margin-left: auto;
align-self: center;
}

&:hover,
&:focus-within {
background: var(--color-background-default-hover);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ export default class AddContact extends PureComponent {

validateName = (newName) => {
const { addressBook, internalAccounts } = this.props;
return isDuplicateContact(addressBook, internalAccounts, newName);
return !isDuplicateContact(addressBook, internalAccounts, newName);
};

handleNameChange = (newName) => {
Expand Down
Loading

0 comments on commit 60d7635

Please sign in to comment.