Skip to content

chore: fix time range change race condition cp-7.74.0#28939

Merged
sahar-fehri merged 3 commits into
mainfrom
fix/reset-webview-on-timeRange-change
Apr 16, 2026
Merged

chore: fix time range change race condition cp-7.74.0#28939
sahar-fehri merged 3 commits into
mainfrom
fix/reset-webview-on-timeRange-change

Conversation

@sahar-fehri
Copy link
Copy Markdown
Contributor

@sahar-fehri sahar-fehri commented Apr 16, 2026

Description

Fixes the token advanced chart staying scrolled back in time when users pan and then change the time range quickly. Also avoids sending previous range OHLCV into the WebView while the new fetch is still in flight

Problem
Viewport / scroll — TradingView keeps pan/scroll state inside the WebView. Changing range without a full new surface could reuse that state, so the chart looked wrong after a fast time-range change.

Stale data — useOHLCVChart keeps the last ohlcvData until the new request completes. Right after a range change, props can show the new ohlcvSeriesKey but the old candle array (same reference). Sending that as SET_OHLCV_DATA for the new range causes bad data / races.

Solution
WebView key — Tie key to ohlcvSeriesKey (with ?? '' when the prop is omitted) so each series change remounts the WebView and drops inherited TradingView scroll state.

ohlcvSeriesStaleSnapshotRef + guard — When the series key changes but ohlcvData is still the previous array reference, don’t sync full OHLCV; wait until the hook returns a new array reference, then send.

useEffect on ohlcvSeriesKey — Reset loading / ready state (chartReadyCount, webViewLoaded, layout settle timers) and clear ohlcvSeriesStaleSnapshotRef. Reset activeIndicatorsRef, prevPositionLinesRef, and prevChartTypeRef so the new WebView gets indicators, position lines, and chart type (line vs candles) applied correctly without a default-candles flash.

Changelog

CHANGELOG entry: Fixes race condition on timeRange switch

Related issues

Fixes:

Manual testing steps

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

Before

Uploading Screen Recording 2026-04-16 at 23.39.54.mov…

After

Uploading Screen Recording 2026-04-17 at 00.10.19.mov…

Pre-merge author checklist

Performance checks (if applicable)

  • I've tested on Android
    • Ideally on a mid-range device; emulator is acceptable
  • I've tested with a power user scenario
    • Use these power-user SRPs to import wallets with many accounts and tokens
  • I've instrumented key operations with Sentry traces for production performance metrics

For performance guidelines and tooling, see the Performance Guide.

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.

Note

Medium Risk
Touches chart lifecycle/sync logic and WebView remount behavior, which could regress loading/skeleton timing or data updates across range switches if edge cases weren’t covered.

Overview
Fixes a race on time-range changes in AdvancedChart by remounting the WebView when ohlcvSeriesKey changes (via a key) and resetting “ready/loading/layout settle” state so the new instance re-applies indicators, position lines, and chart type deterministically.

Adds a stale-series guard (ohlcvSeriesStaleSnapshotRef) that suppresses SET_OHLCV_DATA syncing when the series key updates but ohlcvData is still the previous array reference, resuming only once fresh data arrives; updates tests to reflect the WebView remount/load sequence and stale-data wait behavior.

Reviewed by Cursor Bugbot for commit f7a52c7. Bugbot is set up for automated code reviews on this repo. Configure here.

@github-actions
Copy link
Copy Markdown
Contributor

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.

<View style={styles.container}>
<View style={styles.chartSurface}>
<WebView
key={`advanced-chart-${ohlcvSeriesKey ?? ''}`}
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

When ohlcvSeriesKey changes, the key string changes and react destroys the old WebView and creates a new one.

@github-actions
Copy link
Copy Markdown
Contributor

E2E Fixture Validation — Schema is up to date
11 value mismatches detected (expected — fixture represents an existing user).
View details

return;
}
}

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

check to make sure we do not send stale data when ohlcvSeriesKey changes, but should wait for fresh data. If it is stale, it returns and skips sendOHLCVData

