From f5ec16c65af201a59116e1821ecd343e22811277 Mon Sep 17 00:00:00 2001 From: Mark Stacey Date: Fri, 5 Jun 2020 17:24:51 -0300 Subject: [PATCH] Fix account menu entry for imported accounts (#8747) The entry for imported accounts in the account menu looked wrong with the new connected site icon - there was no padding between the site icon and the 'imported' label. The entry was pretty crowded with these three symbols as well (the third being the 'x' used to remove the account). The site icon has been made the right-most icon, so that it lines up with the site icons shown for other accounts, and spacing has been added between the site icon and the 'imported' label. The 'x' used to remove accounts has been removed. Accounts can still be removed from the 'Account Options' menu on the Home screen. This seemed like the wrong place for this button to exist, as it's the only action that can be taken from that menu aside from navigation. --- test/e2e/from-import-ui.spec.js | 14 +++++-- .../account-menu/account-menu.component.js | 37 +------------------ .../account-menu/account-menu.container.js | 4 -- ui/app/components/app/account-menu/index.scss | 1 + .../account-menu/tests/account-menu.test.js | 13 ------- .../app/menu-bar/account-options-menu.js | 1 + 6 files changed, 13 insertions(+), 57 deletions(-) diff --git a/test/e2e/from-import-ui.spec.js b/test/e2e/from-import-ui.spec.js index 6771dfd6b..b65786629 100644 --- a/test/e2e/from-import-ui.spec.js +++ b/test/e2e/from-import-ui.spec.js @@ -3,7 +3,6 @@ const webdriver = require('selenium-webdriver') const { By, Key, until } = webdriver const { - tinyDelayMs, regularDelayMs, largeDelayMs, } = require('./helpers') @@ -278,7 +277,7 @@ describe('Using MetaMask with an existing account', function () { await driver.delay(regularDelayMs) }) - it('should open the remove account modal', async function () { + it('should see new account in account menu', async function () { const accountName = await driver.findElement(By.css('.selected-account__name')) assert.equal(await accountName.getText(), 'Account 5') await driver.delay(regularDelayMs) @@ -289,8 +288,13 @@ describe('Using MetaMask with an existing account', function () { const accountListItems = await driver.findElements(By.css('.account-menu__account')) assert.equal(accountListItems.length, 5) - await driver.clickElement(By.css('.account-menu__account:last-of-type > .remove-account-icon')) - await driver.delay(tinyDelayMs) + await driver.clickPoint(By.css('.account-menu__icon'), 0, 0) + }) + + it('should open the remove account modal', async function () { + await driver.clickElement(By.css('[data-testid="account-options-menu-button"]')) + + await driver.clickElement(By.css('[data-testid="account-options-menu__remove-account"]')) await driver.findElement(By.css('.confirm-remove-account__account')) }) @@ -304,6 +308,8 @@ describe('Using MetaMask with an existing account', function () { assert.equal(await accountName.getText(), 'Account 1') await driver.delay(regularDelayMs) + await driver.clickElement(By.css('.account-menu__icon')) + const accountListItems = await driver.findElements(By.css('.account-menu__account')) assert.equal(accountListItems.length, 4) }) diff --git a/ui/app/components/app/account-menu/account-menu.component.js b/ui/app/components/app/account-menu/account-menu.component.js index fb9ac9ac6..f6245f304 100644 --- a/ui/app/components/app/account-menu/account-menu.component.js +++ b/ui/app/components/app/account-menu/account-menu.component.js @@ -7,7 +7,6 @@ import InputAdornment from '@material-ui/core/InputAdornment' import { Menu, Item, Divider, CloseArea } from '../dropdowns/components/menu' import { ENVIRONMENT_TYPE_POPUP } from '../../../../../app/scripts/lib/enums' import { getEnvironmentType } from '../../../../../app/scripts/lib/util' -import Tooltip from '../../ui/tooltip' import Identicon from '../../ui/identicon' import IconWithFallBack from '../../ui/icon-with-fallback' import UserPreferencedCurrencyDisplay from '../user-preferenced-currency-display' @@ -38,7 +37,6 @@ export default class AccountMenu extends Component { lockMetamask: PropTypes.func, selectedAddress: PropTypes.string, showAccountDetail: PropTypes.func, - showRemoveAccountConfirmationModal: PropTypes.func, toggleAccountMenu: PropTypes.func, addressConnectedDomainMap: PropTypes.object, originOfCurrentTab: PropTypes.string, @@ -176,6 +174,7 @@ export default class AccountMenu extends Component { type={PRIMARY} /> + { this.renderKeyringType(keyring) } { iconAndNameForOpenDomain ? (
@@ -184,45 +183,11 @@ export default class AccountMenu extends Component { ) : null } - { this.renderKeyringType(keyring) } - { this.renderRemoveAccount(keyring, identity) }
) }) } - renderRemoveAccount (keyring, identity) { - const { t } = this.context - - // Sometimes keyrings aren't loaded yet - if (!keyring) { - return null - } - - // Any account that's not from the HD wallet Keyring can be removed - const { type } = keyring - const isRemovable = type !== 'HD Key Tree' - - return isRemovable && ( - - this.removeAccount(e, identity)} - /> - - ) - } - - removeAccount (e, identity) { - e.preventDefault() - e.stopPropagation() - const { showRemoveAccountConfirmationModal } = this.props - showRemoveAccountConfirmationModal(identity) - } - renderKeyringType (keyring) { const { t } = this.context diff --git a/ui/app/components/app/account-menu/account-menu.container.js b/ui/app/components/app/account-menu/account-menu.container.js index e6f64335c..a9739dccc 100644 --- a/ui/app/components/app/account-menu/account-menu.container.js +++ b/ui/app/components/app/account-menu/account-menu.container.js @@ -7,7 +7,6 @@ import { hideSidebar, lockMetamask, hideWarning, - showModal, } from '../../../store/actions' import { getAddressConnectedDomainMap, @@ -54,9 +53,6 @@ function mapDispatchToProps (dispatch) { dispatch(hideSidebar()) dispatch(toggleAccountMenu()) }, - showRemoveAccountConfirmationModal: (identity) => { - return dispatch(showModal({ name: 'CONFIRM_REMOVE_ACCOUNT', identity })) - }, } } diff --git a/ui/app/components/app/account-menu/index.scss b/ui/app/components/app/account-menu/index.scss index 62f76e10c..95755f2d0 100644 --- a/ui/app/components/app/account-menu/index.scss +++ b/ui/app/components/app/account-menu/index.scss @@ -78,6 +78,7 @@ .keyring-label { margin-top: 5px; + margin-right: 10px; background-color: $dusty-gray; color: $black; font-weight: normal; diff --git a/ui/app/components/app/account-menu/tests/account-menu.test.js b/ui/app/components/app/account-menu/tests/account-menu.test.js index 9d03c7616..3ed2005b1 100644 --- a/ui/app/components/app/account-menu/tests/account-menu.test.js +++ b/ui/app/components/app/account-menu/tests/account-menu.test.js @@ -99,19 +99,6 @@ describe('Account Menu', function () { const importedAccount = wrapper.find('.keyring-label.allcaps') assert.equal(importedAccount.text(), 'imported') }) - - it('remove account', function () { - const removeAccount = wrapper.find('.remove-account-icon') - removeAccount.simulate('click', { - preventDefault: () => {}, - stopPropagation: () => {}, - }) - - assert(props.showRemoveAccountConfirmationModal.calledOnce) - assert.deepEqual(props.showRemoveAccountConfirmationModal.getCall(0).args[0], - { address: '0xImportedAddress', balance: '0x0', name: 'Imported Account 1' } - ) - }) }) describe('Log Out', function () { diff --git a/ui/app/components/app/menu-bar/account-options-menu.js b/ui/app/components/app/menu-bar/account-options-menu.js index 6158c51a8..c87416cdb 100644 --- a/ui/app/components/app/menu-bar/account-options-menu.js +++ b/ui/app/components/app/menu-bar/account-options-menu.js @@ -117,6 +117,7 @@ export default function AccountOptionsMenu ({ anchorElement, onClose }) { isRemovable ? ( { dispatch(showModal({ name: 'CONFIRM_REMOVE_ACCOUNT', identity: selectedIdentity })) onClose()