Skip to content

Commit f55869f

Browse files
authored
fix: advanced charts interactions (#28653)
## **Description** Fix chart interaction lag on token details page by connecting the advanced chart to the existing PriceChartContext to disable page scroll during zoom/pan gestures. ## **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: Fix AC interactions in token details. ## **Related issues** Fixes: https://consensyssoftware.atlassian.net/browse/ASSETS-3041 ## **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] --> https://github.com/user-attachments/assets/8906b9d2-9983-4a7c-91b7-dcf92a03678d ### **After** <!-- [screenshots/recordings] --> https://github.com/user-attachments/assets/c7c6fe72-6611-4a8f-a1aa-b866cfb28971 ## **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] > **Medium Risk** > Adds new touch handlers that mutate `PriceChartContext` state and removes a local `PriceChartProvider` wrapper, so incorrect provider placement could cause runtime errors or regress gesture/scroll behavior. > > **Overview** > Connects the advanced token overview chart to `PriceChartContext` by toggling `setIsChartBeingTouched(true/false)` on touch start/end/cancel via a new touchable container (`advanced-chart-touch-container`). > > Updates `Price.advanced` tests to mock the context and assert the touch lifecycle calls, and removes the now-redundant `PriceChartProvider` wrapper around `Price` in token details `AssetOverviewContent`. > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit ba54dd8. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup> <!-- /CURSOR_SUMMARY -->
1 parent 4aedbb4 commit f55869f

File tree

3 files changed

+85
-14
lines changed

3 files changed

+85
-14
lines changed

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

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

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

11+
const mockSetIsChartBeingTouched = jest.fn();
12+
jest.mock('../PriceChart/PriceChart.context', () => ({
13+
usePriceChart: () => ({
14+
isChartBeingTouched: false,
15+
setIsChartBeingTouched: mockSetIsChartBeingTouched,
16+
}),
17+
}));
18+
1119
jest.mock('react-redux', () => {
1220
const actual = jest.requireActual('react-redux');
1321
return {
@@ -283,4 +291,54 @@ describe('PriceAdvanced', () => {
283291
const { getByTestId } = render(<PriceAdvanced {...baseProps} />);
284292
expect(getByTestId('price-label')).toBeOnTheScreen();
285293
});
294+
295+
describe('touch gesture handling', () => {
296+
it('sets isChartBeingTouched to true on touch start', () => {
297+
const { getByTestId } = render(<PriceAdvanced {...baseProps} />);
298+
const chartContainer = getByTestId('advanced-chart-touch-container');
299+
300+
fireEvent(chartContainer, 'touchStart');
301+
302+
expect(mockSetIsChartBeingTouched).toHaveBeenCalledWith(true);
303+
});
304+
305+
it('sets isChartBeingTouched to false on touch end', () => {
306+
const { getByTestId } = render(<PriceAdvanced {...baseProps} />);
307+
const chartContainer = getByTestId('advanced-chart-touch-container');
308+
309+
fireEvent(chartContainer, 'touchStart');
310+
expect(mockSetIsChartBeingTouched).toHaveBeenCalledWith(true);
311+
312+
fireEvent(chartContainer, 'touchEnd');
313+
expect(mockSetIsChartBeingTouched).toHaveBeenCalledWith(false);
314+
});
315+
316+
it('sets isChartBeingTouched to false on touch cancel', () => {
317+
const { getByTestId } = render(<PriceAdvanced {...baseProps} />);
318+
const chartContainer = getByTestId('advanced-chart-touch-container');
319+
320+
fireEvent(chartContainer, 'touchStart');
321+
expect(mockSetIsChartBeingTouched).toHaveBeenCalledWith(true);
322+
323+
fireEvent(chartContainer, 'touchCancel');
324+
expect(mockSetIsChartBeingTouched).toHaveBeenCalledWith(false);
325+
});
326+
327+
it('handles multiple touch start/end cycles', () => {
328+
const { getByTestId } = render(<PriceAdvanced {...baseProps} />);
329+
const chartContainer = getByTestId('advanced-chart-touch-container');
330+
331+
fireEvent(chartContainer, 'touchStart');
332+
expect(mockSetIsChartBeingTouched).toHaveBeenCalledWith(true);
333+
334+
fireEvent(chartContainer, 'touchEnd');
335+
expect(mockSetIsChartBeingTouched).toHaveBeenCalledWith(false);
336+
337+
fireEvent(chartContainer, 'touchStart');
338+
expect(mockSetIsChartBeingTouched).toHaveBeenCalledWith(true);
339+
340+
fireEvent(chartContainer, 'touchEnd');
341+
expect(mockSetIsChartBeingTouched).toHaveBeenCalledWith(false);
342+
});
343+
});
286344
});

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

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,7 @@ import { MetaMetricsEvents } from '../../../../core/Analytics';
4545
import { useAnalytics } from '../../../hooks/useAnalytics/useAnalytics';
4646
import { selectTokenOverviewChartType } from '../../../../reducers/user/selectors';
4747
import { setTokenOverviewChartType } from '../../../../actions/user';
48+
import { usePriceChart } from '../PriceChart/PriceChart.context';
4849

4950
const EMPTY_INDICATORS: IndicatorType[] = [];
5051

@@ -135,12 +136,21 @@ const PriceAdvanced = ({
135136
const [crosshairData, setCrosshairData] = useState<CrosshairData | null>(
136137
null,
137138
);
139+
const { setIsChartBeingTouched } = usePriceChart();
138140

139141
const handleCrosshairMove = useCallback(
140142
(data: CrosshairData | null) => setCrosshairData(data),
141143
[],
142144
);
143145

146+
const handleTouchStart = useCallback(() => {
147+
setIsChartBeingTouched(true);
148+
}, [setIsChartBeingTouched]);
149+
150+
const handleTouchEnd = useCallback(() => {
151+
setIsChartBeingTouched(false);
152+
}, [setIsChartBeingTouched]);
153+
144154
const handleChartInteracted = useCallback(
145155
(payload: ChartInteractedPayload) => {
146156
trackEvent(
@@ -346,7 +356,13 @@ const PriceAdvanced = ({
346356
{crosshairData && chartType === ChartType.Candles && (
347357
<OHLCVBar data={crosshairData} currency={currentCurrency} />
348358
)}
349-
<View style={[styles.chartContainer, { height: CHART_HEIGHT }]}>
359+
<View
360+
testID="advanced-chart-touch-container"
361+
style={[styles.chartContainer, { height: CHART_HEIGHT }]}
362+
onTouchStart={handleTouchStart}
363+
onTouchEnd={handleTouchEnd}
364+
onTouchCancel={handleTouchEnd}
365+
>
350366
{showEmptyState ? (
351367
<NoDataOverlay
352368
hasInsufficientData={hasInsufficientData}

app/components/UI/TokenDetails/components/AssetOverviewContent.tsx

Lines changed: 10 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,6 @@ import Price from '../../AssetOverview/Price';
4343
import ChartNavigationButton from '../../AssetOverview/ChartNavigationButton';
4444
import Balance from '../../AssetOverview/Balance';
4545
import TokenDetails from '../../AssetOverview/TokenDetails';
46-
import { PriceChartProvider } from '../../AssetOverview/PriceChart/PriceChart.context';
4746
import AssetDetailsActions from '../../../Views/AssetDetails/AssetDetailsActions';
4847
import { TokenDetailsActions } from './TokenDetailsActions';
4948
import AssetOverviewClaimBonus from '../../Earn/components/AssetOverviewClaimBonus';
@@ -812,18 +811,16 @@ const AssetOverviewContent: React.FC<AssetOverviewContentProps> = ({
812811
</Box>
813812
)}
814813

815-
<PriceChartProvider>
816-
<Price
817-
asset={token}
818-
prices={prices}
819-
timePeriod={timePeriod}
820-
priceDiff={priceDiff}
821-
currentCurrency={currentCurrency}
822-
currentPrice={currentPrice}
823-
comparePrice={comparePrice}
824-
isLoading={isLoading}
825-
/>
826-
</PriceChartProvider>
814+
<Price
815+
asset={token}
816+
prices={prices}
817+
timePeriod={timePeriod}
818+
priceDiff={priceDiff}
819+
currentCurrency={currentCurrency}
820+
currentPrice={currentPrice}
821+
comparePrice={comparePrice}
822+
isLoading={isLoading}
823+
/>
827824
{/* Same as main: chart period tabs under the legacy line chart. Omitted when the advanced chart is on (range selector lives inside Price). */}
828825
{!isTokenOverviewAdvancedChartEnabled && (
829826
<View style={styles.chartNavigationWrapper}>

0 commit comments

Comments
 (0)