Skip to content

Commit c594a57

Browse files
fix: prevent balance empty state flash during wallet import and account switching (MetaMask#23351)
## **Description** This PR creates a temporary fix for a UX issue where the "Fund your wallet" empty state briefly flashes when importing a wallet with existing funds or switching between funded accounts. This happened because the balance calculation returned $0 while price and balance data were still loading, causing the empty state to appear before the actual balance was displayed. _**"Telling a user they need to fund an account that might have thousands of dollars in it is confusing at best and mildly panic-inducing at worst."**_ ### Problem PR MetaMask#21391 introduced a balance empty state that displays when users have zero mainnet balance. However, due to how balance data loads asynchronously, users would see: 1. Skeleton loader (correct) 2. **"Fund your wallet" empty state (incorrect - account has funds!)** 3. Actual balance appears (e.g., $49.21) This flash occurred because: - Balance calculations depend on multiple data sources (price data, token balances, native balances) - These data sources load at different times via polling - The `AccountTrackerController `always initializes accounts with `balance: "0x0"` - We cannot distinguish between "balance is loading" vs "balance is genuinely $0" ### Solutions Considered We evaluated several approaches to solve this issue: #### Option 1: Add Loading State to `AccountTrackerController` ❌ - **Approach**: Modify the core controller to track whether native balances have been fetched - **Why rejected**: This is the ideal state but would require changes to `@metamask/assets-controllers` (shared between mobile and extension), introducing complexity and breaking type changes across the codebase. Too large of a scope for this fix. #### Option 2: Check Controller Initialization State ❌ - **Approach**: Create a selector that validates all price/balance controllers have initialized - **Why rejected**: Controllers persist state via redux-persist, so they always appear "initialized" even with stale data from previous sessions. Logs confirmed this - all controllers report "state persisted successfully" immediately on load. #### Option 3: Balance Change Tracking + Timeout ✅ (Chosen) - **Approach**: Track when balance values actually change from their initial value, with a timeout fallback - **Why chosen**: - Works with existing architecture without controller modifications - Relies on observable behavior (balance updates) rather than internal controller state - Handles all edge cases: fresh imports, account switching, genuinely empty wallets - Simple component-level solution - **Most importantly**: Prevents showing "fund your wallet" to users with existing funds ### Implementation The solution adds balance change tracking to `AccountGroupBalance` component: 1. **Track account switches** via `groupId` changes - resets all tracking state 2. **Monitor balance changes** from initial $0 value - marks as "fetched" when balance updates 3. **Fallback timeout** (3 seconds) - handles genuinely $0 balances or slow API responses 4. **Dual balance tracking** - watches both `groupBalance` and `accountGroupBalance` since empty state decision uses the latter The skeleton loader now displays until either: - Balance changes from its initial value, OR - 3-second timeout expires This ensures users never see the empty state flash for funded accounts. ## **Changelog** CHANGELOG entry: Fixed an issue where "Fund your wallet" empty state briefly appeared when importing wallets with existing funds or switching between funded accounts ## **Related issues** Fixes: (related to MetaMask#21391 - balance empty state implementation) ## **Manual testing steps** \`\`\`gherkin Feature: Balance empty state loading behavior Scenario: Import wallet with funded accounts Given the app is freshly installed When user imports a wallet using SRP with funded accounts Then user should see skeleton loader And user should NOT see "Fund your wallet" empty state And user should see actual balance (e.g., "$49.21") Scenario: Switch between funded accounts Given user has multiple accounts with funds When user switches from Account 1 to Account 2 Then user should see skeleton loader briefly And user should NOT see "Fund your wallet" empty state And user should see Account 2's balance Scenario: Import wallet with zero balance Given the app is freshly installed When user imports a wallet with zero mainnet balance Then user should see skeleton loader And after 3 seconds, user should see "Fund your wallet" empty state Scenario: Switch to account with zero balance Given user has account with zero mainnet balance When user switches to the zero-balance account Then user should see skeleton loader And after 3 seconds, user should see "Fund your wallet" empty state \`\`\` ## **Screenshots/Recordings** ### **Before** Empty state incorrectly flashed when importing wallet with funds or switching accounts. https://github.com/user-attachments/assets/945567d9-e9b1-458d-aecc-66f05384bcc9 ### **After** Skeleton loader now prevents layout shift displays until balance is confirmed, preventing false "fund your wallet" messaging. https://github.com/user-attachments/assets/f1763e5c-f9b4-43c3-98a5-4525a2e83a9c ## **Pre-merge author checklist** - [x] I've followed [MetaMask Contributor Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Mobile Coding Standards](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/CODING_GUIDELINES.md). - [x] I've completed the PR template to the best of my ability - [x] I've included tests if applicable - [x] I've documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [x] I've applied the right labels on the PR (see [labeling guidelines](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/LABELING_GUIDELINES.md)). Not required for external contributors. ## **Pre-merge reviewer checklist** - [ ] I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed). - [ ] I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots. <!-- CURSOR_SUMMARY --> --- > [!NOTE] > Prevents the “Fund your wallet” flash by tracking balance fetch with a 3s timeout and using Skeleton to hide content until ready; updates Skeleton behavior and related tests/styles. > > - **Account Group Balance** > - Implement balance fetch tracking with a 3s timeout using `hasBalanceFetched`, `groupId` change detection, and initial balance refs in `AccountGroupBalance.tsx`. > - Gate empty state rendering (`BalanceEmptyState`) until loading completes; compute `isLoading = !groupBalance || !hasBalanceFetched`. > - Wrap balance and change components with `Skeleton` using `hideChildren` to avoid content flash. > - Adjust styles to column layout and left alignment in `AccountGroupBalance.styles.ts`. > - **Skeleton Component** > - Change render logic to return `children` directly when `hideChildren` is false; otherwise render animated placeholder and optional children wrapper. > - **Tests** > - Add timer-based tests covering timeout-driven display, immediate display on balance change, and zero-balance empty state. > - Update Skeleton tests to reflect new child rendering behavior and animation conditions. > - Remove redundant story/use case and align stories with new `hideChildren` flow. > > <sup>Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit 4396697. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot).</sup> <!-- /CURSOR_SUMMARY --> --------- Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
1 parent 896144b commit c594a57

6 files changed

Lines changed: 302 additions & 102 deletions

File tree

app/component-library/components/Skeleton/Skeleton.stories.tsx

Lines changed: 0 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -56,28 +56,6 @@ export const WidthHeight = () => {
5656
);
5757
};
5858

59-
export const WithChildren = () => {
60-
const styles = StyleSheet.create({
61-
container: {
62-
display: 'flex',
63-
flexDirection: 'column',
64-
borderRadius: 12,
65-
padding: 16,
66-
},
67-
skeleton: {
68-
marginBottom: 8,
69-
},
70-
});
71-
72-
return (
73-
<SkeletonComponent style={styles.container}>
74-
<SkeletonComponent height={32} width="100%" style={styles.skeleton} />
75-
<SkeletonComponent height={16} width="95%" style={styles.skeleton} />
76-
<SkeletonComponent height={16} width="95%" />
77-
</SkeletonComponent>
78-
);
79-
};
80-
8159
export const HideChildren = () => {
8260
const [isLoaded, setIsLoaded] = useState(false);
8361
const styles = StyleSheet.create({

app/component-library/components/Skeleton/Skeleton.test.tsx

Lines changed: 2 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -57,22 +57,6 @@ describe('Skeleton', () => {
5757
expect(skeletonElement.props.style.width).toBe('100%');
5858
});
5959

60-
it('should render with children', () => {
61-
const childTestId = 'child-component';
62-
const childrenWrapperId = 'children-wrapper';
63-
64-
const { getByTestId } = render(
65-
<Skeleton childrenWrapperProps={{ testID: childrenWrapperId }}>
66-
<View testID={childTestId} />
67-
</Skeleton>,
68-
);
69-
70-
const childElement = getByTestId(childTestId);
71-
const childrenWrapper = getByTestId(childrenWrapperId);
72-
expect(childElement).toBeDefined();
73-
expect(childrenWrapper).toBeDefined();
74-
});
75-
7660
it('should hide children when hideChildren is true', () => {
7761
const childTestId = 'child-component';
7862
const childrenWrapperId = 'children-wrapper';
@@ -92,16 +76,14 @@ describe('Skeleton', () => {
9276

9377
it('should display children normally when hideChildren is false', () => {
9478
const childTestId = 'child-component';
95-
const childrenWrapperId = 'children-wrapper';
9679

9780
const { getByTestId } = render(
98-
<Skeleton childrenWrapperProps={{ testID: childrenWrapperId }}>
81+
<Skeleton>
9982
<View testID={childTestId} />
10083
</Skeleton>,
10184
);
10285

103-
const childrenWrapper = getByTestId(childrenWrapperId);
104-
expect(childrenWrapper.props.style[1]).toBe(undefined);
86+
expect(getByTestId(childTestId)).toBeDefined();
10587
});
10688

10789
it('should animate when no children are present', () => {

app/component-library/components/Skeleton/Skeleton.tsx

Lines changed: 23 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -63,26 +63,32 @@ const Skeleton: React.FC<SkeletonProps> = ({
6363
}, [children, hideChildren]); // eslint-disable-line react-hooks/exhaustive-deps
6464

6565
return (
66-
<View style={styles.base} {...props}>
67-
{/* Animated background always present */}
68-
<Animated.View
69-
style={[styles.background, { opacity: opacityAnim }]}
70-
pointerEvents="none"
71-
{...animatedViewProps}
72-
/>
66+
<>
67+
{!hideChildren && children ? (
68+
children
69+
) : (
70+
<View style={styles.base} {...props}>
71+
{/* Animated background always present */}
72+
<Animated.View
73+
style={[styles.background, { opacity: opacityAnim }]}
74+
pointerEvents="none"
75+
{...animatedViewProps}
76+
/>
7377

74-
{children && (
75-
<View
76-
style={[
77-
styles.childrenContainer,
78-
hideChildren ? styles.hideChildren : undefined,
79-
]}
80-
{...childrenWrapperProps}
81-
>
82-
{children}
78+
{children && (
79+
<View
80+
style={[
81+
styles.childrenContainer,
82+
hideChildren ? styles.hideChildren : undefined,
83+
]}
84+
{...childrenWrapperProps}
85+
>
86+
{children}
87+
</View>
88+
)}
8389
</View>
8490
)}
85-
</View>
91+
</>
8692
);
8793
};
8894

app/components/UI/Assets/components/Balance/AccountGroupBalance.styles.ts

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5,12 +5,9 @@ const createStyles = () =>
55
marginHorizontal: 16,
66
},
77
balanceContainer: {
8-
flexDirection: 'row',
9-
alignItems: 'center',
10-
},
11-
skeletonContainer: {
128
flexDirection: 'column',
139
gap: 4,
10+
alignItems: 'flex-start',
1411
},
1512
});
1613

app/components/UI/Assets/components/Balance/AccountGroupBalance.test.tsx

Lines changed: 173 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import React from 'react';
2+
import { act } from '@testing-library/react-native';
23
import AccountGroupBalance from './AccountGroupBalance';
34
import { WalletViewSelectorsIDs } from '../../../../../../e2e/selectors/wallet/WalletView.selectors';
45
import renderWithProvider from '../../../../../util/test/renderWithProvider';
@@ -56,15 +57,41 @@ const testState = {
5657
};
5758

5859
describe('AccountGroupBalance', () => {
59-
it('renders loader when balance is not ready', () => {
60-
const { queryByTestId } = renderWithProvider(<AccountGroupBalance />, {
60+
beforeEach(() => {
61+
jest.clearAllMocks();
62+
// Reset mock implementations to default (null) before each test
63+
const {
64+
selectBalanceBySelectedAccountGroup,
65+
selectAccountGroupBalanceForEmptyState,
66+
selectBalanceChangeBySelectedAccountGroup,
67+
} = jest.requireMock('../../../../../selectors/assets/balances');
68+
(selectBalanceBySelectedAccountGroup as jest.Mock).mockImplementation(
69+
() => null,
70+
);
71+
(selectAccountGroupBalanceForEmptyState as jest.Mock).mockImplementation(
72+
() => null,
73+
);
74+
(selectBalanceChangeBySelectedAccountGroup as jest.Mock).mockImplementation(
75+
() => () => null,
76+
);
77+
jest.useFakeTimers();
78+
});
79+
80+
afterEach(() => {
81+
jest.runOnlyPendingTimers();
82+
jest.useRealTimers();
83+
});
84+
85+
it('renders without crashing when balance is not ready', () => {
86+
const { getByTestId } = renderWithProvider(<AccountGroupBalance />, {
6187
state: testState,
6288
});
6389

64-
expect(queryByTestId(WalletViewSelectorsIDs.TOTAL_BALANCE_TEXT)).toBeNull();
90+
// Component should render the balance container even when loading
91+
expect(getByTestId('balance-container')).toBeOnTheScreen();
6592
});
6693

67-
it('renders formatted balance when selector returns data', () => {
94+
it('renders formatted balance when selector returns data and timeout expires', () => {
6895
const { selectBalanceBySelectedAccountGroup } = jest.requireMock(
6996
'../../../../../selectors/assets/balances',
7097
);
@@ -81,20 +108,27 @@ describe('AccountGroupBalance', () => {
81108
state: testState,
82109
});
83110

111+
// After timeout expires (3 seconds), balance should display
112+
act(() => {
113+
jest.advanceTimersByTime(3000);
114+
});
115+
84116
const el = getByTestId(WalletViewSelectorsIDs.TOTAL_BALANCE_TEXT);
85-
expect(el).toBeTruthy();
117+
expect(el).toBeOnTheScreen();
86118
});
87119

88-
it('renders empty state when account group balance is zero', () => {
120+
it('renders empty state when account group balance is zero after timeout', () => {
89121
const {
90122
selectAccountGroupBalanceForEmptyState,
91123
selectBalanceBySelectedAccountGroup,
92124
} = jest.requireMock('../../../../../selectors/assets/balances');
93125

94-
// Mock the regular balance selector to return data (prevents skeleton loader)
126+
// Mock the regular balance selector to return zero balance data
95127
(selectBalanceBySelectedAccountGroup as jest.Mock).mockImplementation(
96128
() => ({
97-
totalBalanceInUserCurrency: 100, // Some non-zero amount for current network
129+
walletId: 'wallet-1',
130+
groupId: 'wallet-1/group-1',
131+
totalBalanceInUserCurrency: 0, // Zero on current network
98132
userCurrency: 'usd',
99133
}),
100134
);
@@ -107,13 +141,142 @@ describe('AccountGroupBalance', () => {
107141
}),
108142
);
109143

110-
const { getByTestId } = renderWithProvider(<AccountGroupBalance />, {
111-
state: testState,
144+
const { getByTestId, queryByTestId } = renderWithProvider(
145+
<AccountGroupBalance />,
146+
{
147+
state: testState,
148+
},
149+
);
150+
151+
// Initially shows loader because hasBalanceFetched is false
152+
expect(
153+
queryByTestId(WalletViewSelectorsIDs.BALANCE_EMPTY_STATE_CONTAINER),
154+
).toBeNull();
155+
156+
// After timeout expires (3 seconds), empty state should display
157+
act(() => {
158+
jest.advanceTimersByTime(3000);
112159
});
113160

114161
const el = getByTestId(
115162
WalletViewSelectorsIDs.BALANCE_EMPTY_STATE_CONTAINER,
116163
);
117164
expect(el).toBeOnTheScreen();
118165
});
166+
167+
it('renders balance immediately when balance changes from 0 to non-zero before timeout', () => {
168+
const {
169+
selectBalanceBySelectedAccountGroup,
170+
selectAccountGroupBalanceForEmptyState,
171+
} = jest.requireMock('../../../../../selectors/assets/balances');
172+
173+
// Start with zero balance
174+
(selectBalanceBySelectedAccountGroup as jest.Mock).mockImplementation(
175+
() => ({
176+
walletId: 'wallet-1',
177+
groupId: 'wallet-1/group-1',
178+
totalBalanceInUserCurrency: 0,
179+
userCurrency: 'usd',
180+
}),
181+
);
182+
183+
(selectAccountGroupBalanceForEmptyState as jest.Mock).mockImplementation(
184+
() => ({
185+
totalBalanceInUserCurrency: 0,
186+
userCurrency: 'usd',
187+
}),
188+
);
189+
190+
const { getByTestId, rerender } = renderWithProvider(
191+
<AccountGroupBalance />,
192+
{
193+
state: testState,
194+
},
195+
);
196+
197+
// Update mocks to return non-zero balance (simulating balance fetch completing)
198+
(selectBalanceBySelectedAccountGroup as jest.Mock).mockImplementation(
199+
() => ({
200+
walletId: 'wallet-1',
201+
groupId: 'wallet-1/group-1',
202+
totalBalanceInUserCurrency: 123.45,
203+
userCurrency: 'usd',
204+
}),
205+
);
206+
207+
(selectAccountGroupBalanceForEmptyState as jest.Mock).mockImplementation(
208+
() => ({
209+
totalBalanceInUserCurrency: 123.45,
210+
userCurrency: 'usd',
211+
}),
212+
);
213+
214+
// Trigger re-render with new balance
215+
rerender(<AccountGroupBalance />);
216+
217+
// Balance should display immediately without waiting for timeout
218+
const el = getByTestId(WalletViewSelectorsIDs.TOTAL_BALANCE_TEXT);
219+
expect(el).toBeOnTheScreen();
220+
});
221+
222+
it('renders balance after updating when initially zero', () => {
223+
const {
224+
selectBalanceBySelectedAccountGroup,
225+
selectAccountGroupBalanceForEmptyState,
226+
} = jest.requireMock('../../../../../selectors/assets/balances');
227+
228+
// Start with zero balance (simulates account with no funds or just switched)
229+
(selectBalanceBySelectedAccountGroup as jest.Mock).mockImplementation(
230+
() => ({
231+
walletId: 'wallet-1',
232+
groupId: 'wallet-1/group-1',
233+
totalBalanceInUserCurrency: 0,
234+
userCurrency: 'usd',
235+
}),
236+
);
237+
238+
(selectAccountGroupBalanceForEmptyState as jest.Mock).mockImplementation(
239+
() => ({
240+
totalBalanceInUserCurrency: 0,
241+
userCurrency: 'usd',
242+
}),
243+
);
244+
245+
const { getByTestId, rerender } = renderWithProvider(
246+
<AccountGroupBalance />,
247+
{
248+
state: testState,
249+
},
250+
);
251+
252+
// Advance time less than timeout
253+
act(() => {
254+
jest.advanceTimersByTime(1000);
255+
});
256+
257+
// Update mocks to show balance has loaded with funds
258+
(selectBalanceBySelectedAccountGroup as jest.Mock).mockImplementation(
259+
() => ({
260+
walletId: 'wallet-1',
261+
groupId: 'wallet-1/group-1',
262+
totalBalanceInUserCurrency: 150,
263+
userCurrency: 'usd',
264+
}),
265+
);
266+
267+
(selectAccountGroupBalanceForEmptyState as jest.Mock).mockImplementation(
268+
() => ({
269+
totalBalanceInUserCurrency: 150,
270+
userCurrency: 'usd',
271+
}),
272+
);
273+
274+
// Trigger re-render
275+
rerender(<AccountGroupBalance />);
276+
277+
// Should show balance immediately after update (hasChanged condition)
278+
expect(
279+
getByTestId(WalletViewSelectorsIDs.TOTAL_BALANCE_TEXT),
280+
).toBeOnTheScreen();
281+
});
119282
});

0 commit comments

Comments
 (0)