-
Notifications
You must be signed in to change notification settings - Fork 297
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
Feature/daef 300 exporting wallet to file #426
Feature/daef 300 exporting wallet to file #426
Conversation
…ef-300-exporting-wallet-to-file
…s WalletExportToFileDialog story import
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great work @DominikGuzei!
New feature works great!
I have only fixed few minor eslint warnings and ran npm run manage-translations
to populate missing translation keys.
I still have some questions/issues that I faced:
- Why have you removed paper-wallet-export-dialog? Is this export-to-file a replacement for the paper one?
- We are missing Japanese translations:
global.spendingPasswordLabel: !!!Spending Password
global.spendingPasswordPlaceholder: !!!Password
wallet.exportToFile.dialog.headline: !!!Export Wallet
wallet.exportToFile.dialog.introduction: !!!You are exporting <strong>{walletName}</strong> to a file.
wallet.exportToFile.dialog.submit.label: !!!Export
No. We need paper wallets but we are still waiting for the endpoint. We need to bring this back. Translations are in the works. |
@nikolaglumac thanks for reviewing, im re-installing my npm deps to see if i can reproduce your storybook problem! |
@nikolaglumac @darko-mijic i think we don't need to bring back the |
@nikolaglumac i found the issue with broken storybook (im referencing |
…ithub.com/input-output-hk/daedalus into feature/daef-300-exporting-wallet-to-file
@nikolaglumac all issues apart from translations are fixed now! |
Great @DominikGuzei - will review again in the evening ;) |
@DominikGuzei I have checked your update and fixed some minor theming/styling issues on Settings screen as well as loading inline-svgs in storybook. |
…ithub.com/input-output-hk/daedalus into feature/daef-300-exporting-wallet-to-file
@nikolaglumac i have fixed the remaining issues with storybook! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great now!
Approved ;)
This PR adds the "export to file" feature on wallet settings, which currently doesn't work correctly on the backend (it exports all wallets) but is mostly done on the frontend. I think we should wait with implementing acceptance tests for the feature until we have a working backend. If the wallet has a spending password it is also asked for but currently not passed to the backend since that's not supported yet.
Apart from the feature, the following smaller fixes and improvements have been made:
TODO: