Skip to content

Commit 413327c

Browse files
sahar-fehrisahar-fehri
authored
fix: persist chart type in state (#28580)
## **Description** Implemented persistent chart type preference (Line vs Candlestick) for Advanced Charts across all asset pages using Redux user state with automatic persistence via redux-persist. ## Problem When users switched between Line and Candlestick chart types, the preference was not saved. Upon navigating away and returning to any token details page, the chart would always default to Line chart. ## Solution Store the chart type preference in Redux `user` state, which is automatically persisted to filesystem storage via `redux-persist`. ## **Changelog** <!-- If this PR is not End-User-Facing and should not show up in the CHANGELOG, you can choose to either: 1. Write `CHANGELOG entry: null` 2. Label with `no-changelog` If this PR is End-User-Facing, please write a short User-Facing description in the past tense like: `CHANGELOG entry: Added a new tab for users to see their NFTs` `CHANGELOG entry: Fixed a bug that was causing some NFTs to flicker` (This helps the Release Engineer do their job more quickly and accurately) --> CHANGELOG entry: Persist chart type upon switch in user state ## **Related issues** Fixes: https://consensyssoftware.atlassian.net/browse/ASSETS-3034 ## **Manual testing steps** ```gherkin Feature: my feature name Scenario: user [verb for user action] Given [describe expected initial app state] When user [verb for user action] Then [describe expected outcome] ``` ## **Screenshots/Recordings** <!-- If applicable, add screenshots and/or recordings to visualize the before and after of your change. --> ### **Before** <!-- [screenshots/recordings] --> ### **After** <!-- [screenshots/recordings] --> ## **Pre-merge author checklist** - [ ] 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). - [ ] I've completed the PR template to the best of my ability - [ ] I've included tests if applicable - [ ] I've documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [ ] 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] > **Low Risk** > Small Redux state extension and wiring change; low risk aside from potential regressions in chart type toggle behavior and persisted-state defaults. > > **Overview** > Persists the Advanced Token Overview chart type (line vs candlestick) by moving `chartType` from local component state into Redux `user` state. > > Adds `UserActionType.SET_TOKEN_OVERVIEW_CHART_TYPE`, `setTokenOverviewChartType`, `tokenOverviewChartType` in the user reducer (defaulting to `ChartType.Line`), plus `selectTokenOverviewChartType`; updates `Price.advanced` to read/write this preference via `useSelector`/`dispatch`, and adjusts/extends unit tests for the component, selector, and reducer. > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit e3991d0. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup> <!-- /CURSOR_SUMMARY -->
1 parent c8f106d commit 413327c

10 files changed

Lines changed: 169 additions & 4 deletions

File tree

‎app/actions/user/index.ts‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import { type AppThemeKey } from '../../util/theme/models';
2+
import { type ChartType } from '../../components/UI/Charts/AdvancedChart/AdvancedChart.types';
23
import {
34
type LockAppAction,
45
type CheckForDeeplinkAction,
@@ -24,6 +25,7 @@ import {
2425
type SetMultichainAccountsIntroModalSeenAction,
2526
type SetMusdConversionEducationSeenAction,
2627
type SetMusdConversionAssetDetailCtaSeenAction,
28+
type SetTokenOverviewChartTypeAction,
2729
UserActionType,
2830
} from './types';
2931

@@ -213,3 +215,15 @@ export function setMusdConversionAssetDetailCtaSeen(
213215
payload: { key },
214216
};
215217
}
218+
219+
/**
220+
* Action to set token overview chart type preference
221+
*/
222+
export function setTokenOverviewChartType(
223+
chartType: ChartType,
224+
): SetTokenOverviewChartTypeAction {
225+
return {
226+
type: UserActionType.SET_TOKEN_OVERVIEW_CHART_TYPE,
227+
payload: { chartType },
228+
};
229+
}

‎app/actions/user/types.ts‎

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import { type AppThemeKey } from '../../util/theme/models';
22
import { type Action } from 'redux';
3+
import { type ChartType } from '../../components/UI/Charts/AdvancedChart/AdvancedChart.types';
34

45
// Action type enum
56
export enum UserActionType {
@@ -27,6 +28,7 @@ export enum UserActionType {
2728
SET_MULTICHAIN_ACCOUNTS_INTRO_MODAL_SEEN = 'SET_MULTICHAIN_ACCOUNTS_INTRO_MODAL_SEEN',
2829
SET_MUSD_CONVERSION_EDUCATION_SEEN = 'SET_MUSD_CONVERSION_EDUCATION_SEEN',
2930
SET_MUSD_CONVERSION_ASSET_DETAIL_CTA_SEEN = 'SET_MUSD_CONVERSION_ASSET_DETAIL_CTA_SEEN',
31+
SET_TOKEN_OVERVIEW_CHART_TYPE = 'SET_TOKEN_OVERVIEW_CHART_TYPE',
3032
}
3133

3234
// User actions
@@ -107,6 +109,11 @@ export type SetMusdConversionAssetDetailCtaSeenAction =
107109
payload: { key: string };
108110
};
109111

112+
export type SetTokenOverviewChartTypeAction =
113+
Action<UserActionType.SET_TOKEN_OVERVIEW_CHART_TYPE> & {
114+
payload: { chartType: ChartType };
115+
};
116+
110117
/**
111118
* User actions union type
112119
*/
@@ -134,4 +141,5 @@ export type UserAction =
134141
| SetIsConnectionRemovedAction
135142
| SetMultichainAccountsIntroModalSeenAction
136143
| SetMusdConversionEducationSeenAction
137-
| SetMusdConversionAssetDetailCtaSeenAction;
144+
| SetMusdConversionAssetDetailCtaSeenAction
145+
| SetTokenOverviewChartTypeAction;

‎app/components/UI/AssetOverview/Price/Price.advanced.test.tsx‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,15 @@ import { createMockUseAnalyticsHook } from '../../../../util/test/analyticsMock'
88

99
jest.mock('../../../hooks/useAnalytics/useAnalytics');
1010

11+
jest.mock('react-redux', () => {
12+
const actual = jest.requireActual('react-redux');
13+
return {
14+
...actual,
15+
useSelector: jest.fn(() => 2), // ChartType.Line = 2
16+
useDispatch: jest.fn(() => jest.fn()),
17+
};
18+
});
19+
1120
jest.mock('react-native-skeleton-placeholder', () => {
1221
const { View } = jest.requireActual('react-native');
1322
const MockSkeleton = ({ children }: { children: React.ReactNode }) => (

‎app/components/UI/AssetOverview/Price/Price.advanced.tsx‎

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import React, {
66
useState,
77
} from 'react';
88
import { Dimensions, View } from 'react-native';
9+
import { useDispatch, useSelector } from 'react-redux';
910
import SkeletonPlaceholder from 'react-native-skeleton-placeholder';
1011
import Svg, { Path } from 'react-native-svg';
1112
import { strings } from '../../../../../locales/i18n';
@@ -41,6 +42,8 @@ import {
4142
} from '@metamask/design-system-react-native';
4243
import { MetaMetricsEvents } from '../../../../core/Analytics';
4344
import { useAnalytics } from '../../../hooks/useAnalytics/useAnalytics';
45+
import { selectTokenOverviewChartType } from '../../../../reducers/user/selectors';
46+
import { setTokenOverviewChartType } from '../../../../actions/user';
4447

4548
const EMPTY_INDICATORS: IndicatorType[] = [];
4649

@@ -124,9 +127,10 @@ const PriceAdvanced = ({
124127
comparePrice,
125128
isLoading,
126129
}: PriceAdvancedProps) => {
130+
const dispatch = useDispatch();
127131
const { trackEvent, createEventBuilder } = useAnalytics();
128132
const [timeRange, setTimeRange] = useState<TimeRange>('1D');
129-
const [chartType, setChartType] = useState<ChartType>(ChartType.Line);
133+
const chartType = useSelector(selectTokenOverviewChartType);
130134
const [crosshairData, setCrosshairData] = useState<CrosshairData | null>(
131135
null,
132136
);
@@ -168,8 +172,8 @@ const PriceAdvanced = ({
168172
})
169173
.build(),
170174
);
171-
setChartType(next);
172-
}, [chartType, createEventBuilder, trackEvent]);
175+
dispatch(setTokenOverviewChartType(next));
176+
}, [chartType, createEventBuilder, trackEvent, dispatch]);
173177

