Skip to content

fix(charts): handle fast timeframe reloads in advanced chart cp-7.74.0#28985

Merged
sahar-fehri merged 2 commits into
mainfrom
fix/chart-timeframe-reloads
Apr 18, 2026
Merged

fix(charts): handle fast timeframe reloads in advanced chart cp-7.74.0#28985
sahar-fehri merged 2 commits into
mainfrom
fix/chart-timeframe-reloads

Conversation

@geositta

@geositta geositta commented Apr 17, 2026

Copy link
Copy Markdown
Contributor

Description

This PR fixes a post-merge regression in the advanced chart timeframe switch flow. After the recent remount based fix, there was still a race where a new OHLCV response could arrive before the remounted WebView finished loading. In that case, the stale snapshot guard could misclassify the already fresh candle array as stale and never send SET_OHLCV_DATA, leaving the new chart instance without data.

The fix narrows the stale snapshot logic so it captures only the previously synced OHLCV array at the moment the series key changes, instead of marking again whatever data happens to be in props later. This preserves the original protection against sending the previous range into the new WebView, while also allowing fast new range responses to sync correctly as soon as the remounted WebView finishes loading.

The PR also adds a focused regression test covering the fast-response / delayed-WebView-load case.

Changelog

CHANGELOG entry: Covered fast response / delayed WebView load case in stale snapshot guard

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

After

staleGuardTweak.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 data synchronization and remount timing logic, which can impact whether users see up-to-date candles during timeframe switches. Changes are localized and backed by a targeted regression test.

Overview
Fixes a timeframe-switch race in AdvancedChart where a fast new ohlcvData response could be incorrectly treated as “stale” during a WebView remount, preventing SET_OHLCV_DATA from ever being sent.

The stale-snapshot guard is tightened to snapshot the previously synced OHLCV array at the moment ohlcvSeriesKey changes (instead of whatever happens to be in props later), and the extra early-return path on series-key change is removed so fresh data can sync once the new WebView finishes loading. Adds a regression test covering “fresh data arrives before onLoadEnd after series change.”

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

@geositta geositta marked this pull request as ready for review April 17, 2026 15:37
@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.

@metamaskbotv2 metamaskbotv2 Bot added the team-perps Perps team label Apr 17, 2026
@github-actions

Copy link
Copy Markdown
Contributor

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

@github-actions github-actions Bot added the risk-low Low testing needed · Low bug introduction risk label Apr 17, 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 (app/components/UI/Charts/AdvancedChart/AdvancedChart.tsx) and its unit test file. This is a bug fix for how stale OHLCV (candlestick chart) data snapshots are handled when switching between chart series keys. Specifically:

  1. The fix changes ohlcvSeriesStaleSnapshotRef to preserve previous OHLCV data (instead of always nulling it) when a previous series key existed during reset.
  2. It removes an early-return branch that was preventing fresh data from being sent when the series key changed.
  3. A new unit test covers the fixed scenario.

No E2E tests exist for the AdvancedChart component specifically. The component is used in AssetOverview/Price for token price chart display, but there are no E2E smoke tests targeting this chart functionality. The changes are isolated UI component bug fixes with no impact on:

  • Core controllers or Engine
  • Navigation or shared infrastructure
  • Transaction/confirmation flows
  • Account management
  • Network management
  • Any other feature area covered by E2E smoke tags

The unit tests added alongside the fix provide adequate coverage for this behavioral change. No E2E tags are warranted.

Performance Test Selection:
The AdvancedChart changes are a bug fix for data snapshot handling in a WebView-based chart component. While it affects chart rendering, it's a correctness fix (ensuring fresh data is sent after series key changes) rather than a performance optimization or regression. The change does not affect rendering performance metrics, list rendering, data loading pipelines, or any of the performance-sensitive paths measured by the available performance test tags.

View GitHub Actions results

@sonarqubecloud

Copy link
Copy Markdown

@geositta geositta changed the title fix(charts): handle fast timeframe reloads in advanced chart fix(charts): handle fast timeframe reloads in advanced chart cp-7.74.0 Apr 17, 2026
@sahar-fehri sahar-fehri added this pull request to the merge queue Apr 18, 2026
Merged via the queue into main with commit 217e2c4 Apr 18, 2026
71 of 73 checks passed
@sahar-fehri sahar-fehri deleted the fix/chart-timeframe-reloads branch April 18, 2026 10:30
@github-actions github-actions Bot locked and limited conversation to collaborators Apr 18, 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 18, 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-S team-perps Perps team

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants