Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

receive button #621

Merged
merged 15 commits into from
Apr 12, 2018
Merged

receive button #621

merged 15 commits into from
Apr 12, 2018

Conversation

jbibla
Copy link
Collaborator

@jbibla jbibla commented Apr 6, 2018

closes #522

screen shot 2018-04-06 at 3 11 30 pm

@jbibla jbibla changed the title Jordan/522 receive button WIP: receive button Apr 6, 2018
@codecov
Copy link

codecov bot commented Apr 9, 2018

Codecov Report

Merging #621 into develop will increase coverage by 0.02%.
The diff coverage is 100%.

@@             Coverage Diff             @@
##           develop     #621      +/-   ##
===========================================
+ Coverage     85.6%   85.62%   +0.02%     
===========================================
  Files           89       91       +2     
  Lines         1507     1524      +17     
  Branches        69       69              
===========================================
+ Hits          1290     1305      +15     
- Misses         203      205       +2     
  Partials        14       14
Impacted Files Coverage Δ
app/src/renderer/components/common/NiListItem.vue 100% <ø> (ø) ⬆️
app/src/renderer/components/wallet/PageWallet.vue 85.29% <100%> (-4.71%) ⬇️
.../src/renderer/components/common/NiModalReceive.vue 100% <100%> (ø)
...pp/src/renderer/components/common/NiBtnReceive.vue 100% <100%> (ø)
app/src/renderer/App.vue 90.9% <100%> (+0.9%) ⬆️

@nylira
Copy link
Contributor

nylira commented Apr 9, 2018

Sorry for not getting to this earlier. I like this idea a lot. It can be improved for usability.

  • Let's show the entire address using hard text wrap instead of an ellipsis. The address is already shown in truncated on the main app.
  • How about showing a QR code (with the cosmos atom embedded in the center) as well as the address? And providing a Share button to share the image through social media.
  • There's one button too many now. How about we delete the Copy button? And then make it when you click Receive, your address is automatically copied to the clipboard.

@faboweb
Copy link
Collaborator

faboweb commented Apr 10, 2018

I would vote against QR codes for now. There is no way to use them currently.
I like the idea of removing the copy button and auto copy on receive click

@jbibla
Copy link
Collaborator Author

jbibla commented Apr 11, 2018

The address is already shown in truncated on the main app.

it's a desktop app - and while voyager is at around 40% of my 13 inch mac you can still see the whole address.

when you click Receive, your address is automatically copied to the clipboard.

so you're saying we should replace the copy button with the receive button, which copies the address to clipboard and opens the receive modal? we're still going to have to show the user a copy notification so they know what happened. this feels a bit awkward to me. and an extra annoying modal for people that already knew they just wanted to copy the address.

the purpose, is just to add some familiarity (like other wallet apps) and provide a bit of a learning opportunity for people. the existing address + copy was a fine ux IMO.

@nylira
Copy link
Contributor

nylira commented Apr 11, 2018

@jolesbi I agree that combining the two buttons and showing a modal is not a great compromise. It sucks for power users. But I still don't like having two buttons that do similar things. Here are my thoughts on how we can improve the receive experience without adding a second button.

  1. We should rename the section title "Your Address" to "My Receive Address". This should help reduce ambiguity.
  2. Let's remove the copy button. Instead, the user can click on the address itself. The notification can be updated to something like title: 'Copied your address to clipboard', subtitle: 'Share your address with others to receive tokens.'.
  3. Keep the "Receive" button and modal as-is. Although we should write in there something about only Cosmos tokens can be accepted.

@jbibla
Copy link
Collaborator Author

jbibla commented Apr 11, 2018

great ideas @nylira. i like 2 and 3 very much.

is it ok if we don't do 1? it's not really a "receiving address" only. by implementing 2 and 3 i think the title is ok as is.

@nylira
Copy link
Contributor

nylira commented Apr 11, 2018

@jolesbi Yes, we don't have to do 1 :)

@jbibla
Copy link
Collaborator Author

jbibla commented Apr 12, 2018

i think this is good to go.

@jbibla jbibla changed the title WIP: receive button receive button Apr 12, 2018
@nylira
Copy link
Contributor

nylira commented Apr 12, 2018

It looks and works great. Love it! I made a small change to use a different part of NiListItem for the receive button--it should look exactly the same.

@nylira nylira merged commit 710547a into develop Apr 12, 2018
@nylira nylira deleted the jordan/522-receive-button branch April 12, 2018 08:32
@faboweb
Copy link
Collaborator

faboweb commented Apr 12, 2018

Great teamwork guys

faboweb pushed a commit that referenced this pull request Jun 2, 2020
* fix polkadot overview for 0 balance accounts

* changelog
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Receive button
3 participants