@sahar-fehri sahar-fehri marked this pull request as ready for review April 16, 2026 21:35
const prevOhlcvDataRef = useRef<OHLCVBar[]>([]);
const prevOhlcvSeriesKeyRef = useRef<string | undefined>(undefined);
/** When non-null, `ohlcvData` is still the previous series' array; skip sync until the hook replaces it. */
const ohlcvSeriesStaleSnapshotRef = useRef<OHLCVBar[] | null>(null);
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Holds the previous series' ohlcvData array reference after ohlcvSeriesKey changes (see
guard below). useOHLCVChart does not clear data until the new fetch resolves.

Copy link
Copy Markdown
Contributor

@cursor cursor Bot left a comment

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit f7a52c7. Configure here.

Comment thread app/components/UI/Charts/AdvancedChart/AdvancedChart.tsx
@sahar-fehri sahar-fehri changed the title chore: fix time range change race condition chore: fix time range change race condition cp-7.74.0 Apr 16, 2026
@github-actions github-actions Bot added size-M risk-low Low testing needed · Low bug introduction risk and removed size-S labels Apr 16, 2026
@github-actions
Copy link
Copy Markdown
Contributor

🔍 Smart E2E Test Selection

  • Selected E2E tags: None (no tests recommended)
  • Selected Performance tags: None (no tests recommended)
  • Risk Level: low
  • AI Confidence: 90%
click to see 🤖 AI reasoning details

E2E Test Selection:
The changes are confined to the AdvancedChart component (a TradingView/OHLCV charting widget used in token overview) and its unit tests. The PR fixes a race condition where stale OHLCV data was being sent to the TradingView WebView during time range switches. Key observations:

  1. Scope: Only 2 files changed - the component itself and its unit tests. No controller, engine, navigation, or shared infrastructure changes.

  2. Nature of change: Internal bug fix adding a stale snapshot ref (ohlcvSeriesStaleSnapshotRef) and a key prop to force WebView remounting on series key changes. This prevents stale data from being sent to TradingView during time range switches.

  3. No E2E test coverage: Searching across all E2E test directories found zero references to AdvancedChart, OHLCV, TradingView, or related charting functionality. No existing E2E tests cover this feature.

  4. No shared component impact: The changes don't touch TabBar, navigation, modals, confirmations, or any other shared components that could break existing E2E tests.

  5. Feature flag gated: The AdvancedChart is behind a selectTokenOverviewAdvancedChartEnabled feature flag, further limiting its blast radius.

Since no E2E test tags cover this charting functionality and the changes are well-contained internal bug fixes, no E2E tags need to be run.

Performance Test Selection:
The changes fix a race condition in WebView data synchronization for the AdvancedChart component. While a key prop is added to force WebView remounting on series key changes, this is a targeted bug fix for correctness (preventing stale data), not a broad performance regression. The WebView remounting is intentional and controlled behavior. No performance test tags cover chart rendering or token overview specifically, and the change doesn't affect account lists, onboarding, login, swaps, app launch, or asset loading in a way that would meaningfully impact the measured performance metrics.

View GitHub Actions results

@sahar-fehri sahar-fehri enabled auto-merge April 16, 2026 21:53
@sonarqubecloud
Copy link
Copy Markdown

@sahar-fehri sahar-fehri added this pull request to the merge queue Apr 16, 2026
Merged via the queue into main with commit af96485 Apr 16, 2026
73 checks passed
@sahar-fehri sahar-fehri deleted the fix/reset-webview-on-timeRange-change branch April 16, 2026 22:50
@github-actions github-actions Bot locked and limited conversation to collaborators Apr 16, 2026
@metamaskbotv2 metamaskbotv2 Bot added the release-7.75.0 Issue or pull request that will be included in release 7.75.0 label Apr 16, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

release-7.75.0 Issue or pull request that will be included in release 7.75.0 risk-low Low testing needed · Low bug introduction risk size-M team-assets

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants