Skip to content
This repository has been archived by the owner on May 24, 2022. It is now read-only.

feat: Relates to #98. Add Etherscan account or token link to send page TokenBalance component #310

Closed
wants to merge 3 commits into from

Conversation

ltfschoen
Copy link
Contributor

@ltfschoen ltfschoen commented Dec 19, 2018

  • Opens link to the Etherscan account page when the user clicks the TokenBalance component (the area shown in 'purple' colour below) on the "Send Ether" page

screen shot 2018-12-19 at 8 07 37 pm

  • Opens link to directly to the specific token page of the Etherscan account when the user clicks the TokenBalance component on the "Send THIBCoin" (or other non-Ether token) page

screen shot 2018-12-19 at 8 07 46 pm

…e TokenBalance component

* Opens link to the Etherscan account page when the user clicks the TokenBalance component on the "Send Ether" page

* Opens link to directly to the specific token page of the Etherscan account when the user clicks the TokenBalance component on the "Send THIBCoin" (or other non-Ether token) page
@amaury1093
Copy link
Collaborator

I would propose a more explicit button. Now it seems like the user would accidentally click on that part of the screen, and gets redirected, which is undesired.

Copy link
Collaborator

@Tbaut Tbaut left a comment

Choose a reason for hiding this comment

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

I'm quite strongly opposed to this PR as it adds a feature that is not needed IMHO, or at least not on this screen, also it isn't backed by an issue where we could have discussed it.

Also I think Etherscan should be avoided (maybe for something like blockscout? I'll open an issue for that), the first minutes of this talk tell enough about the why https://slideslive.com/38911970/plugging-the-metadata-leaks-in-the-ethereum-ecosystem

@ltfschoen
Copy link
Contributor Author

ltfschoen commented Dec 19, 2018

The issue was raised to close-out #98.

I agree that an explicit button/icon would be better. I'm also open to different options on where the link goes if we still proceed with it.

If as a provider we need to protect users by not providing a link to something that they may be familiar with or provides relevant information like Etherscan (as suggested at 5:30 in those slides https://slideslive.com/38911970/plugging-the-metadata-leaks-in-the-ethereum-ecosystem) that they can access without having to send a transaction, then shouldn't we also protect them the same way by not providing a link to Etherscan after they've send a transaction as we've done here https://github.com/paritytech/fether/blob/master/packages/fether-react/src/Send/Sent/Sent.js#L145.

I'm not familiar with the potential Blockscout alternative but it looks like it'll also do the job (potentially without the same consequences as Etherscan?).

Another option is to show a potentially annoying warning message if they click the icon that warns them of the consequences (as discussed in that slide) and suggests that they protect themselves (as also discussed in that slide). Similar to how we show a warning to users to backup their private key in Polkadot-JS Apps

If we have Settings section then we could allow the user to choose what convenience links they want to be shown in the UI (i.e. link to Etherscan or Blockscout) and then they'd only have to be warned and acknowledge the warning once

@Tbaut
Copy link
Collaborator

Tbaut commented Dec 19, 2018

My bad, this issue should have been discussed further IMHO and with mockups so that it's clear how and where to implement that. I updated it.

then shouldn't we also protect them the same way by not providing a link to Etherscan

Absolutely, just created the issue to track the idea that's been stuck in my head for a while #311.

@ltfschoen
Copy link
Contributor Author

Closing as this PR is being replaced by this PR #314 that uses BlockScout instead of Etherscan and uses icons as links instead of allowing user to click anywhere on the card (which wasn't done correctly anyway)

@ltfschoen ltfschoen closed this Dec 20, 2018
@Tbaut Tbaut deleted the luke-98-link-etherscan-eth-or-tokens branch January 18, 2019 11:25
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants