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

Confirm Screen Updates #11

Closed
cjeria opened this issue Apr 9, 2018 · 11 comments
Closed

Confirm Screen Updates #11

cjeria opened this issue Apr 9, 2018 · 11 comments
Assignees
Labels
Permissions / Pop-ups Related to user permissions and dapp-wallet actions

Comments

@cjeria
Copy link
Contributor

cjeria commented Apr 9, 2018

We have a few GH issues that are addressing different problems with the confirm transaction screen.

Problems:

Total amount + Gas not visible

Total and Amount/Gas Fee values are hidden below main action buttons because tx body data is pushing it all the way down.

See the gihub issue here: MetaMask/metamask-extension#3885
image.png

Show method data

Need to show transaction “method data” in transaction ui somewhere. Dan suggested bubbling up this piece of data. Need to find a balance between this and amount.

See the gihub issue here: MetaMask/metamask-extension#2847

image.png

Trust Badge

Another team at ConsenSys is wanting us to design for adding a trust badge to verified smart contracts. This is a work in progress and we still don't have details regarding what this will mean, but there's a mock with a checkmark in the sketch file to reference.

image.png

Side Notes

We need to strike a balance between displaying the transaction type, method data, amounts and action buttons in such a way can be consistently displayed across confirm screens.

@celinepark94 I've added a new page in our sketch file called "Confirm TX screen" with all the screens related to the above. Let me know if you have any questions!

@cjeria
Copy link
Contributor Author

cjeria commented May 22, 2018

Latest confirm screen designs that addresses all of the problems discussed above. This design also fits within the latest size constraints (width and height) that @alextsg and I have been solidifying.
Feedback is welcomed @danfinlay @kumavis

confirm screen ixd

image

@cjeria cjeria self-assigned this May 22, 2018
@danfinlay
Copy link

This is a nice design for when the transfer is pure ether. A couple topics I'd want to consider more:

The primary currency shown

These current designs massively prioritize the user's currency, even making the ether very small by comparison.

I think I'm okay with this, but would like to ask: Is there a way we can represent the approximate nature of these dollar values?

One way I've suggested is adding a tilde (~) character before the converted values, so it would read: ~$3,000,000.00 in this case. It might be even more correct to use the "about equal" symbol ().

Pure Function Calls

Not all transactions have ether implicitly attached to them. Often, the transaction really is about the Data. For those, the big header (about 30% of the view) would be dedicated to showing $0.00 0.0000 ether.

For cases when there is no ether being sent, and there is data associated with the transaction, and we are able to retrieve a function name from the on-chain registry, I strongly feel we should show the function name as a primary label on the confirmation screen. It is clearly the most useful information we could show the user.

Rendering Function Info

I'm sorry if providing this JSON ABI to you in the past was confusing. This data structure is probably not actually the ideal thing to show the end user.

The data you see in the above screen shot includes some specific things that could be rendered nicely:

  • Function name: Transfer To
  • Address: 0xabcdefg1234567890
  • Uint256 (This just means a potentially big number): 235.

That address could be a link to etherscan, we could render an icon next to it, we could make it useful.

That uint256 is harder to do anything with, but in any case, I think this information is better shown as a table or something than this intimidating-looking JSON data. We want to invite even casual users to get a sense of what's happening, and we may have to do some work to make this less intimidating for them.

The Hex Data

If we're able to render the Function data, we can probably tuck the Hex Data under a expandable section. It's big, it's ugly, and it's basically meaningless to the vast majority of people who might look at the view.

An Advanced Option: Full Tx Params

If under Hex Data, we had another section called Full Parameters, we could have a JSON blob that was fully copyable, representing the whole transaction. This would allow the user to take the transaction and sign it elsewhere if they wanted, it allows a variety of advanced use cases. This would be a simple thing to add, we would probably tuck it under an expandable section, and it would actually be a big step towards allowing a minimum viable integration with hardware wallets or even multisig wallet sites.

Conclusion

It looks very nice, so much cleaner & smaller now that we got rid of the redundant to/from, that's great. It's definitely an upgrade from what we have now, so if we just wanted to load something into the queue, I'd be happy to approve this as is, and iterate from there.

@cjeria
Copy link
Contributor Author

cjeria commented May 22, 2018

Thanks for the feedback

This is a nice design for when the transfer is pure ether.

Yes, this is what this issue is focused on addressing.

I think we can extract other use-cases from your point about pure function calls or "actions". Here's an issue I created for that. #16

@cjeria cjeria added the Permissions / Pop-ups Related to user permissions and dapp-wallet actions label May 25, 2018
@bdresser
Copy link

bdresser commented Jun 4, 2018

this is nice @cjeria!

the nonce element jumps out at me – it's reminiscent of the little orange Ad badge that google uses on mobile, makes the screen seem a little like a sponsored post 😆perhaps another color could solve?

also, what's the approach to the new "review transfer" header here? would it update if the function changes? might be worth thinking about how we utilize the header text more generally across all confirmation screens since it'll be big for explaining / clarity.

@MicahZoltu
Copy link

MicahZoltu commented Jun 5, 2018

I would like to see ETH as the primary currency and USD (or whatever) as the secondary currency. At the moment, I cannot even choose ETH as a currency in the currency selector so if I want to transact in (and think about) ETH, I'm still forced to see USD (or whatever) and have to squint to see ETH.

Suggestions:

  1. Allow the user to choose both primary and secondary currencies in the settings. If the user wants to display USD and CNY as their two currencies they can, in which case they won't see ETH anywhere in the UI.
  2. Provide the user with a checkbox in the options indicating whether they want ETH to be the primary display currency or the secondary one. This would go along with the current alternate currency chooser.
  3. Make ETH always the primary currency because we are working in the Ethereum ecosystem and it is the only currency that will ever be "exact" (all other currencies will be an approximation based on exchange feeds that can be manipulated).
  4. Let the user choose ETH as an alternate currency, and if selected would only show ETH in the UI.

@MicahZoltu
Copy link

Re: Presentation of transaction call data, if your team has the resources and/or interest in championing ethereum/EIPs#719 that would be the ultimate solution (IMO) to presenting users with contract call data in a way that is actually consumable by them. Even if you convert the JSON into something more "human readable" like a table, you still have the problem that most humans (non-engineers) won't have any idea what any of the data means.

@bdresser
Copy link

bdresser commented Jun 5, 2018

thanks @MicahZoltu - I've opened an issue on the main extension to discuss what you're suggesting.

@cjeria as we chatted about today, these screens should be able to cover

could you do another mock with @danfinlay's suggestions on the data screen just to make sure we're on the same page? function name, address (+link to etherscan), unit256, hex data (collapsed), full function call (collapsed)

@bdresser
Copy link

@alextsg @cjeria whatever happened to the little nonce indicator? I don't see it in the version that got merged. Was it scrapped intentionally or can I open another issue to re-add?

@alextsg
Copy link

alextsg commented Jul 18, 2018

@bdresser Since the nonce is calculated in the background after user confirmation, we're only sure of the nonce when the transaction is a retry transaction, so that's where you can see it.

@bdresser
Copy link

@alextsg thanks, makes sense. and looks like we're including in the designs for the outbox stuff on #25 👍

@bdresser
Copy link

closing this since it's been merged!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Permissions / Pop-ups Related to user permissions and dapp-wallet actions
Projects
None yet
Development

No branches or pull requests

6 participants