1
0
mirror of https://github.com/kremalicious/metamask-extension.git synced 2024-11-22 18:00:18 +01:00

Fix account name duplicates (#12867)

* Fix account name duplicates

* Change button disabling logic

* Fix function error

* Requested changes

* Move functions from selectors file into components

* Applying requested changes
This commit is contained in:
filipsekulic 2021-12-16 11:41:55 +01:00 committed by GitHub
parent f946c030b5
commit 62f41600f2
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
9 changed files with 107 additions and 9 deletions

View File

@ -73,6 +73,10 @@
"accountName": { "accountName": {
"message": "Account Name" "message": "Account Name"
}, },
"accountNameDuplicate": {
"message": "This account name already exists",
"description": "This is an error message shown when the user enters a new account name that matches an existing account name"
},
"accountOptions": { "accountOptions": {
"message": "Account Options" "message": "Account Options"
}, },

View File

@ -17,6 +17,7 @@ export default class AccountDetailsModal extends Component {
setAccountLabel: PropTypes.func, setAccountLabel: PropTypes.func,
keyrings: PropTypes.array, keyrings: PropTypes.array,
rpcPrefs: PropTypes.object, rpcPrefs: PropTypes.object,
accounts: PropTypes.array,
}; };
static contextTypes = { static contextTypes = {
@ -32,6 +33,7 @@ export default class AccountDetailsModal extends Component {
setAccountLabel, setAccountLabel,
keyrings, keyrings,
rpcPrefs, rpcPrefs,
accounts,
} = this.props; } = this.props;
const { name, address } = selectedIdentity; const { name, address } = selectedIdentity;
@ -39,6 +41,12 @@ export default class AccountDetailsModal extends Component {
return kr.accounts.includes(address); return kr.accounts.includes(address);
}); });
const getAccountsNames = (allAccounts, currentName) => {
return Object.values(allAccounts)
.map((item) => item.name)
.filter((itemName) => itemName !== currentName);
};
let exportPrivateKeyFeatureEnabled = true; let exportPrivateKeyFeatureEnabled = true;
// This feature is disabled for hardware wallets // This feature is disabled for hardware wallets
if (isHardwareKeyring(keyring?.type)) { if (isHardwareKeyring(keyring?.type)) {
@ -51,6 +59,7 @@ export default class AccountDetailsModal extends Component {
className="account-details-modal__name" className="account-details-modal__name"
defaultValue={name} defaultValue={name}
onSubmit={(label) => setAccountLabel(address, label)} onSubmit={(label) => setAccountLabel(address, label)}
accountsNames={getAccountsNames(accounts, name)}
/> />
<QrView <QrView

View File

@ -4,6 +4,7 @@ import {
getSelectedIdentity, getSelectedIdentity,
getRpcPrefsForCurrentProvider, getRpcPrefsForCurrentProvider,
getCurrentChainId, getCurrentChainId,
getMetaMaskAccountsOrdered,
} from '../../../../selectors'; } from '../../../../selectors';
import AccountDetailsModal from './account-details-modal.component'; import AccountDetailsModal from './account-details-modal.component';
@ -13,6 +14,7 @@ const mapStateToProps = (state) => {
selectedIdentity: getSelectedIdentity(state), selectedIdentity: getSelectedIdentity(state),
keyrings: state.metamask.keyrings, keyrings: state.metamask.keyrings,
rpcPrefs: getRpcPrefsForCurrentProvider(state), rpcPrefs: getRpcPrefsForCurrentProvider(state),
accounts: getMetaMaskAccountsOrdered(state),
}; };
}; };

View File

@ -30,6 +30,12 @@ describe('Account Details Modal', () => {
name: 'Account 1', name: 'Account 1',
}, },
}, },
accounts: {
address: '0xAddress',
lastSelected: 1637764711510,
name: 'Account 1',
balance: '0x543a5fb6caccf599',
},
}; };
beforeEach(() => { beforeEach(() => {

View File

@ -7,6 +7,11 @@ class EditableLabel extends Component {
onSubmit: PropTypes.func.isRequired, onSubmit: PropTypes.func.isRequired,
defaultValue: PropTypes.string, defaultValue: PropTypes.string,
className: PropTypes.string, className: PropTypes.string,
accountsNames: PropTypes.array,
};
static contextTypes = {
t: PropTypes.func,
}; };
state = { state = {
@ -16,8 +21,9 @@ class EditableLabel extends Component {
handleSubmit() { handleSubmit() {
const { value } = this.state; const { value } = this.state;
const { accountsNames } = this.props;
if (value === '') { if (value === '' || accountsNames.includes(value)) {
return; return;
} }
@ -28,6 +34,7 @@ class EditableLabel extends Component {
renderEditing() { renderEditing() {
const { value } = this.state; const { value } = this.state;
const { accountsNames } = this.props;
return [ return [
<input <input
@ -43,7 +50,8 @@ class EditableLabel extends Component {
}} }}
onChange={(event) => this.setState({ value: event.target.value })} onChange={(event) => this.setState({ value: event.target.value })}
className={classnames('large-input', 'editable-label__input', { className={classnames('large-input', 'editable-label__input', {
'editable-label__input--error': value === '', 'editable-label__input--error':
value === '' || accountsNames.includes(value),
})} })}
autoFocus autoFocus
/>, />,
@ -73,13 +81,25 @@ class EditableLabel extends Component {
} }
render() { render() {
const { isEditing } = this.state; const { isEditing, value } = this.state;
const { className } = this.props; const { className, accountsNames } = this.props;
return ( return (
<div className={classnames('editable-label', className)}> <>
{isEditing ? this.renderEditing() : this.renderReadonly()} <div className={classnames('editable-label', className)}>
</div> {isEditing ? this.renderEditing() : this.renderReadonly()}
</div>
{accountsNames.includes(value) ? (
<div
className={classnames(
'editable-label__error',
'editable-label__error-amount',
)}
>
{this.context.t('accountNameDuplicate')}
</div>
) : null}
</>
); );
} }
} }

View File

@ -34,4 +34,15 @@
cursor: pointer; cursor: pointer;
color: $dusty-gray; color: $dusty-gray;
} }
&__error {
@include H7;
left: 8px;
color: $red;
}
&__error-amount {
margin-top: 5px;
}
} }

View File

@ -87,6 +87,21 @@
color: $scorpion; color: $scorpion;
margin-top: 15px; margin-top: 15px;
padding: 0 20px; padding: 0 20px;
&__error {
border: 1px solid $error-3;
}
}
&__error {
@include H7;
left: 8px;
color: $red;
}
&__error-amount {
margin-top: 5px;
} }
&__buttons { &__buttons {

View File

@ -1,5 +1,6 @@
import React, { Component } from 'react'; import React, { Component } from 'react';
import PropTypes from 'prop-types'; import PropTypes from 'prop-types';
import classnames from 'classnames';
import Button from '../../components/ui/button'; import Button from '../../components/ui/button';
export default class NewAccountCreateForm extends Component { export default class NewAccountCreateForm extends Component {
@ -16,7 +17,13 @@ export default class NewAccountCreateForm extends Component {
render() { render() {
const { newAccountName, defaultAccountName } = this.state; const { newAccountName, defaultAccountName } = this.state;
const { history, createAccount, mostRecentOverviewPage } = this.props; const {
history,
createAccount,
mostRecentOverviewPage,
accounts,
} = this.props;
const createClick = (_) => { const createClick = (_) => {
createAccount(newAccountName || defaultAccountName) createAccount(newAccountName || defaultAccountName)
.then(() => { .then(() => {
@ -43,6 +50,14 @@ export default class NewAccountCreateForm extends Component {
}); });
}; };
const accountNameExists = (allAccounts, accountName) => {
const accountsNames = allAccounts.map((item) => item.name);
return accountsNames.includes(accountName);
};
const existingAccountName = accountNameExists(accounts, newAccountName);
return ( return (
<div className="new-account-create-form"> <div className="new-account-create-form">
<div className="new-account-create-form__input-label"> <div className="new-account-create-form__input-label">
@ -50,7 +65,9 @@ export default class NewAccountCreateForm extends Component {
</div> </div>
<div> <div>
<input <input
className="new-account-create-form__input" className={classnames('new-account-create-form__input', {
'new-account-create-form__input__error': existingAccountName,
})}
value={newAccountName} value={newAccountName}
placeholder={defaultAccountName} placeholder={defaultAccountName}
onChange={(event) => onChange={(event) =>
@ -58,6 +75,16 @@ export default class NewAccountCreateForm extends Component {
} }
autoFocus autoFocus
/> />
{existingAccountName ? (
<div
className={classnames(
' new-account-create-form__error',
' new-account-create-form__error-amount',
)}
>
{this.context.t('accountNameDuplicate')}
</div>
) : null}
<div className="new-account-create-form__buttons"> <div className="new-account-create-form__buttons">
<Button <Button
type="secondary" type="secondary"
@ -72,6 +99,7 @@ export default class NewAccountCreateForm extends Component {
large large
className="new-account-create-form__button" className="new-account-create-form__button"
onClick={createClick} onClick={createClick}
disabled={existingAccountName}
> >
{this.context.t('create')} {this.context.t('create')}
</Button> </Button>
@ -87,6 +115,7 @@ NewAccountCreateForm.propTypes = {
newAccountNumber: PropTypes.number, newAccountNumber: PropTypes.number,
history: PropTypes.object, history: PropTypes.object,
mostRecentOverviewPage: PropTypes.string.isRequired, mostRecentOverviewPage: PropTypes.string.isRequired,
accounts: PropTypes.array,
}; };
NewAccountCreateForm.contextTypes = { NewAccountCreateForm.contextTypes = {

View File

@ -1,6 +1,7 @@
import { connect } from 'react-redux'; import { connect } from 'react-redux';
import * as actions from '../../store/actions'; import * as actions from '../../store/actions';
import { getMostRecentOverviewPage } from '../../ducks/history/history'; import { getMostRecentOverviewPage } from '../../ducks/history/history';
import { getMetaMaskAccountsOrdered } from '../../selectors';
import NewAccountCreateForm from './new-account.component'; import NewAccountCreateForm from './new-account.component';
const mapStateToProps = (state) => { const mapStateToProps = (state) => {
@ -13,6 +14,7 @@ const mapStateToProps = (state) => {
return { return {
newAccountNumber, newAccountNumber,
mostRecentOverviewPage: getMostRecentOverviewPage(state), mostRecentOverviewPage: getMostRecentOverviewPage(state),
accounts: getMetaMaskAccountsOrdered(state),
}; };
}; };