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

Discourage private keys (pt. 1) #780

Merged
merged 5 commits into from
Jan 24, 2018
Merged

Discourage private keys (pt. 1) #780

merged 5 commits into from
Jan 24, 2018

Conversation

wbobeirne
Copy link
Contributor

Checks off a few from #557. Implements the warning text from v3, but cranks it up to 11. This was a pretty quick pass at this, so I'm looking for plenty of feedback on how it looks and reads. I call this part 1 since it doesn't hit all checkboxes, and I'd like to see #778 get resolved as a part 2.

What This Does

  • Makes users read and confirm warnings before unlocking private keys, keystore files, or mnemonic phrases
  • Makes the Account view the default homepage.
  • Adds a link to the Generate tab on Account's wallet unlock for if you don't have a wallet.

What This Doesn't Do

  • Passwordify private key / mnemonic
  • Force user to wait. I felt the checkboxes would be enough of a roadblock.
  • Homepagify account. Could use some guidance on what this might look like.

Screenshots

New Homepage

screen shot 2018-01-10 at 2 47 08 pm

Insecure Unlock Warning

screen shot 2018-01-10 at 2 47 17 pm

Acknowledging

acknowledge

@wbobeirne wbobeirne added status: ready for review Open PR's that are ready for technical review. Replaced with "changes needed" or "ready to merge". type: discussion Items that are primarily a discussion around a feature or issue. May evolve to be actionable. labels Jan 10, 2018
@wbobeirne wbobeirne requested review from dternyak and tayvano January 10, 2018 19:50
@wbobeirne
Copy link
Contributor Author

From Taylor on Slack:

I like it for when we have the apps. let’s circle back in a week after we see what the general feedback on the current changes are from the community
Im curious to know if we’re being “too scary”
finding the balance between educate, scary, etc will be good

I'll leave this PR open while we wait for more feedback about the messaging in v3.

@wbobeirne
Copy link
Contributor Author

I'm pretty sure we're moving forward on this one. I've merged the latest develop in, and confirmed that it does not bother you on the Electron or HTML download builds.

@dternyak dternyak merged commit 4fb342a into develop Jan 24, 2018
@dternyak dternyak deleted the discourage-private-keys branch January 24, 2018 22:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: ready for review Open PR's that are ready for technical review. Replaced with "changes needed" or "ready to merge". type: discussion Items that are primarily a discussion around a feature or issue. May evolve to be actionable.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants