Register as delegate - Closes #354 #543
Conversation
| @@ -20,7 +21,13 @@ const HeaderElement = props => ( | |||
| theme={styles} | |||
| > | |||
| <MenuItem caption="Register second passphrase" /> | |||
| <MenuItem caption="Register as delegate" /> | |||
| <MenuItem caption="Register as delegate" | |||
| className={(props.account.isDelegate) ? styles.hidden : ''} | |||
There was a problem hiding this comment.
I see the PR is in progress, but I just noticed this one thing. IMO more elegant way to conditionally hide an element in react is the ternary operator - {shouldShow ? <Element /> : null }.
There was a problem hiding this comment.
On a small component with no reprinting effect, there's no difference. I'll change anyways.
| @@ -98,8 +98,10 @@ class LoginFormComponent extends React.Component { | |||
| getAccount(this.props.peers.data, accountInfo.address).then((result) => { | |||
| onAccountUpdated(result); | |||
| getDelegate(this.props.peers.data, accountInfo.publicKey).then((data) => { | |||
| console.log('success'); | |||
There was a problem hiding this comment.
Please remove this console.log
| onAccountUpdated({ delegate: data.delegate, isDelegate: true }); | ||
| }).catch(() => { | ||
| console.log('error'); |
There was a problem hiding this comment.
Please remove this console.log
| className='second-secret' | ||
| value={this.state.secondSecret} /> | ||
| } | ||
| <hr/> |
There was a problem hiding this comment.
hr is not visible due to display:none from passphrase.css. passphrase.css needs to be fixed.
| }); | ||
|
|
||
| const mapDispatchToProps = dispatch => ({ | ||
| onAccountUpdated: data => dispatch(accountUpdated(data)), |
There was a problem hiding this comment.
I cannot find where is this used.
src/components/header/header.css
Outdated
| @@ -20,3 +20,6 @@ | |||
| .menu { | |||
| right: -16px !important; | |||
| } | |||
| .hidden { | |||
| it('should return a dispatch object', () => { | ||
| const props = mountedAccount.find(RegisterDelegate).props(); | ||
| const data = props.showSuccessAlert('sample text'); | ||
| expect(data).to.be.equal(); |
| it('should return a dispatch object', () => { | ||
| const props = mountedAccount.find(RegisterDelegate).props(); | ||
| const data = props.showErrorAlert('sample text'); | ||
| expect(data).to.be.equal(); |
| wrapper = mount(<RegisterDelegate {...normalProps} />); | ||
| }); | ||
|
|
||
| it('renders an InfoParagraph components', () => { |
There was a problem hiding this comment.
typo "components" -> "component"
| }, | ||
| closeDialog: () => {}, | ||
| onAccountUpdated: () => {}, | ||
| showSuccessAlert: () => {}, |
There was a problem hiding this comment.
You can make this showSuccessAlert: sinon.spy() and then expect that it has been called when necessary.
Those tests ending with wrapper.find('.submit-button').simulate('click'); and not checking anything with expect.
There was a problem hiding this comment.
I noticed that I'm not doing this either (in send.test.js). I tried to but the spy does not seem to be called.
| closeDialog: () => {}, | ||
| onAccountUpdated: () => {}, | ||
| showSuccessAlert: () => {}, | ||
| showErrorAlert: () => {}, |
| }); | ||
|
|
||
| it('does not allow registering an existing username', () => { | ||
| delegateApiMock.expects('registerDelegate').resolves({ success: false }); |
There was a problem hiding this comment.
Should be rejects not resolves
| }); | ||
|
|
||
| it('does not allow register as delegate for a delegate account', () => { | ||
| delegateApiMock.expects('registerDelegate').resolves({ success: false }); |
There was a problem hiding this comment.
Should be rejects not resolves
|
|
||
| wrapper.find('.username input').simulate('change', { target: { value: 'sample_username' } }); | ||
| wrapper.find('.submit-button').simulate('click'); | ||
| }); |
There was a problem hiding this comment.
Test case for 'Username already exists' missing
| }, | ||
| closeDialog: () => {}, | ||
| onAccountUpdated: () => {}, | ||
| showSuccessAlert: () => {}, |
There was a problem hiding this comment.
I noticed that I'm not doing this either (in send.test.js). I tried to but the spy does not seem to be called.
| this.props.account.secondSecret && | ||
| <Input label='Second secret' required={true} | ||
| className='second-secret' | ||
| value={this.state.secondSecret} /> |
There was a problem hiding this comment.
The Input won't set this.state.secondSecret without onChange
|
Now that pending transactions PR is merged this should also dispatch a pending transaction (see how it's used in send send https://github.com/LiskHQ/lisk-nano/blob/development/src/components/send/send.js#L74-L81 ) |
reyraa
force-pushed
the
354-register-as-delegate
branch
from
d82b187 to
55ac449
Compare
reyraa
dismissed
slaweet’s stale review
Since Vit is not here and I've done the change requests I could handle
Closes #354