174178
const handleTimeRangeSelect = useCallback(
175179
(range: TimeRange) => {

‎app/components/UI/AssetOverview/Price/Price.test.tsx‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,8 @@ import Price from './Price';
55
import type { TokenI } from '../../Tokens/types';
66
import { PriceChartProvider } from '../PriceChart/PriceChart.context';
77
import { selectTokenOverviewAdvancedChartEnabled } from '../../../../selectors/featureFlagController/tokenOverviewAdvancedChart';
8+
import { selectTokenOverviewChartType } from '../../../../reducers/user/selectors';
9+
import { ChartType } from '../../Charts/AdvancedChart/AdvancedChart.types';
810

911
jest.mock('../../Bridge/hooks/useRWAToken', () => ({
1012
useRWAToken: () => ({
@@ -18,6 +20,7 @@ jest.mock('react-redux', () => {
1820
return {
1921
...actual,
2022
useSelector: jest.fn(),
23+
useDispatch: jest.fn(() => jest.fn()),
2124
};
2225
});
2326

@@ -94,6 +97,9 @@ describe('Price Component', () => {
9497
if (selector === selectTokenOverviewAdvancedChartEnabled) {
9598
return false;
9699
}
100+
if (selector === selectTokenOverviewChartType) {
101+
return ChartType.Line;
102+
}
97103
return undefined;
98104
});
99105
});
@@ -103,6 +109,9 @@ describe('Price Component', () => {
103109
if (selector === selectTokenOverviewAdvancedChartEnabled) {
104110
return true;
105111
}
112+
if (selector === selectTokenOverviewChartType) {
113+
return ChartType.Line;
114+
}
106115
return undefined;
107116
});
108117
const { getByTestId } = renderWithProviders(
@@ -117,6 +126,9 @@ describe('Price Component', () => {
117126
if (selector === selectTokenOverviewAdvancedChartEnabled) {
118127
return true;
119128
}
129+
if (selector === selectTokenOverviewChartType) {
130+
return ChartType.Line;
131+
}
120132
return undefined;
121133
});
122134
mockUseOHLCVChart.mockReturnValueOnce({
@@ -138,6 +150,9 @@ describe('Price Component', () => {
138150
if (selector === selectTokenOverviewAdvancedChartEnabled) {
139151
return true;
140152
}
153+
if (selector === selectTokenOverviewChartType) {
154+
return ChartType.Line;
155+
}
141156
return undefined;
142157
});
143158
const { getByTestId } = renderWithProviders(<Price {...unifiedProps} />);

‎app/reducers/user/index.test.ts‎

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,63 @@
1+
import userReducer, { userInitialState } from './index';
2+
import {
3+
UserActionType,
4+
type SetTokenOverviewChartTypeAction,
5+
} from '../../actions/user/types';
6+
import { ChartType } from '../../components/UI/Charts/AdvancedChart/AdvancedChart.types';
7+
8+
describe('user reducer', () => {
9+
describe('initial state', () => {
10+
it('has ChartType.Line as default tokenOverviewChartType', () => {
11+
expect(userInitialState.tokenOverviewChartType).toBe(ChartType.Line);
12+
});
13+
});
14+
15+
describe('SET_TOKEN_OVERVIEW_CHART_TYPE', () => {
16+
it('updates tokenOverviewChartType to Candles', () => {
17+
const action: SetTokenOverviewChartTypeAction = {
18+
type: UserActionType.SET_TOKEN_OVERVIEW_CHART_TYPE,
19+
payload: { chartType: ChartType.Candles },
20+
};
21+
22+
const newState = userReducer(userInitialState, action);
23+
24+
expect(newState.tokenOverviewChartType).toBe(ChartType.Candles);
25+
});
26+
27+
it('updates tokenOverviewChartType to Line', () => {
28+
const currentState = {
29+
...userInitialState,
30+
tokenOverviewChartType: ChartType.Candles,
31+
};
32+
33+
const action: SetTokenOverviewChartTypeAction = {
34+
type: UserActionType.SET_TOKEN_OVERVIEW_CHART_TYPE,
35+
payload: { chartType: ChartType.Line },
36+
};
37+
38+
const newState = userReducer(currentState, action);
39+
40+
expect(newState.tokenOverviewChartType).toBe(ChartType.Line);
41+
});
42+
43+
it('does not modify other state properties', () => {
44+
const currentState = {
45+
...userInitialState,
46+
userLoggedIn: true,
47+
seedphraseBackedUp: true,
48+
tokenOverviewChartType: ChartType.Line,
49+
};
50+
51+
const action: SetTokenOverviewChartTypeAction = {
52+
type: UserActionType.SET_TOKEN_OVERVIEW_CHART_TYPE,
53+
payload: { chartType: ChartType.Candles },
54+
};
55+
56+
const newState = userReducer(currentState, action);
57+
58+
expect(newState.userLoggedIn).toBe(true);
59+
expect(newState.seedphraseBackedUp).toBe(true);
60+
expect(newState.tokenOverviewChartType).toBe(ChartType.Candles);
61+
});
62+
});
63+
});

‎app/reducers/user/index.ts‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import { UserAction, UserActionType } from '../../actions/user/types';
22
import { AppThemeKey } from '../../util/theme/models';
33
import { UserState } from './types';
4+
import { ChartType } from '../../components/UI/Charts/AdvancedChart/AdvancedChart.types';
45

56
export * from './types';
67

@@ -28,6 +29,7 @@ export const userInitialState: UserState = {
2829
multichainAccountsIntroModalSeen: false,
2930
musdConversionEducationSeen: false,
3031
musdConversionAssetDetailCtasSeen: {},
32+
tokenOverviewChartType: ChartType.Line,
3133
};
3234

3335
/**
@@ -148,6 +150,11 @@ const userReducer = (
148150
[action.payload.key]: true,
149151
},
150152
};
153+
case UserActionType.SET_TOKEN_OVERVIEW_CHART_TYPE:
154+
return {
155+
...state,
156+
tokenOverviewChartType: action.payload.chartType,
157+
};
151158
default:
152159
return state;
153160
}

‎app/reducers/user/selectors.test.ts‎

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,9 @@ import {
88
selectUserState,
99
selectMusdConversionEducationSeen,
1010
selectMusdConversionAssetDetailCtasSeen,
11+
selectTokenOverviewChartType,
1112
} from './selectors';
13+
import { ChartType } from '../../components/UI/Charts/AdvancedChart/AdvancedChart.types';
1214

1315
// Mock the redux store state
1416
const mockState = {
@@ -18,6 +20,7 @@ const mockState = {
1820
isConnectionRemoved: false,
1921
musdConversionEducationSeen: false,
2022
musdConversionAssetDetailCtasSeen: {} as Record<string, boolean>,
23+
tokenOverviewChartType: ChartType.Line as ChartType,
2124
},
2225
};
2326

@@ -118,4 +121,37 @@ describe('user state selectors', () => {
118121
});
119122
});
120123
});
124+
125+
describe('selectTokenOverviewChartType', () => {
126+
it('returns ChartType.Line when chart type is set to Line', () => {
127+
mockState.user.tokenOverviewChartType = ChartType.Line;
128+
129+
const { result } = renderHook(() =>
130+
useSelector(selectTokenOverviewChartType),
131+
);
132+
133+
expect(result.current).toBe(ChartType.Line);
134+
});
135+
136+
it('returns ChartType.Candles when chart type is set to Candles', () => {
137+
mockState.user.tokenOverviewChartType = ChartType.Candles;
138+
139+
const { result } = renderHook(() =>
140+
useSelector(selectTokenOverviewChartType),
141+
);
142+
143+
expect(result.current).toBe(ChartType.Candles);
144+
});
145+
146+
it('returns ChartType.Line default when tokenOverviewChartType is not set', () => {
147+
// @ts-expect-error - Testing undefined state
148+
mockState.user.tokenOverviewChartType = undefined;
149+
150+
const { result } = renderHook(() =>
151+
useSelector(selectTokenOverviewChartType),
152+
);
153+
154+
expect(result.current).toBe(ChartType.Line);
155+
});
156+
});
121157
});

‎app/reducers/user/selectors.ts‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import { RootState } from '..';
2+
import { ChartType } from '../../components/UI/Charts/AdvancedChart/AdvancedChart.types';
23

34
/**
45
* Selects the user state
@@ -55,3 +56,9 @@ export const selectMusdConversionEducationSeen = (state: RootState) =>
5556
*/
5657
export const selectMusdConversionAssetDetailCtasSeen = (state: RootState) =>
5758
state.user?.musdConversionAssetDetailCtasSeen ?? {};
59+
60+
/**
61+
* Selects the token overview chart type preference
62+
*/
63+
export const selectTokenOverviewChartType = (state: RootState) =>
64+
state.user?.tokenOverviewChartType ?? ChartType.Line;

‎app/reducers/user/types.ts‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import { AppThemeKey } from '../../util/theme/models';
2+
import { ChartType } from '../../components/UI/Charts/AdvancedChart/AdvancedChart.types';
23

34
/**
45
* User state
@@ -22,4 +23,5 @@ export interface UserState {
2223
multichainAccountsIntroModalSeen: boolean;
2324
musdConversionEducationSeen: boolean;
2425
musdConversionAssetDetailCtasSeen: Record<string, boolean>;
26+
tokenOverviewChartType: ChartType;
2527
}

0 commit comments

Comments
 (0)