Skip to content
This repository has been archived by the owner. It is now read-only.

Bugfix: Registering delegate doesn't update UI - Closes #682 #688

Merged
merged 5 commits into from Sep 1, 2017

Conversation

2 participants
@slaweet
Copy link
Contributor

commented Aug 30, 2017

Closes #682

slaweet added some commits Aug 30, 2017

@slaweet slaweet self-assigned this Aug 30, 2017

@slaweet slaweet requested a review from reyraa Aug 30, 2017

@slaweet slaweet added this to Pull Requests in Sprint Board 28-08-17 Aug 30, 2017

@slaweet slaweet added this to Pull Requests in Version 1.1.0 Aug 30, 2017

@slaweet slaweet requested review from yasharAyari and removed request for reyraa Aug 31, 2017

@yasharAyari
Copy link
Contributor

left a comment

we don't need delegateRegistration middleware. you can simply call getDelegate in account middleware.

@@ -6,7 +6,7 @@ const Address = (props) => {
const title = props.isDelegate ? 'Delegate' : 'Address';
const content = props.isDelegate ?
(<div>
<p className="inner primary">
<p className="inner primary delegate-name">

This comment has been minimized.

Copy link
@yasharAyari

yasharAyari Aug 31, 2017

Contributor

please remove delegate-name class and use an ID instead of that. when we want to select a an element in js , it is better to use ID.

This comment has been minimized.

Copy link
@slaweet

slaweet Aug 31, 2017

Author Contributor

I added this class for e2e tests. All e2e tests use classes for selectors. It is not possible to change just this one. We can discuss if we should change classes to IDs in e2e tests, but it is definitely outside the scope of this PR.

@@ -0,0 +1,7 @@
export default {

This comment has been minimized.

Copy link
@yasharAyari

yasharAyari Aug 31, 2017

Contributor

we’re creating src/constants/transactionTypes.js when there are some other places to use this but you only uses this in one file.So, we don't need this constant.

This comment has been minimized.

Copy link
@slaweet

slaweet Aug 31, 2017

Author Contributor

I use it in three files:

src/components/transactions/amount.js
src/store/middlewares/delegateRegistration.js
src/store/middlewares/delegateRegistration.test.js

slaweet added some commits Aug 31, 2017

@slaweet slaweet merged commit 27195e4 into development Sep 1, 2017

1 check passed

continuous-integration/jenkins/pr-merge This commit looks good
Details

@slaweet slaweet deleted the 682-register-delegate-update branch Sep 1, 2017

@slaweet slaweet moved this from Pull Requests to Merged Pull Requests in Version 1.1.0 Sep 1, 2017

@slaweet slaweet moved this from Pull Requests to Merged Pull Requests in Sprint Board 28-08-17 Sep 1, 2017

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
You can’t perform that action at this time.