Skip to content
This repository has been archived by the owner on Nov 6, 2020. It is now read-only.

Close on double-click for Signer Account selection #4540

Merged
merged 2 commits into from
Feb 14, 2017

Conversation

ngotchac
Copy link
Contributor

Closes #4525

Close only on double-click. Change default on simple click.
Also update the default in an optimistic way (not waiting for network request). The request will be valid 99% of the time. If not, it will just be reverted.

@ngotchac ngotchac added A0-pleasereview 🤓 Pull request needs code review. M7-ui labels Feb 14, 2017
this.defaultAccount = defaultAccount;
transaction(() => {
this.accounts = this.accounts.map((account) => {
account.default = account.address === this.defaultAccount;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this correct? It basically sets the default to the previous default, i.e. this.defaultAccount is previous version since default is only set afterwards. Looks wrong. (If correct, a NOTE explanation may be in order here)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes right...

return this._api.parity
.setNewDappsWhitelist(accounts)
.catch((error) => {
console.warn('makeDefaultAccount', error);
});
}

loadDefaultAccount () {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would probably make sense to add this to the spec.js as well (since the store is more-or-less tested already)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added

@jacogr jacogr added A5-grumble 🔥 Pull request has minor issues that must be addressed before merging. and removed A0-pleasereview 🤓 Pull request needs code review. labels Feb 14, 2017
@ngotchac ngotchac added A0-pleasereview 🤓 Pull request needs code review. and removed A5-grumble 🔥 Pull request has minor issues that must be addressed before merging. labels Feb 14, 2017
@jacogr jacogr added A8-looksgood 🦄 Pull request is reviewed well. and removed A0-pleasereview 🤓 Pull request needs code review. labels Feb 14, 2017
@gavofyork gavofyork merged commit ac27806 into master Feb 14, 2017
@gavofyork gavofyork deleted the ng-dbl-click-signer branch February 14, 2017 21:47
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A8-looksgood 🦄 Pull request is reviewed well.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants