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.

Sign up for GitHub

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

Jump to bottom

[Seedless-Onboarding] export account #2610

Merged
merged 13 commits into from
Oct 20, 2023
Merged

[Seedless-Onboarding] export account #2610

merged 13 commits into from
Oct 20, 2023

Conversation

schmanu
Copy link
Member

@schmanu schmanu commented Oct 10, 2023 •
edited
Loading

What it solves

Button to export private key of a signer account.

How to test it

Navigate to Settings->Signer account
Click export

Screenshots

Screenshot 2023-10-19 at 17 44 35
Screenshot 2023-10-19 at 17 44 41
Screenshot 2023-10-19 at 17 46 13

Checklist

  • I've tested the branch on mobile 📱
  • I've documented how it affects the analytics (if at all) 📊
  • I've written a unit/e2e test for it (if applicable) 🧑‍💻

@schmanu schmanu requested a review from usame-algan October 10, 2023 14:14
@github-actions
Copy link

github-actions bot commented Oct 10, 2023 •
edited
Loading

Branch preview

✅ Deploy successful!

https://seedless_export_pk--walletweb.review-wallet-web.5afe.dev

@schmanu schmanu changed the title Seedless export pk [Seedless-Onboarding] export account Oct 10, 2023
@github-actions
Copy link

github-actions bot commented Oct 10, 2023 •
edited
Loading

ESLint Summary View Full Report

Annotations are provided inline on the Files Changed tab. You can also see all annotations that were generated on the annotations page.

Type Occurrences Fixable
Errors 0 0
Warnings 0 0
Ignored 0 N/A
  • Result: ✅ success
  • Annotations: 0 total
Report generated by eslint-plus-action

@schmanu schmanu requested a review from usame-algan October 19, 2023 15:47
@schmanu schmanu force-pushed the seedless-export-pk branch from 0752271 to 09a1626 Compare October 19, 2023 15:50
@github-actions
Copy link

github-actions bot commented Oct 20, 2023 •
edited
Loading

ESLint Summary View Full Report

Annotations are provided inline on the Files Changed tab. You can also see all annotations that were generated on the annotations page.

Type Occurrences Fixable
Errors 0 0
Warnings 0 0
Ignored 0 N/A
  • Result: ✅ success
  • Annotations: 0 total
Report generated by eslint-plus-action

@github-actions
Copy link

github-actions bot commented Oct 20, 2023 •
edited
Loading

Coverage report

St.❔
 Category Percentage Covered / Total
🟡 Statements
74.6% (-0.06% 🔻)
 9292/12456
🔴 Branches
48.71% (-0.03% 🔻)
 1864/3827
🔴 Functions
56.45% (-0.05% 🔻)
 1366/2420
🟡 Lines
76.14% (-0.07% 🔻)
 8398/11029
Show files with reduced coverage 🔻
St.❔
 File Statements Branches Functions Lines
🟡
... / useMPCWallet.ts
77.92% (-10.14% 🔻)
73.68% (-8.67% 🔻)
66.67% (-8.33% 🔻)
77.63% (-10.25% 🔻)
🔴
... / MPCWalletProvider.tsx
58.33% (-5.3% 🔻)
 100%
16.67% (-3.33% 🔻)
50% (-5.56% 🔻)

Test suite run success

991 tests passing in 140 suites.

Report generated by 🧪jest coverage report action from ad3e34f

usame-algan
usame-algan approved these changes Oct 20, 2023
View reviewed changes
Copy link
Member

@usame-algan usame-algan left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Looks good 👍 We might want to hide the form if the user is not connected via Google but we can also do it in a separate PR and maybe even for the whole settings page instead of each section.

src/components/settings/ExportMPCAccount/ExportMPCAccountModal.tsx

const ExportMPCAccountModal = ({ onClose, open }: { onClose: () => void; open: boolean }) => {
const { exportPk } = useContext(MpcWalletContext)
const [error, setError] = useState<string>()
Copy link
Member

Choose a reason for hiding this comment

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

nit: Just found out we can even use RHF to display an error separate from its input https://react-hook-form.com/docs/useformstate/errormessage

Copy link
Member

Choose a reason for hiding this comment

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

Although on second thought if we set the error via RHF it will not be persisted afair so this is probably better.

src/components/settings/ExportMPCAccount/ExportMPCAccountModal.tsx Outdated
setValue(ExportFieldNames.pk, pk)
} catch (err) {
logError(ErrorCodes._305, err)
setError('Error exporting account. Your entered password might be invalid.')
Copy link
Member

Choose a reason for hiding this comment

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

Is it possible to reuse the same string that we throw from exportPk? I would slightly reword it to sound more active e.g. Error exporting account. Make sure the password is correct.

src/components/settings/ExportMPCAccount/ExportMPCAccountModal.tsx
/>
</>
)}
{error && <ErrorMessage className={css.modalError}>{error}</ErrorMessage>}
Copy link
Member

Choose a reason for hiding this comment

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

I think the ErrorMessage component adds vertical margin by default which makes the UI jump a bit more. I would suggest removing it via the css class.

@schmanu schmanu merged commit 1b153d4 into web3authcoresdk Oct 20, 2023
7 of 8 checks passed
@schmanu schmanu deleted the seedless-export-pk branch October 20, 2023 11:27
@github-actions github-actions bot locked and limited conversation to collaborators Oct 20, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@usame-algan usame-algan usame-algan approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@schmanu @usame-algan