JSON Keyfile backup #101 #260

pmespresso · 2018-11-20T00:13:23Z

#101
…
todo:

~~add option to export json keyfile to backup from Account~~
~~json encoding check~~
~~clean up duplicated code in store~~
~~add import options screen~~

todo: * add option to export json keyfile to backup * clean up duplicated code in store

uses react-dropzone

…into yj-json-keyfile

axelchalon

cool :)

I tried exporting to json an existing account in Fether and importing it again and it gave me an account with a different address than that in the json file, not sure why that is

packages/fether-react/package.json

packages/fether-react/src/stores/createAccountStore.js

packages/fether-react/src/Tokens/Tokens.js

packages/fether-react/src/Accounts/CreateAccount/AccountRewritePhrase/AccountRewritePhrase.js

Tbaut · 2018-11-21T19:48:39Z

A bit late for a mockup, but what do you think about something like this for the new account:

I think it's important to clearly distinguish the 2 types of account recovery/import.

The transition from screen 1 to screen 2 would happen automatically when the user select a file. The "next" button on screen 1 is only useful for recovery with the phrase.

move jsonbackup to its own component/route lint lint

Tbaut · 2018-11-21T23:06:15Z

Also take a look at my proposals to add more menu items at #264

no drop, only browse for file lint

Tbaut · 2018-11-22T11:55:15Z

This is looking really good!

I have a similar problem as the one mentioned by Axel. Importing an account that I have gives me a different one at the import. I noticed that the account doesn't have 0x, could that be the culprit?
Edit: the account seems to be the same, but the identicon (blocky) is not.

We should be extra careful I think and ask for a password confirmation. This doesn't make the UX so much worse, and we better make sure the user did not mistype his password.

Also this makes the password screen quite large, I'd rather have a different screen rather than a dropped view, and we can add a little explanation to guide users. Something like: "Choose a password to encrypt Json keystore file".

Tbaut · 2018-11-23T09:22:12Z

Cool, all my previous comments are solved.

Nit: It'd be nice to accept any sort of file, Parity produces keystore file names without .json and those are not accepted right now unless I specifically add .json.
Also when opening the system's file browser to select a file, can we set it so that it shows any file? Right now it doesn't show anything else than the directories and I have to switch to "any file" to be able to see my .json ones. It requires extra clicks and a novice user might be confused.

I had one weird behavior, when importing an account:

I was asked to enter a name (with prefilled name which is great) the enter doesn't allow me to go to the next screen, I have to click the button.
but more importantly, on the password screen the enter key is mapped to the back button I guess, so when you think you're done, hit enter but it goes back :)

edit: just realized this is everywhere.. so we might address this in another PR.

…-json-keyfile

Tbaut

Some nits:

the next button should be in the recovery phrase card and centered (see inline comment)
focus the "password" field at import and export
map the enter key to the "confirm account import" and "backup account" buttons (now on the back)

Otherwise looks great :)

Tbaut · 2018-11-22T11:31:03Z

packages/fether-react/src/Accounts/CreateAccount/AccountImportOptions/AccountImportOptions.js

+        <br />
+        <Card> {phraseCard} </Card>
+        <br />
+        {this.renderButton()}


Can we put the next button in the "phrase card" as it's only relevant to this one, I believe it should be centred as well as are buttons usually.

Tbaut · 2018-12-06T10:06:26Z

Not sure you expected a review on this yet (simply change the label to "please review" when you think we should have a look)

The import function is now broken (gives a different address every time, looks like the JSON is not read properly)
The "enter" key is not mapped for the export function.
nit (as in "nice to have" if not hard to implement otherwise we can leave it): focus the only available input field (passwords) when getting the screen save a click to users.

pmespresso · 2018-12-07T01:30:02Z

@Tbaut, I think it'd be best to make the "focus on password field" a separate PR as it'd involve refactoring the Form Fields as stateful components so we can use React refs etc.

That PR would also fix the current behavior where pressing Enter maps to the Back button in a much simpler way than the past couple commits which detects the keyPress events and keyCodes per form, blabla... but doesn't relate to json import particularly. I will raise this as a separate issue and PR.

Tbaut · 2018-12-07T09:14:15Z

I'm fine with that.
I just tested it and had a problem to import the account, it wouldn't let me pass this screen (with the right password)

ddorgan · 2018-12-12T01:50:56Z

[clabot:check]

parity-cla-bot · 2018-12-12T01:50:58Z

It looks like @yjkimjunior hasn't signed our Contributor License Agreement, yet.

The purpose of a CLA is to ensure that the guardian of a project's outputs has the necessary ownership or grants of rights over all contributions to allow them to distribute under the chosen licence.
Wikipedia

You can read and sign our full Contributor License Agreement at the following URL: https://cla.parity.io

Once you've signed, please reply to this thread with [clabot:check] to prove it.

Many thanks,

Parity Technologies CLA Bot

Tbaut

My local branch was running crazy, I did a complete test:

recovery from phrase
export from account
import account (previously exported)

all works well, thank you so much :)

amaury1093 · 2018-12-12T13:34:43Z

@yjkimjunior Can you rebase/merge lastest master?

pmespresso · 2018-12-14T14:07:59Z

[clabot:check]

parity-cla-bot · 2018-12-14T14:08:01Z

It looks like @yjkimjunior signed our Contributor License Agreement. 👍

Many thanks,

Parity Technologies CLA Bot

pmespresso added 3 commits November 19, 2018 16:06

WIP: Added JSON Keyfile dropzone logic.

f0939bd

todo: * add option to export json keyfile to backup * clean up duplicated code in store

FIle input component

fb43acf

uses react-dropzone

this endpoint

72d8b71

Tbaut added the A3-inprogress label Nov 20, 2018

pmespresso added 11 commits November 20, 2018 12:44

move data through ui to export account api call

ddee8a8

saves backup as json

20bb41f

json encoding check, err handling

4db5528

center stuff

6f54eea

error handling

0d19739

lint!

7de6323

lintgit add .

6669bc2

browser filereader

e6a5b5f

Merge branch 'master' into yj-json-keyfile

55a404d

final lint

f526b12

Merge branch 'yj-json-keyfile' of https://github.com/paritytech/fether …

d372758

…into yj-json-keyfile

pmespresso changed the title ~~WIP: JSON Keyfile backup #101~~ JSON Keyfile backup #101 Nov 21, 2018

pmespresso added A0-pleasereview and removed A3-inprogress labels Nov 21, 2018

axelchalon suggested changes Nov 21, 2018

View reviewed changes

Tbaut added A5-grumble and removed A0-pleasereview labels Nov 21, 2018

change element pos, typos

18f66d5

move jsonbackup to its own component/route lint lint

import options screen

7a9a43f

no drop, only browse for file lint

move backup to separate screen, add 0x prefix

3eb3ebf

pmespresso added 2 commits November 23, 2018 13:18

Merge branch 'master' of https://github.com/paritytech/fether into yj…

c09d93c

…-json-keyfile

accept any file type

01d7b6c

amaury1093 added A8-looksgood and removed A5-grumble labels Dec 4, 2018

Tbaut suggested changes Dec 4, 2018

View reviewed changes

center button

7b6e3eb

Tbaut added A5-grumble and removed A8-looksgood labels Dec 6, 2018

revert to working version

9aa209d

pmespresso force-pushed the yj-json-keyfile branch from 56ff553 to 9aa209d Compare December 7, 2018 02:03

Tbaut added A0-pleasereview and removed A5-grumble labels Dec 7, 2018

Tbaut added A5-grumble and removed A0-pleasereview labels Dec 7, 2018

axelchalon mentioned this pull request Dec 11, 2018

Parity Signer integration #146

Closed

fix backup

697f610

Tbaut approved these changes Dec 12, 2018

View reviewed changes

Merge branch 'master' into yj-json-keyfile

49b41cd

axelchalon approved these changes Dec 12, 2018

View reviewed changes

axelchalon added A8-looksgood and removed A5-grumble labels Dec 12, 2018

axelchalon merged commit 844c657 into master Dec 12, 2018

amaury1093 deleted the yj-json-keyfile branch December 12, 2018 14:40

Tbaut mentioned this pull request Dec 17, 2018

Add an account with JSON Keystore file #101

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

JSON Keyfile backup #101 #260

JSON Keyfile backup #101 #260

pmespresso commented Nov 20, 2018 •

edited

Loading

axelchalon left a comment

Tbaut commented Nov 21, 2018

Tbaut commented Nov 21, 2018

Tbaut commented Nov 22, 2018 •

edited

Loading

Tbaut commented Nov 23, 2018 •

edited

Loading

Tbaut left a comment

Tbaut Nov 22, 2018

Tbaut commented Dec 6, 2018

pmespresso commented Dec 7, 2018 •

edited by Tbaut

Loading

Tbaut commented Dec 7, 2018

ddorgan commented Dec 12, 2018

parity-cla-bot commented Dec 12, 2018

Tbaut left a comment

amaury1093 commented Dec 12, 2018

pmespresso commented Dec 14, 2018

parity-cla-bot commented Dec 14, 2018

JSON Keyfile backup #101 #260

JSON Keyfile backup #101 #260

Conversation

pmespresso commented Nov 20, 2018 • edited Loading

axelchalon left a comment

Choose a reason for hiding this comment

Tbaut commented Nov 21, 2018

Tbaut commented Nov 21, 2018

Tbaut commented Nov 22, 2018 • edited Loading

Tbaut commented Nov 23, 2018 • edited Loading

Tbaut left a comment

Choose a reason for hiding this comment

Tbaut Nov 22, 2018

Choose a reason for hiding this comment

Tbaut commented Dec 6, 2018

pmespresso commented Dec 7, 2018 • edited by Tbaut Loading

Tbaut commented Dec 7, 2018

ddorgan commented Dec 12, 2018

parity-cla-bot commented Dec 12, 2018

Tbaut left a comment

Choose a reason for hiding this comment

amaury1093 commented Dec 12, 2018

pmespresso commented Dec 14, 2018

parity-cla-bot commented Dec 14, 2018

pmespresso commented Nov 20, 2018 •

edited

Loading

Tbaut commented Nov 22, 2018 •

edited

Loading

Tbaut commented Nov 23, 2018 •

edited

Loading

pmespresso commented Dec 7, 2018 •

edited by Tbaut

Loading