From ae5d076de98456767a2d2a0624a8f5f28dc75942 Mon Sep 17 00:00:00 2001 From: Jaco Greeff Date: Thu, 3 Sep 2020 22:36:49 +0200 Subject: [PATCH] Display accounts first-and-foremost on all creations (#426) * Display accounts first-and-foremost on all creations * Fix tests --- CHANGELOG.md | 1 + .../src/background/handlers/State.ts | 2 +- .../src/Popup/Authorize/Request.tsx | 20 ++++++--- .../src/Popup/CreateAccount/AccountName.tsx | 14 ++---- .../CreateAccount/CreateAccount.test.tsx | 16 +++---- .../src/Popup/CreateAccount/Mnemonic.tsx | 5 +-- .../src/Popup/CreateAccount/index.tsx | 45 +++++++++++-------- .../src/Popup/Derive/AddressDropdown.tsx | 21 ++++++--- .../src/Popup/Derive/SelectParent.tsx | 22 +++++++-- .../extension-ui/src/Popup/Derive/index.tsx | 42 ++++++++--------- packages/extension-ui/src/Popup/ImportQr.tsx | 10 +++-- .../extension-ui/src/Popup/ImportSeed.tsx | 28 ++++++++---- .../extension-ui/src/Popup/RestoreJson.tsx | 11 +++-- .../src/Popup/Signing/Request.tsx | 1 + .../src/components/MnemonicSeed.tsx | 2 +- packages/extension/public/index.html | 2 +- 16 files changed, 145 insertions(+), 97 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 45822b01..42b78660 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,7 @@ Changes: - Derived accounts uses the parent genesisHash by default (attaches to same chain) - Make import from seed, QR & JSON options available on first-start - Adjust popup width, allowing full display of e.g. addresses +- Always display network selection on all accounts - Handling signing rejections (any order) transparently - Small overall UI and use adjustments - Latest upstream polkadot-js dependencies diff --git a/packages/extension-base/src/background/handlers/State.ts b/packages/extension-base/src/background/handlers/State.ts index 7503fe4a..7332c503 100644 --- a/packages/extension-base/src/background/handlers/State.ts +++ b/packages/extension-base/src/background/handlers/State.ts @@ -65,7 +65,7 @@ const WINDOW_OPTS = { top: 150, type: 'popup', url: chrome.extension.getURL('index.html'), - width: 480 + width: 560 }; function getId (): string { diff --git a/packages/extension-ui/src/Popup/Authorize/Request.tsx b/packages/extension-ui/src/Popup/Authorize/Request.tsx index 5e0c29a8..0f6f6b54 100644 --- a/packages/extension-ui/src/Popup/Authorize/Request.tsx +++ b/packages/extension-ui/src/Popup/Authorize/Request.tsx @@ -5,7 +5,7 @@ import { RequestAuthorizeTab } from '@polkadot/extension-base/background/types'; import { ThemeProps } from '../../types'; -import React, { useContext } from 'react'; +import React, { useCallback, useContext } from 'react'; import styled from 'styled-components'; import { ActionBar, ActionContext, Button, Icon, Link, Warning } from '../../components'; @@ -21,14 +21,20 @@ interface Props { function Request ({ authId, className, isFirst, request: { origin }, url }: Props): React.ReactElement { const onAction = useContext(ActionContext); - const _onApprove = (): Promise => - approveAuthRequest(authId) + + const _onApprove = useCallback( + () => approveAuthRequest(authId) .then(() => onAction()) - .catch((error: Error) => console.error(error)); - const _onReject = (): Promise => - rejectAuthRequest(authId) + .catch((error: Error) => console.error(error)), + [authId, onAction] + ); + + const _onReject = useCallback( + () => rejectAuthRequest(authId) .then(() => onAction()) - .catch((error: Error) => console.error(error)); + .catch((error: Error) => console.error(error)), + [authId, onAction] + ); return (
diff --git a/packages/extension-ui/src/Popup/CreateAccount/AccountName.tsx b/packages/extension-ui/src/Popup/CreateAccount/AccountName.tsx index d10bbebc..d88d1e37 100644 --- a/packages/extension-ui/src/Popup/CreateAccount/AccountName.tsx +++ b/packages/extension-ui/src/Popup/CreateAccount/AccountName.tsx @@ -4,7 +4,7 @@ import React, { useCallback, useState } from 'react'; -import { Address, BackButton, ButtonArea, NextStepButton, VerticalSpace } from '../../components'; +import { BackButton, ButtonArea, NextStepButton, VerticalSpace } from '../../components'; import { Name, Password } from '../../partials'; interface Props { @@ -14,7 +14,7 @@ interface Props { onCreate: (name: string, password: string) => void | Promise; } -function AccountName ({ address, isBusy, onBackClick, onCreate }: Props): React.ReactElement { +function AccountName ({ isBusy, onBackClick, onCreate }: Props): React.ReactElement { const [name, setName] = useState(null); const [password, setPassword] = useState(null); @@ -29,13 +29,7 @@ function AccountName ({ address, isBusy, onBackClick, onCreate }: Props): React. isFocused onChange={setName} /> - {name && } - {name && password && ( -
- )} + @@ -52,4 +46,4 @@ function AccountName ({ address, isBusy, onBackClick, onCreate }: Props): React. ); } -export default AccountName; +export default React.memo(AccountName); diff --git a/packages/extension-ui/src/Popup/CreateAccount/CreateAccount.test.tsx b/packages/extension-ui/src/Popup/CreateAccount/CreateAccount.test.tsx index e04cd450..24e3de83 100644 --- a/packages/extension-ui/src/Popup/CreateAccount/CreateAccount.test.tsx +++ b/packages/extension-ui/src/Popup/CreateAccount/CreateAccount.test.tsx @@ -42,7 +42,7 @@ describe('Create Account', () => { wrapper.update(); }; - const enterName = (name: string): Promise => type(wrapper.find('input'), name); + const enterName = (name: string): Promise => type(wrapper.find('input').first(), name); const password = (password: string) => (): Promise => type(wrapper.find('input[type="password"]').first(), password); const repeat = (password: string) => (): Promise => type(wrapper.find('input[type="password"]').last(), password); @@ -89,20 +89,20 @@ describe('Create Account', () => { it('only account name input is visible at first', () => { expect(wrapper.find(InputWithLabel).find('[data-input-name]').find(Input)).toHaveLength(1); - expect(wrapper.find(InputWithLabel).find('[data-input-password]')).toHaveLength(0); + expect(wrapper.find(InputWithLabel).find('[data-input-password]').find(Input)).toHaveLength(1); expect(wrapper.find(InputWithLabel).find('[data-input-repeat-password]')).toHaveLength(0); expect(wrapper.find('[data-button-action="add new root"] button').prop('disabled')).toBe(true); }); it('input should not be highlighted as error until first interaction', () => { - expect(wrapper.find(Input).prop('withError')).toBe(false); + expect(wrapper.find(InputWithLabel).find('[data-input-name]').find(Input).prop('withError')).toBe(false); }); it('after typing less than 3 characters into name input, password input is not visible', async () => { await enterName('ab'); - expect(wrapper.find(Input).prop('withError')).toBe(true); - expect(wrapper.find('ErrorMessage').text()).toBe('Account name is too short'); - expect(wrapper.find(InputWithLabel).find('[data-input-password]')).toHaveLength(0); + expect(wrapper.find(InputWithLabel).find('[data-input-name]').find(Input).prop('withError')).toBe(true); + expect(wrapper.find('ErrorMessage').first().text()).toBe('Account name is too short'); + expect(wrapper.find(InputWithLabel).find('[data-input-password]').find(Input)).toHaveLength(1); expect(wrapper.find(InputWithLabel).find('[data-input-repeat-password]')).toHaveLength(0); expect(wrapper.find('[data-button-action="add new root"] button').prop('disabled')).toBe(true); }); @@ -110,12 +110,12 @@ describe('Create Account', () => { it('input should keep showing error when something has been typed but then erased', async () => { await enterName('ab'); await enterName(''); - expect(wrapper.find(Input).prop('withError')).toBe(true); + expect(wrapper.find(InputWithLabel).find('[data-input-name]').find(Input).prop('withError')).toBe(true); }); it('after typing 3 characters into name input, first password input is visible', async () => { await enterName('abc'); - expect(wrapper.find(Input).first().prop('withError')).toBe(false); + expect(wrapper.find(InputWithLabel).find('[data-input-name]').find(Input).first().prop('withError')).toBe(false); expect(wrapper.find(InputWithLabel).find('[data-input-password]').find(Input)).toHaveLength(1); expect(wrapper.find(InputWithLabel).find('[data-input-repeat-password]')).toHaveLength(0); expect(wrapper.find('[data-button-action="add new root"] button').prop('disabled')).toBe(true); diff --git a/packages/extension-ui/src/Popup/CreateAccount/Mnemonic.tsx b/packages/extension-ui/src/Popup/CreateAccount/Mnemonic.tsx index 1d6e811a..eb834882 100644 --- a/packages/extension-ui/src/Popup/CreateAccount/Mnemonic.tsx +++ b/packages/extension-ui/src/Popup/CreateAccount/Mnemonic.tsx @@ -33,12 +33,11 @@ function Mnemonic ({ onNextStep, seed }: Props): React.ReactElement { return ( <> - Please write down your wallet’s mnemonic seed and keep it in a safe place.
- Mnemonic seed is used to restore your wallet. Keep it carefully in case you lose your assets. + Please write down your wallet’s mnemonic seed and keep it in a safe place. The mnemonic can be used to restore your wallet. Keep it carefully to not lose your assets. { ); } -export default Mnemonic; +export default React.memo(Mnemonic); diff --git a/packages/extension-ui/src/Popup/CreateAccount/index.tsx b/packages/extension-ui/src/Popup/CreateAccount/index.tsx index cfa82148..ff455708 100644 --- a/packages/extension-ui/src/Popup/CreateAccount/index.tsx +++ b/packages/extension-ui/src/Popup/CreateAccount/index.tsx @@ -3,7 +3,7 @@ // of the Apache-2.0 license. See the LICENSE file for details. import React, { useCallback, useContext, useEffect, useState } from 'react'; -import { ActionContext, Loading } from '../../components'; +import { ActionContext, Address, Loading } from '../../components'; import { createAccountSuri, createSeed } from '../../messaging'; import { HeaderWithSteps } from '../../partials'; import AccountName from './AccountName'; @@ -27,6 +27,7 @@ export default function CreateAccount (): React.ReactElement { // this should always be the case if (name && password && account) { setIsBusy(true); + createAccountSuri(name, password, account.seed) .then(() => onAction('/')) .catch((error: Error): void => { @@ -47,23 +48,31 @@ export default function CreateAccount (): React.ReactElement { step={step} text='Create an account: ' /> - {account && ( - step === 1 - ? ( - - ) - : ( - - ) - )} + +
+
+
+ {account && ( + step === 1 + ? ( + + ) + : ( + + ) + )} + ); } diff --git a/packages/extension-ui/src/Popup/Derive/AddressDropdown.tsx b/packages/extension-ui/src/Popup/Derive/AddressDropdown.tsx index 67fd805b..31d5527a 100644 --- a/packages/extension-ui/src/Popup/Derive/AddressDropdown.tsx +++ b/packages/extension-ui/src/Popup/Derive/AddressDropdown.tsx @@ -12,13 +12,14 @@ import { Address, Svg } from '../../components'; import useOutsideClick from '../../hooks/useOutsideClick'; interface Props { - allAddresses: string[]; + allAddresses: [string, string | null][]; className?: string; onSelect: (address: string) => void; selectedAddress: string; + selectedGenesis: string | null; } -function AddressDropdown ({ allAddresses, className, onSelect, selectedAddress }: Props): React.ReactElement { +function AddressDropdown ({ allAddresses, className, onSelect, selectedAddress, selectedGenesis }: Props): React.ReactElement { const [isDropdownVisible, setDropdownVisible] = useState(false); const ref = useRef(null); @@ -34,16 +35,22 @@ function AddressDropdown ({ allAddresses, className, onSelect, selectedAddress } onClick={_toggleDropdown} ref={ref} > -
+
- {allAddresses.map((address) => ( + {allAddresses.map(([address, genesisHash]) => (
-
+
))} @@ -54,7 +61,7 @@ function AddressDropdown ({ allAddresses, className, onSelect, selectedAddress } const HiddenOptions = styled.div<{ visible: boolean }>` position: absolute; visibility: ${({ visible }): string => (visible ? 'visible' : 'hidden')}; - width: 430px; + width: 510px; z-index: 100; background: ${({ theme }: ThemeProps): string => theme.bodyColor}; max-height: ${({ visible }): string => (visible ? '200px' : '0')}; @@ -77,7 +84,7 @@ export default styled(AddressDropdown)(({ theme }: ThemeProps) => ` & > div:first-child > ${Address}::after { content: ''; position: absolute; - top: 50%; + top: 66%; transform: translateY(-50%); right: 11px; width: 30px; diff --git a/packages/extension-ui/src/Popup/Derive/SelectParent.tsx b/packages/extension-ui/src/Popup/Derive/SelectParent.tsx index 7044193a..f8fefe4b 100644 --- a/packages/extension-ui/src/Popup/Derive/SelectParent.tsx +++ b/packages/extension-ui/src/Popup/Derive/SelectParent.tsx @@ -2,7 +2,7 @@ // This software may be modified and distributed under the terms // of the Apache-2.0 license. See the LICENSE file for details. -import React, { useCallback, useContext, useEffect, useRef, useState } from 'react'; +import React, { useCallback, useContext, useEffect, useMemo, useRef, useState } from 'react'; import styled from 'styled-components'; import { assert } from '@polkadot/util'; @@ -15,10 +15,11 @@ import DerivationPath from './DerivationPath'; interface Props { isLocked?: boolean; parentAddress: string; + parentGenesis: string | null; onDerivationConfirmed: (derivation: { account: { address: string; suri: string }; parentPassword: string }) => void; } -export function SelectParent ({ isLocked, onDerivationConfirmed, parentAddress }: Props): React.ReactElement { +export function SelectParent ({ isLocked, onDerivationConfirmed, parentAddress, parentGenesis }: Props): React.ReactElement { const onAction = useContext(ActionContext); const [isBusy, setIsBusy] = useState(false); const { accounts, hierarchy } = useContext(AccountContext); @@ -30,6 +31,13 @@ export function SelectParent ({ isLocked, onDerivationConfirmed, parentAddress } const passwordInputRef = useRef(null); + const allAddresses = useMemo( + () => hierarchy + .filter(({ isExternal }) => !isExternal) + .map(({ address, genesisHash }): [string, string | null] => [address, genesisHash || null]), + [hierarchy] + ); + const _goCreate = useCallback( () => onAction('/account/create'), [onAction] @@ -91,13 +99,19 @@ export function SelectParent ({ isLocked, onDerivationConfirmed, parentAddress } )} {isLocked - ?
+ ? ( +
+ ) : ( ) diff --git a/packages/extension-ui/src/Popup/Derive/index.tsx b/packages/extension-ui/src/Popup/Derive/index.tsx index c08e08ce..2ca35351 100644 --- a/packages/extension-ui/src/Popup/Derive/index.tsx +++ b/packages/extension-ui/src/Popup/Derive/index.tsx @@ -2,7 +2,7 @@ // This software may be modified and distributed under the terms // of the Apache-2.0 license. See the LICENSE file for details. -import React, { useCallback, useContext, useState } from 'react'; +import React, { useCallback, useContext, useMemo, useState } from 'react'; import { useParams } from 'react-router'; import { AccountContext, ActionContext, Address, BackButton, ButtonArea, NextStepButton, VerticalSpace } from '../../components'; @@ -37,22 +37,24 @@ function Derive ({ isLocked }: Props): React.ReactElement { const [password, setPassword] = useState(null); const [parentPassword, setParentPassword] = useState(null); + const parentGenesis = useMemo( + () => accounts.find((a) => a.address === parentAddress)?.genesisHash || null, + [accounts, parentAddress] + ); + const _onCreate = useCallback(() => { if (!account || !name || !password || !parentPassword) { return; } - const parentAccount = accounts.find((a) => a.address === parentAddress); - const genesisHash = parentAccount?.genesisHash || null; - setIsBusy(true); - deriveAccount(parentAddress, account.suri, parentPassword, name, password, genesisHash) + deriveAccount(parentAddress, account.suri, parentPassword, name, password, parentGenesis) .then(() => onAction('/')) .catch((error): void => { setIsBusy(false); console.error(error); }); - }, [account, accounts, name, password, onAction, parentAddress, parentPassword]); + }, [account, name, password, onAction, parentAddress, parentGenesis, parentPassword]); const _onDerivationConfirmed = useCallback(({ account, parentPassword }: ConfirmState) => { setAccount(account); @@ -74,23 +76,23 @@ function Derive ({ isLocked }: Props): React.ReactElement { isLocked={isLocked} onDerivationConfirmed={_onDerivationConfirmed} parentAddress={parentAddress} - /> - )} - {account && ( - - )} - {account && name && } - {account && name && password && ( -
)} {account && ( <> +
+
+
+ + @@ -108,4 +110,4 @@ function Derive ({ isLocked }: Props): React.ReactElement { ); } -export default Derive; +export default React.memo(Derive); diff --git a/packages/extension-ui/src/Popup/ImportQr.tsx b/packages/extension-ui/src/Popup/ImportQr.tsx index 40dee916..fb39a1da 100644 --- a/packages/extension-ui/src/Popup/ImportQr.tsx +++ b/packages/extension-ui/src/Popup/ImportQr.tsx @@ -36,14 +36,16 @@ export default function ImportQr (): React.ReactElement { )} {account && ( <> +
+
+
-
{name && ( diff --git a/packages/extension-ui/src/Popup/ImportSeed.tsx b/packages/extension-ui/src/Popup/ImportSeed.tsx index e5941d4c..c10255f4 100644 --- a/packages/extension-ui/src/Popup/ImportSeed.tsx +++ b/packages/extension-ui/src/Popup/ImportSeed.tsx @@ -40,10 +40,14 @@ export default function Import (): React.ReactElement { const _onChangeSeed = useCallback( async (suri: string | null): Promise => { if (suri) { - setAccount(await validateSeed(suri)); - } else { - setAccount(null); + try { + return setAccount(await validateSeed(suri)); + } catch (error) { + console.error(error); + } } + + setAccount(null); }, [] ); @@ -69,6 +73,12 @@ export default function Import (): React.ReactElement { showBackArrow text='Import account' /> +
+
+
- {account && } - {account && name && } + {account && ( + <> + + + + )} {account && name && password && ( <> -
-
+
+
+
{ setIsLocked(null); + !isExternal && isSignLocked(signId) .then((isLocked) => setIsLocked(isLocked)) .catch((error: Error) => console.error(error)); diff --git a/packages/extension-ui/src/components/MnemonicSeed.tsx b/packages/extension-ui/src/components/MnemonicSeed.tsx index 6d11da83..a28a79f1 100644 --- a/packages/extension-ui/src/components/MnemonicSeed.tsx +++ b/packages/extension-ui/src/components/MnemonicSeed.tsx @@ -40,7 +40,7 @@ function MnemonicSeed ({ className, onCopy, seed }: Props): React.ReactElement

` - margin-top: 21px; + margin-bottom: 21px; .buttonsRow { display: flex; diff --git a/packages/extension/public/index.html b/packages/extension/public/index.html index d86bc8f6..8dc3fb4a 100644 --- a/packages/extension/public/index.html +++ b/packages/extension/public/index.html @@ -5,7 +5,7 @@ polkadot{.js} -

+