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

Fix address and accounts links #4491

Merged
merged 3 commits into from Feb 9, 2017
Merged

Fix address and accounts links #4491

merged 3 commits into from Feb 9, 2017

Conversation

ngotchac
Copy link
Contributor

@ngotchac ngotchac commented Feb 9, 2017

Closes #4314

In Signer and Transaction lists, redirect the links to Parity UI and not Etherscan

@ngotchac ngotchac added A0-pleasereview 🤓 Pull request needs code review. M7-ui labels Feb 9, 2017
@jacogr
Copy link
Contributor

jacogr commented Feb 9, 2017

Failing tests.

@ngotchac
Copy link
Contributor Author

ngotchac commented Feb 9, 2017

Fixed test (and added 2 actually)

@observable balances = {};
@observable localHashes = [];

constructor (api, withLocalTransactions = false) {
externalLink = '';
Copy link
Contributor

@jacogr jacogr Feb 9, 2017

Choose a reason for hiding this comment

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

Prefer _externalLink for non-exposed internal variables, i.e. no observable. (As per rest of the class)

const { accounts } = initState.personal;
const accountAddresses = Object.keys(accounts);

return () => {
Copy link
Contributor

Choose a reason for hiding this comment

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

Why not a simple

return {
  accountAddresses
};

(As opposed to the function?)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's only so it doesn't re-render on every Redux dispatched action (since the same Object is thus returned)

@jacogr
Copy link
Contributor

jacogr commented Feb 9, 2017

I'm just wondering if externalLink is anything but '' anywhere? Adds a lot of complexity (especially in terms of yet more change listeners), however I'm not seeing where it is set to any value that is not the default.

(I may be going blind)

@ngotchac
Copy link
Contributor Author

ngotchac commented Feb 9, 2017

It's mainly for the embedded Parity Bar for the Chrome Extension ! ;)

@jacogr jacogr added A8-looksgood 🦄 Pull request is reviewed well. and removed A0-pleasereview 🤓 Pull request needs code review. labels Feb 9, 2017
@jacogr jacogr merged commit 48eac51 into master Feb 9, 2017
@jacogr jacogr deleted the ng-fix-account-links branch February 9, 2017 16:41
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.

None yet

2 participants