Skip to content

Conversation

@shawntabrizi
Copy link
Member

@shawntabrizi shawntabrizi commented May 22, 2020

This is an early draft of enabling the Polkadot.JS extension to restore accounts using the JSON backup created by the Polkadot JS Apps.

TODO:

  • Positive/Negative feedback in UI
    • When file selected is good or bad
    • When password is correct or incorrect
    • When restore is successful or not
  • Styling
  • "upload" SVG icon

@shawntabrizi
Copy link
Member Author

shawntabrizi commented May 22, 2020

I seem to have got it working:

  1. On import, you can select "Restore from JSON Backup"

image

  1. It opens a new browser page where you can give the details

image

  1. Account Restored

image

A few things here:

  • What does keyring.isPassValid do when not doing a full restore? It does not take in the JSON file, so it just makes sure the string provided has certain requirements?

  • How do I handle the async messages between the extension and browser? i.e. I want to update a useState or show the user a message as a result of sending a message across the border. Do I just need to add async/await?

@shawntabrizi shawntabrizi requested a review from jacogr May 24, 2020 23:01
@shawntabrizi shawntabrizi marked this pull request as ready for review May 24, 2020 23:09
@shawntabrizi
Copy link
Member Author

Updated Styling:

image

image

image

image

image

image

@shawntabrizi
Copy link
Member Author

@jacogr can you help me fix the linting issues?

I am unable to get the lint error to appear locally, and tbh not quite sure how to solve all of them correctly.

@jacogr
Copy link
Member

jacogr commented May 27, 2020

Yes, will help fix, no issues. (With the recent bump to typescript-eslint 3 there are actually some linter bugs, so happy to help fix.)

Will get to this shortly, really excited to have it - things have just been a bit hectic elsewhere, so this hasn't gotten the attention it deserves.

@shawntabrizi shawntabrizi requested a review from jacogr May 27, 2020 14:09
Copy link
Member

@jacogr jacogr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks a lot for this. I made some small adjustments -

  • some async functions converted to .then/catch - just aligning the code with elsewhere (nothing wrong as-is)
  • made the open window take a param (instead of adding another)
  • go back to root when we have imported
  • added a back button on the import
  • only open new window when we are a popup

It works great. So will get it in as-is, however there are probably a couple of this that should be looked at sometime (not here)

  • when we cannot decode, just display the message (like we have with invalid seed). I removed the message as of now since it was a bit inconsistent with the rest
  • since we operate in window mode, we may as well have a top-level button on-screen (like we have in QR - i would only make it visible in full-screen mode, hence it is not in. We probably want full-screen mode to take up more horizontal space and make it "nice", so not now)
  • am not crazy about the "JSON link", but atm don't have immediate better ideas. Maybe import should toggle between the types like we have in the create new seed page - but we have the window issue, otherwise drag-and-drop and open is just broken

@jacogr
Copy link
Member

jacogr commented May 30, 2020

PS: Once again, sorry it took me absolute ages to get to this, not making excuses, but it was a "bit tight" lately.

@jacogr jacogr merged commit 256c5d9 into master May 30, 2020
@jacogr jacogr deleted the shawntabrizi-restore-json branch May 30, 2020 06:59
@shawntabrizi
Copy link
Member Author

Awesome! Can't wait to use this feature :)

@polkadot-js-bot
Copy link

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@polkadot-js polkadot-js locked as resolved and limited conversation to collaborators Jun 3, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants