Separate derivation and creation flow (#596)

* separate new and derived creation * fix tests
2020-12-24 13:49:42 +01:00 · 2020-12-24 13:49:42 +01:00 · df88d6adab
commit df88d6adab
parent 91c7086357
4 changed files with 84 additions and 140 deletions
--- a/packages/extension-ui/src/Popup/CreateAccount/CreateAccount.test.tsx
+++ b/packages/extension-ui/src/Popup/CreateAccount/CreateAccount.test.tsx
@ -79,6 +79,12 @@ describe('Create Account', () => {
      expect(onActionStub).toBeCalledWith('/');
    });

+    it('checking the checkbox enables the Next button', () => {
+      check(wrapper.find('input[type="checkbox"]'));
+
+      expect(wrapper.find(Button).prop('isDisabled')).toBe(false);
+    });
+
    it('clicking on Next activates phase 2', () => {
      check(wrapper.find('input[type="checkbox"]'));
      wrapper.find('button').simulate('click');
@ -151,14 +157,14 @@ describe('Create Account', () => {
      expect(wrapper.find('[data-button-action="add new root"] button').prop('disabled')).toBe(true);
    });

-    it('submit button is not visible until both passwords are equal', async () => {
+    it('submit button is not enabled until both passwords are equal', async () => {
      await enterName('abc').then(password('abcdef')).then(repeat('abcdeg'));
      expect(wrapper.find('.warning-message').text()).toBe('Passwords do not match');
      expect(wrapper.find(InputWithLabel).find('[data-input-repeat-password]').find(Input).prop('withError')).toBe(true);
      expect(wrapper.find('[data-button-action="add new root"] button').prop('disabled')).toBe(true);
    });

-    it('submit button is visible when both passwords are equal', async () => {
+    it('submit button is enabled when both passwords are equal', async () => {
      await enterName('abc').then(password('abcdef')).then(repeat('abcdef'));
      expect(wrapper.find('.warning-message')).toHaveLength(0);
      expect(wrapper.find(InputWithLabel).find('[data-input-repeat-password]').find(Input).prop('withError')).toBe(false);
@ -189,19 +195,19 @@ describe('Create Account', () => {
      expect(wrapper.find('[data-button-action="add new root"] button').prop('disabled')).toBe(true);
    });

-    it('first password changes - button is not disabled', async () => {
+    it('first password changes - button is disabled', async () => {
      await type(wrapper.find('input[type="password"]').first(), 'aaaaaa');
      expect(wrapper.find('.warning-message').text()).toBe('Passwords do not match');
      expect(wrapper.find('[data-button-action="add new root"] button').prop('disabled')).toBe(true);
    });

-    it('first password changes, then second changes too - button is visible', async () => {
+    it('first password changes, then second changes too - button is enabled', async () => {
      await type(wrapper.find('input[type="password"]').first(), 'aaaaaa');
      await type(wrapper.find('input[type="password"]').last(), 'aaaaaa');
      expect(wrapper.find('[data-button-action="add new root"] button').prop('disabled')).toBe(false);
    });

-    it('second password changes, then first changes too - button is visible', async () => {
+    it('second password changes, then first changes too - button is enabled', async () => {
      await type(wrapper.find('input[type="password"]').last(), 'aaaaaa');
      await type(wrapper.find('input[type="password"]').first(), 'aaaaaa');
      expect(wrapper.find('[data-button-action="add new root"] button').prop('disabled')).toBe(false);
--- a/packages/extension-ui/src/Popup/Derive/Derive.test.tsx
+++ b/packages/extension-ui/src/Popup/Derive/Derive.test.tsx
@ -27,6 +27,7 @@ const accounts = [
  { address: '5GYmFzQCuC5u3tQNiMZNbFGakrz3Jq31NmMg4D2QAkSoQ2g5', name: 'B', type: 'sr25519' },
  { address: '5D2TPhGEy2FhznvzaNYW9AkuMBbg3cyRemnPsBvBY4ZhkZXA', name: 'BB', parentAddress: '5GYmFzQCuC5u3tQNiMZNbFGakrz3Jq31NmMg4D2QAkSoQ2g5', type: 'sr25519' },
  { address: '5GhGENSJBWQZ8d8mARKgqEkiAxiW3hHeznQDW2iG4XzNieb6', isExternal: true, name: 'C', type: 'sr25519' },
+  { address: '0xd5D81CD4236a43F48A983fc5B895975c511f634D', name: 'Ethereum', type: 'ethereum' },
  { address: '5EeaoDj4VDk8V6yQngKBaCD5MpJUCHrhYjVhBjgMHXoYon1s', isExternal: false, name: 'D', type: 'sr25519' },
  { address: '5HRKYp5anSNGtqC7cq9ftiaq4y8Mk7uHk7keaXUrQwZqDWLJ', name: 'DD', parentAddress: '5EeaoDj4VDk8V6yQngKBaCD5MpJUCHrhYjVhBjgMHXoYon1s', type: 'sr25519' }
 ] as AccountJson[];
@ -63,8 +64,6 @@ describe('Derive', () => {
  let wrapper: ReactWrapper;
  let onActionStub: jest.Mock;

-  const uncheck = (input: ReactWrapper): unknown => input.simulate('change', { target: { checked: false } });
-
  const type = async (input: ReactWrapper, value: string): Promise<void> => {
    input.simulate('change', { target: { value } });
    await act(flushAllPromises);
@ -90,48 +89,25 @@ describe('Derive', () => {
      return {} as ResponseDeriveValidate;
    });

-    it('"Derive" checkbox is selected by default', () => {
-      const checkbox = wrapper.find('input[type="checkbox"]');
+    it('Button is disabled and password field visible', () => {
+      const button = wrapper.find('[data-button-action="create derived account"] button');

-      expect(checkbox.prop('checked')).toBe(true);
+      expect(button.exists()).toBe(true);
+      expect(button.prop('disabled')).toBe(true);
    });

-    it('"Create new root" button becomes visible after checkbox is unchecked', () => {
-      uncheck(wrapper.find('input[type="checkbox"]'));
+    it('Password field is visible and not in error state', () => {
+      const passwordField = wrapper.find('[data-input-password]').first();

-      expect(wrapper.exists('[data-button-action="create root account"]')).toBe(true);
-      expect(wrapper.exists('[data-button-action="create derived account"]')).toBe(false);
-    });
-
-    it('Password entry is hidden after checkbox is unchecked', () => {
-      uncheck(wrapper.find('input[type="checkbox"]'));
-
-      expect(wrapper.exists('input[type="password"]')).toBe(false);
-    });
-
-    it('"Create new root" button redirects to /account/create', () => {
-      uncheck(wrapper.find('input[type="checkbox"]'));
-      wrapper.find('[data-button-action="create root account"] button').simulate('click');
-
-      expect(onActionStub).toBeCalledWith('/account/create');
-    });
-
-    it('"Create derived account" is visible when checkbox is checked', () => {
-      expect(wrapper.exists('[data-button-action="create derived account"]')).toBe(true);
-      expect(wrapper.exists('[data-button-action="create root account"]')).toBe(false);
+      expect(passwordField.exists()).toBe(true);
+      expect(passwordField.prop('isError')).toBe(false);
    });

    it('No error is visible when first loading the page', () => {
      expect(wrapper.find('Warning')).toHaveLength(0);
    });

-    it('"Create derived account" is disabled when password is not set', () => {
-      const button = wrapper.find('[data-button-action="create derived account"] button');
-
-      expect(button.prop('disabled')).toBe(true);
-    });
-
-    it('An error is visible and "Create derived account" is disabled when password is incorrect', async () => {
+    it('An error is visible, input higlighted and the button disabled when password is incorrect', async () => {
      await type(wrapper.find('input[type="password"]'), 'wrong_pass');
      wrapper.find('[data-button-action="create derived account"] button').simulate('click');
      await act(flushAllPromises);
@ -140,6 +116,7 @@ describe('Derive', () => {
      const button = wrapper.find('[data-button-action="create derived account"] button');

      expect(button.prop('disabled')).toBe(true);
+      expect(wrapper.find('[data-input-password]').first().prop('isError')).toBe(true);
      expect(wrapper.find('.warning-message')).toHaveLength(1);
      expect(wrapper.find('.warning-message').first().text()).toEqual('Wrong password');
    });
@ -155,10 +132,11 @@ describe('Derive', () => {
      const button = wrapper.find('[data-button-action="create derived account"] button');

      expect(button.prop('disabled')).toBe(false);
+      expect(wrapper.find('[data-input-password]').first().prop('isError')).toBe(false);
      expect(wrapper.find('.warning-message')).toHaveLength(0);
    });

-    it('"Create derived account" is enabled when password is set', async () => {
+    it('Button is enabled when password is set', async () => {
      await type(wrapper.find('input[type="password"]'), 'pass');

      const button = wrapper.find('[data-button-action="create derived account"] button');
@ -167,7 +145,7 @@ describe('Derive', () => {
      expect(wrapper.find('.warning-message')).toHaveLength(0);
    });

-    it('An error is visible and "Create derived account" is disabled when suri is incorrect', async () => {
+    it('An error is visible and the button is disabled when suri is incorrect', async () => {
      await type(wrapper.find('input[type="password"]'), 'pass');
      await type(wrapper.find('[data-input-suri] input'), '//');
      wrapper.find('[data-button-action="create derived account"] button').simulate('click');
@ -199,7 +177,7 @@ describe('Derive', () => {
      expect(wrapper.find('[data-field="name"]').first().text()).toBe('B');
    });

-    it('selects internal root accounts as other options', () => {
+    it('selects internal root accounts as other options, no external and no Ethereum account', () => {
      const options = wrapper.find('[data-parent-option] [data-field="name"]').map((el) => el.text());

      expect(options).toEqual(['A', 'B', 'D']);
@ -217,10 +195,6 @@ describe('Derive', () => {
      wrapper = (await mountComponent(true)).wrapper;
    });

-    it('checkbox does not exist', () => {
-      expect(wrapper.exists('[type="checkbox"]')).toBe(false);
-    });
-
    it('address dropdown does not exist', () => {
      expect(wrapper.exists(AddressDropdown)).toBe(false);
    });
--- a/packages/extension-ui/src/Popup/Derive/SelectParent.tsx
+++ b/packages/extension-ui/src/Popup/Derive/SelectParent.tsx
@ -9,7 +9,7 @@ import styled from 'styled-components';
 import { canDerive } from '@polkadot/extension-base/utils';
 import { assert } from '@polkadot/util';

-import { AccountContext, ActionContext, Address, ButtonArea, Checkbox, InputWithLabel, Label, NextStepButton, VerticalSpace, Warning } from '../../components';
+import { AccountContext, ActionContext, Address, ButtonArea, InputWithLabel, Label, NextStepButton, VerticalSpace, Warning } from '../../components';
 import useTranslation from '../../hooks/useTranslation';
 import { validateAccount, validateDerivationPath } from '../../messaging';
 import { nextDerivationPath } from '../../util/nextDerivationPath';
@ -33,7 +33,6 @@ function SelectParent ({ className, isLocked, onDerivationConfirmed, parentAddre
  const [suriPath, setSuriPath] = useState<null | string>(defaultPath);
  const [parentPassword, setParentPassword] = useState<string>('');
  const [isProperParentPassword, setIsProperParentPassword] = useState(false);
-  const [shouldAccountBeDerived, setShouldAccountBeDerived] = useState(true);
  const passwordInputRef = useRef<HTMLDivElement>(null);

  const allAddresses = useMemo(
@ -44,11 +43,6 @@ function SelectParent ({ className, isLocked, onDerivationConfirmed, parentAddre
    [hierarchy]
  );

-  const _goCreate = useCallback(
-    () => onAction('/account/create'),
-    [onAction]
-  );
-
  const _onParentPasswordEnter = useCallback(
    (parentPassword: string): void => {
      setParentPassword(parentPassword);
@ -98,15 +92,7 @@ function SelectParent ({ className, isLocked, onDerivationConfirmed, parentAddre
  return (
    <>
      <div className={className}>
-        {!isLocked && (
-          <Checkbox
-            checked={shouldAccountBeDerived}
-            className='smallerMargin'
-            label={t<string>('Derive new account from existing')}
-            onChange={setShouldAccountBeDerived}
-          />
-        )}
-        <div className={`disableArea ${shouldAccountBeDerived ? '' : 'disabled'}`}>
+        <div className=''>
          {isLocked
            ? (
              <Address
@ -125,45 +111,41 @@ function SelectParent ({ className, isLocked, onDerivationConfirmed, parentAddre
              </Label>
            )
          }
-          {shouldAccountBeDerived && (
+          <div ref={passwordInputRef}>
+            <InputWithLabel
+              data-input-password
+              isError={!!parentPassword && !isProperParentPassword}
+              isFocused
+              label={t<string>('enter the password for the account you want to derive from')}
+              onChange={_onParentPasswordEnter}
+              type='password'
+              value={parentPassword}
+            />
+            {!!parentPassword && !isProperParentPassword && (
+              <Warning
+                isBelowInput
+                isDanger
+              >
+                {t('Wrong password')}
+              </Warning>
+            )}
+          </div>
+          {isProperParentPassword && (
            <>
-              <div ref={passwordInputRef}>
-                <InputWithLabel
-                  data-export-password
-                  isError={!isProperParentPassword}
-                  isFocused
-                  label={t<string>('enter the password for the account you want to derive from')}
-                  onChange={_onParentPasswordEnter}
-                  type='password'
-                  value={parentPassword}
-                />
-                {!!parentPassword && !isProperParentPassword && (
-                  <Warning
-                    isBelowInput
-                    isDanger
-                  >
-                    {t('Wrong password')}
-                  </Warning>
-                )}
-              </div>
-              {isProperParentPassword && (
-                <>
-                  <DerivationPath
-                    defaultPath={defaultPath}
-                    isError={!suriPath}
-                    onChange={setSuriPath}
-                    parentAddress={parentAddress}
-                    parentPassword={parentPassword}
-                  />
-                  {!suriPath && (
-                    <Warning
-                      isBelowInput
-                      isDanger
-                    >
-                      {t('Incorrect derivation path')}
-                    </Warning>
-                  )}
-                </>
+              <DerivationPath
+                defaultPath={defaultPath}
+                isError={!suriPath}
+                onChange={setSuriPath}
+                parentAddress={parentAddress}
+                parentPassword={parentPassword}
+              />
+              {!suriPath && (
+                <Warning
+                  isBelowInput
+                  isDanger
+                >
+                  {t('Incorrect derivation path')}
+                </Warning>
              )}
            </>
          )}
@ -171,43 +153,18 @@ function SelectParent ({ className, isLocked, onDerivationConfirmed, parentAddre
      </div>
      <VerticalSpace/>
      <ButtonArea>
-        {shouldAccountBeDerived
-          ? (
-            <NextStepButton
-              data-button-action='create derived account'
-              isBusy={isBusy}
-              isDisabled={!isProperParentPassword || !suriPath}
-              onClick={_onSubmit}
-            >
-              {t<string>('Create a derived account')}
-            </NextStepButton>
-          )
-          : (
-            <NextStepButton
-              data-button-action='create root account'
-              onClick={_goCreate}
-            >
-              {t<string>('Create account from new seed')}
-            </NextStepButton>
-          )
-        }
+        <NextStepButton
+          data-button-action='create derived account'
+          isBusy={isBusy}
+          isDisabled={!isProperParentPassword || !suriPath}
+          onClick={_onSubmit}
+        >
+          {t<string>('Create a derived account')}
+        </NextStepButton>
      </ButtonArea>
    </>
  );
 }

 export default styled(SelectParent)`
-  .smallerMargin {
-    margin-top: 0;
-    margin-bottom: 10px;
-  }
-
-  .disableArea {
-    opacity: 1;
-
-    .disabled {
-      opacity: 0.2;
-      pointer-events: none;
-    }
-  }
 `;
--- a/packages/extension-ui/src/partials/MenuAdd.tsx
+++ b/packages/extension-ui/src/partials/MenuAdd.tsx
@ -3,7 +3,7 @@

 import type { ThemeProps } from '../types';

-import { faFileUpload, faKey, faPlusCircle, faQrcode } from '@fortawesome/free-solid-svg-icons';
+import { faCodeBranch, faFileUpload, faKey, faPlusCircle, faQrcode } from '@fortawesome/free-solid-svg-icons';
 import { FontAwesomeIcon } from '@fortawesome/react-fontawesome';
 import React, { useCallback, useContext } from 'react';
 import styled from 'styled-components';
@ -36,16 +36,23 @@ function MenuAdd ({ className, reference }: Props): React.ReactElement<Props> {
      reference={reference}
    >
      <MenuItem className='menuItem'>
-        <Link to={master ? `/account/derive/${master.address}` : '/account/create'}>
+        <Link to={'/account/create'}>
          <FontAwesomeIcon icon={faPlusCircle} />
-          <span>{
-            master
-              ? t('Create new account (root or derived)')
-              : t('Create new account')
-          }</span>
+          <span>{ t('Create new account')}</span>
        </Link>
      </MenuItem>
      <MenuDivider />
+      {!!master && (
+        <>
+          <MenuItem className='menuItem'>
+            <Link to={`/account/derive/${master.address}`}>
+              <FontAwesomeIcon icon={faCodeBranch} />
+              <span>{t('Derive from an account')}</span>
+            </Link>
+          </MenuItem>
+          <MenuDivider />
+        </>
+      )}
      <MenuItem className='menuItem'>
        <Link to='/account/import-seed'>
          <FontAwesomeIcon icon={faKey} />