-
-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
feat(NOTIFY-1096): add account syncing #11190
base: main
Are you sure you want to change the base?
Conversation
CLA Signature Action: All authors have signed the CLA. You may need to manually re-run the blocking PR check if it doesn't pass in a few minutes. |
I have read the CLA Document and I hereby sign the CLA |
New and removed dependencies detected. Learn more about Socket for GitHub ↗︎
🚮 Removed packages: npm/@metamask/[email protected], npm/@metamask/[email protected], npm/@metamask/[email protected], npm/@metamask/[email protected], npm/@metamask/[email protected] |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #11190 +/- ##
==========================================
+ Coverage 55.24% 55.68% +0.43%
==========================================
Files 1558 1573 +15
Lines 37153 37497 +344
Branches 4406 4473 +67
==========================================
+ Hits 20527 20880 +353
+ Misses 16155 16126 -29
- Partials 471 491 +20 ☔ View full report in Codecov by Sentry. |
Bitrise❌❌❌ Commit hash: b9bb37a Note
|
Bitrise❌❌❌ Commit hash: 6c9b2ce Note
|
Bitrise❌❌❌ Commit hash: ab17435 Note
|
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.
LGTM!
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.
Just one small listing issue fix and you're good to go!
5752f14
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.
GTG!
Quality Gate passedIssues Measures |
Bitrise❌❌❌ Commit hash: 9ca93ba Note
Tip
|
@@ -144,4 +153,36 @@ describe('Wallet', () => { | |||
const foxIcon = screen.getByTestId(CommonSelectorsIDs.FOX_ICON); | |||
expect(foxIcon).toBeDefined(); | |||
}); | |||
it('should dispatch account syncing on mount', () => { |
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.
it('should dispatch account syncing on mount', () => { | |
it('dispatches account syncing on mount', () => { |
render(Wallet); | ||
expect(useAccountSyncing().dispatchAccountSyncing).toHaveBeenCalledTimes(1); | ||
}); | ||
it('should dispatch account syncing when appState switches from inactive|background to active', () => { |
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.
it('should dispatch account syncing when appState switches from inactive|background to active', () => { | |
it('dispatches account syncing when appState switches from inactive|background to active', () => { |
|
||
expect(addEventListener).toHaveBeenCalledTimes(1); | ||
|
||
const handleAppStateChange = addEventListener.mock.calls[0][1]; |
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.
C: Not fond of this as if we add another listener in the code, this will break this test even if the feature works.
const handleAppStateChange = addEventListener.mock.calls[0][1]; | |
expect(addEventListener).toHaveBeenCalledWith('change', expect.any(Function)); | |
const handleAppStateChange = (addEventListener as jest.Mock).mock.calls.find( | |
([event]) => event === 'change' | |
)[1]; |
Description
This PR adds account syncing to MetaMask.
As part of profile syncing, with this new feature, users will be able to synchronize all of their accounts and accounts' names across all of their devices.
Related issues
Fixes:
NOTIFY-1096
Manual testing steps
Pre-merge author checklist
Pre-merge reviewer checklist