Skip to content

Commit

Permalink
fix: enable Save button on Add Contact page for address input (#26155)
Browse files Browse the repository at this point in the history
<!--
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?
-->

This patches an issue in `add-contact.component.js` where the disabled
state of the `Save` button would disappear only after a successful ENS
resolution, effectively preventing plain addresses to be entered.
I also added some extra unit tests to check for some of the cases that
weren't covered before.

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

## **Related issues**

fixes #25918
fixes #25889

## **Manual testing steps**

1. Go to Settings > Contacts > Add Contact
2. Enter a name and a valid Ethereum address
3. Observe the Save button getting enabled
4. Edit the address to make it invalid, observe the Save button getting
disabled
5. Instead of an address, enter an ENS name and pick the resolved result
6. Observe the Save button getting enabled
7. Observe that the input is still editable (repeat 4.)

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

- [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
- [ ] 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.

---------

Signed-off-by: Mircea Nistor <mirceanis@gmail.com>
  • Loading branch information
mirceanis authored and dawnseeker8 committed Aug 12, 2024
1 parent 7e3d902 commit d96bc84
Show file tree
Hide file tree
Showing 2 changed files with 56 additions and 12 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -3,15 +3,15 @@ import PropTypes from 'prop-types';
import { debounce } from 'lodash';
import TextField from '../../../../components/ui/text-field';
import { CONTACT_LIST_ROUTE } from '../../../../helpers/constants/routes';
import { IS_FLASK, isValidDomainName } from '../../../../helpers/utils/util';
import { isValidDomainName } from '../../../../helpers/utils/util';
import DomainInput from '../../../confirmations/send/send-content/add-recipient/domain-input';
import PageContainerFooter from '../../../../components/ui/page-container/page-container-footer';
import {
isBurnAddress,
isValidHexAddress,
} from '../../../../../shared/modules/hexstring-utils';
import { INVALID_RECIPIENT_ADDRESS_ERROR } from '../../../confirmations/send/send.constants';
import { DomainInputResolutionCell } from '../../../../components/multichain/pages/send/components/domain-input-resolution-cell';
import { DomainInputResolutionCell } from '../../../../components/multichain/pages/send/components';

export default class AddContact extends PureComponent {
static contextTypes = {
Expand Down Expand Up @@ -68,8 +68,10 @@ export default class AddContact extends PureComponent {
isValidHexAddress(input, { mixedCaseUseChecksum: true });
const validEnsAddress = isValidDomainName(input);

if (!IS_FLASK && !validEnsAddress && !valid) {
if (!validEnsAddress && !valid) {
this.setState({ error: INVALID_RECIPIENT_ADDRESS_ERROR });
} else {
this.setState({ error: null });
}
};

Expand Down Expand Up @@ -104,6 +106,10 @@ export default class AddContact extends PureComponent {
this.props;

const errorToRender = domainError || this.state.error;
const newAddress = this.state.selectedAddress || this.state.input;
const validAddress =
!isBurnAddress(newAddress) &&
isValidHexAddress(newAddress, { mixedCaseUseChecksum: true });

return (
<div className="settings-page__content-row address-book__add-contact">
Expand Down Expand Up @@ -150,7 +156,7 @@ export default class AddContact extends PureComponent {
domainName={addressBookEntryName ?? domainName}
onClick={() => {
this.setState({
selectedAddress: resolvedAddress,
input: resolvedAddress,
newName: this.state.newName || domainName,
});
this.props.resetDomainResolution();
Expand All @@ -171,15 +177,10 @@ export default class AddContact extends PureComponent {
<PageContainerFooter
cancelText={this.context.t('cancel')}
disabled={Boolean(
this.state.error ||
!this.state.selectedAddress ||
!this.state.newName.trim(),
this.state.error || !validAddress || !this.state.newName.trim(),
)}
onSubmit={async () => {
await addToAddressBook(
this.state.selectedAddress,
this.state.newName,
);
await addToAddressBook(newAddress, this.state.newName);
history.push(CONTACT_LIST_ROUTE);
}}
onCancel={() => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ describe('AddContact component', () => {
fireEvent.change(input, { target: { value: 'invalid address' } });
setTimeout(() => {
expect(getByText('Recipient address is invalid')).toBeInTheDocument();
}, 100);
}, 600);
});

it('should get disabled submit button when username field is empty', () => {
Expand All @@ -60,4 +60,47 @@ describe('AddContact component', () => {
const saveButton = getByText('Save');
expect(saveButton).toBeDisabled();
});

it('should enable submit button when input is valid', () => {
const store = configureMockStore(middleware)(state);
const { getByText, getByTestId } = renderWithProvider(
<AddContact {...props} />,
store,
);

const nameInput = document.getElementById('nickname');
fireEvent.change(nameInput, { target: { value: 'friend' } });

const addressInput = getByTestId('ens-input');
fireEvent.change(addressInput, {
target: { value: '0x1234Bf0BBa69C63E2657cF94693cC4A907085678' },
});

const saveButton = getByText('Save');
expect(saveButton).not.toBeDisabled();
});

it('should disable submit button when input is not a valid address', () => {
const store = configureMockStore(middleware)(state);
const { getByText, getByTestId } = renderWithProvider(
<AddContact {...props} />,
store,
);

const nameInput = document.getElementById('nickname');
fireEvent.change(nameInput, { target: { value: 'friend' } });

const addressInput = getByTestId('ens-input');
fireEvent.change(addressInput, {
// invalid length
target: { value: '0x1234' },
});
expect(getByText('Save')).toBeDisabled();

fireEvent.change(addressInput, {
// wrong checksum
target: { value: '0x1234bf0bba69C63E2657cF94693cC4A907085678' },
});
expect(getByText('Save')).toBeDisabled();
});
});

0 comments on commit d96bc84

Please sign in to comment.