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

Display Public key and Key path #1145

Merged
merged 4 commits into from Feb 8, 2019

Conversation

lontivero
Copy link
Collaborator

This PR is for adding more info in the receive address details where previously we only displayed the QR code. This is useful IMO because it can help users to understand the derivation scheme and prevent misunderstanding and let them find an address in tools such as the Ian Coleman bip39 tool.

image

@nopara73
Copy link
Contributor

nopara73 commented Feb 5, 2019

Ack. We'll need this for the Encryption Manager: #1127

But it is half job. If we show it here, we can show it in the Coin expander, too. I think that's uncontroversial, and easy, right?

However this'll inevitably bring up many more questions, I think a Wallet Information tab in the Wallet Manager should be the answer to them, which would list the wallets and if selected it'll show additional information.

Overall, I think this PR is review ready with Coin expander and with a specification issue opened for the Wallet Information tab.

@lontivero lontivero force-pushed the Features/Display-Address-Pubkey branch from 507f499 to 3e83548 Compare February 6, 2019 16:06
@lontivero
Copy link
Collaborator Author

I added it to the coin info.

image

@nopara73
Copy link
Contributor

nopara73 commented Feb 6, 2019

Can you make dump comment. On mobile the "mark as unread" option is not present and I don't want to forget to review.

@lontivero
Copy link
Collaborator Author

Sure. Please review it.

@nopara73
Copy link
Contributor

nopara73 commented Feb 7, 2019

  • On the receive tab the public key and keypath cannot be copied.
  • Highlighting of the receive expander should be the same as the coins expander. It wasn't bothering before, because there was only a QR in it.

image

vs

image

@molnard molnard mentioned this pull request Feb 7, 2019
6 tasks
@lontivero
Copy link
Collaborator Author

Done. It behaves as CoinList now and the data can be selected and copyed.

@nopara73 nopara73 merged commit b15bcc6 into zkSNACKs:master Feb 8, 2019
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.

None yet

2 participants