Skip to content

Commit 87e58f8

Browse files
sahar-fehrisahar-fehri
authored
fix: fix price line color with ambient TDP theme (#32164)
<!-- Please submit this PR as a draft initially. Do not mark it as "Ready for review" until this PR meets the canonical Definition of Ready For Review in `docs/readme/ready-for-review.md`. In short: the template must be materially complete (not just section titles present), all status checks must be currently passing, and the only expected follow-up commits must be reviewer-driven. --> <!-- mms-check directive vocabulary — read by .github/scripts/shared/pr-template-checks.ts at module load to build the validation plan. Directives are invisible in rendered markdown and must NOT be removed or edited without updating the validator registry. type=text Section must contain non-placeholder prose. type=changelog Section must have a valid CHANGELOG entry: line. type=issue-link Section must have a Fixes:/Closes:/Refs: line with a value. type=manual-testing Section must have real testing steps or an explicit N/A. type=screenshot Section must have evidence (image/URL) or an explicit N/A. type=checklist Section must have all checkboxes consciously checked. required=true|false Whether a missing/invalid section runs the validator at all. blocking=true|false Whether a failure of this check fails the CI workflow. Default: false — failures are shown as warnings in the sticky comment but do not block the PR. Sections without a directive are checked for structural presence only. --> ## **Description** <!-- mms-check: type=text required=true --> When the ambient price color experiment is enabled on Token Details (`useAmbientPriceColor` treatment), the advanced chart correctly hot-swaps the **series line** and **filled last-close price pill** to orange when price is negative — but the **dashed last-price horizontal line** stayed green. **Root cause:** `createLastPriceLine` and the `SET_THEME_COLORS` shape update for candle-mode dashed lines resolved color via `currentPriceColor || successColor` instead of following `lineColor` (which ambient pricing drives independently of `successColor`). Realtime bar updates then recreated the dashed line with the same wrong fallback on every tick. **Changes (WebView only — `chartLogic.js`, synced to `chartLogicString.ts`):** * **`createLastPriceLine`:** Dashed line color now uses the same resolution as line-chart mode. * **`handleSetThemeColors`:** Updates and recreates dashed last-price shapes using the corrected color; * **Refactor:** Introduced `getThemeLineColor()` and `getThemeLastPriceLineColor()` helpers so series line, filled pill, end marker, and dashed line share one color source and cannot drift again. **No behavior change when the ambient feature flag is off** — `lineColor` and `successColor` are identical in that path. ## **Changelog** <!-- mms-check: type=changelog required=true blocking=true --> <!-- 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: Fixed token details advanced chart dashed last-price line not matching ambient price color when price is negative. ## **Related issues** <!-- mms-check: type=issue-link required=true --> Fixes: https://consensyssoftware.atlassian.net/browse/ASSETS-3430 ## **Manual testing steps** <!-- mms-check: type=manual-testing required=true --> ```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** <!-- mms-check: type=screenshot required=true --> <!-- If applicable, add screenshots and/or recordings to visualize the before and after of your change. --> ### **Before** <!-- [screenshots/recordings] --> ### **After** <!-- [screenshots/recordings] --> https://github.com/user-attachments/assets/a06e93a9-01ec-4100-9e92-288e37d94b5d ## **Pre-merge author checklist** <!-- mms-check: type=checklist required=true --> <!-- Every checklist item must be consciously assessed before marking this PR as "Ready for review". A checked box means you deliberately considered that responsibility, not that you literally performed every action listed. Unchecked boxes are ambiguous: they are not an implicit "N/A" and they are not a silent "skip". See `docs/readme/ready-for-review.md` for the full checklist semantics. --> - [ ] 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. #### 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](https://consensyssoftware.atlassian.net/wiki/spaces/TL1/pages/edit-v2/401401446401?draftShareId=9d77e1e1-4bdc-4be1-9ebb-ccd916988d93) to import wallets with many accounts and tokens - [ ] I've instrumented key operations with Sentry traces for production performance metrics - See [`trace()`](/app/util/trace.ts) for usage and [`addToken`](/app/components/Views/AddAsset/components/AddCustomToken/AddCustomToken.tsx#L274) for an example For performance guidelines and tooling, see the [Performance Guide](https://consensyssoftware.atlassian.net/wiki/spaces/TL1/pages/400085549067/Performance+Guide+for+Engineers). ## **Pre-merge reviewer checklist** <!-- Reviewer checklist items follow the same semantics as the author checklist: an unchecked box is ambiguous, a checked box means the reviewer consciously assessed that responsibility. See `docs/readme/ready-for-review.md`. --> - [ ] 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** > WebView-only chart styling refactor with centralized fallbacks; no auth, data, or API changes. > > **Overview** > Fixes the token details advanced chart so the **dashed last-price horizontal line** matches ambient pricing when `useAmbientPriceColor` is on (e.g. orange for negative moves instead of staying green). > > The WebView chart logic now uses shared **`getThemeLineColor()`** and **`getThemeLastPriceLineColor()`** helpers (`lineColor` → `successColor`, and `currentPriceColor` → line color) everywhere series strokes, last-close pills, end dots, and dashed overlays are painted—including **`createLastPriceLine`**, **`createLineLastPriceLine`**, **`handleSetThemeColors`**, chart init, and realtime redraw paths. The same edits are mirrored in **`chartLogicString.ts`**. > > When ambient pricing is off, behavior is unchanged because `lineColor` and `successColor` stay aligned on the existing path. > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit 291d35d. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup> <!-- /CURSOR_SUMMARY -->
1 parent c2bb189 commit 87e58f8

2 files changed

Lines changed: 74 additions & 38 deletions

File tree

‎app/components/UI/Charts/AdvancedChart/webview/chartLogic.js‎

Lines changed: 37 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -818,6 +818,31 @@ function generatePaletteShades(hex) {
818818
return shades;
819819
}
820820

821+
// ============================================
822+
// Theme color helpers (series line, last-price overlays, end dot)
823+
// ============================================
824+
825+
/**
826+
* Ambient / series stroke color: line chart, filled last-close pill, line-end dot.
827+
* Falls back to successColor when lineColorOverride is unset (ambient feature off).
828+
* @param {object} [theme] — defaults to window.CONFIG.theme
829+
*/
830+
function getThemeLineColor(theme) {
831+
const t = theme || (window.CONFIG && window.CONFIG.theme) || {};
832+
return t.lineColor || t.successColor;
833+
}
834+
835+
/**
836+
* Custom dashed last-price horizontal_line. Honors currentPriceLineColorOverride when set,
837+
* else matches series line / filled pill via {@link getThemeLineColor}.
838+
* @param {object} [theme] — defaults to window.CONFIG.theme
839+
*/
840+
function getThemeLastPriceLineColor(theme) {
841+
const t = theme || (window.CONFIG && window.CONFIG.theme) || {};
842+
const lineColor = getThemeLineColor(t);
843+
return t.currentPriceColor || lineColor;
844+
}
845+
821846
function getSeriesColorOverrides(color) {
822847
return {
823848
'mainSeriesProperties.lineStyle.color': color,
@@ -848,8 +873,7 @@ function getSeriesColorOverrides(color) {
848873
*/
849874
function applySeriesColors() {
850875
if (!window.chartWidget) return;
851-
const color =
852-
window.CONFIG.theme.lineColor || window.CONFIG.theme.successColor;
876+
const color = getThemeLineColor();
853877
try {
854878
window.chartWidget.applyOverrides(getSeriesColorOverrides(color));
855879
let series = window.chartWidget.activeChart().getSeries();
@@ -895,8 +919,8 @@ function handleSetThemeColors(payload) {
895919
applySeriesColors();
896920

897921
let chart = window.chartWidget.activeChart();
898-
let lineColor = theme.lineColor || theme.successColor;
899-
let currentPriceColor = theme.currentPriceColor || lineColor;
922+
let lineColor = getThemeLineColor(theme);
923+
let lastPriceLineColor = getThemeLastPriceLineColor(theme);
900924

901925
// Update volume study colors if present
902926
if (window.volumeStudyId) {
@@ -927,15 +951,15 @@ function handleSetThemeColors(payload) {
927951
if (window.lastPriceShapeId) {
928952
try {
929953
chart.getShapeById(window.lastPriceShapeId).setProperties({
930-
linecolor: theme.currentPriceColor || theme.successColor,
954+
linecolor: lastPriceLineColor,
931955
});
932956
} catch (e) {}
933957
}
934958

935959
if (window.lineLastPriceShapeId) {
936960
try {
937961
chart.getShapeById(window.lineLastPriceShapeId).setProperties({
938-
linecolor: currentPriceColor,
962+
linecolor: lastPriceLineColor,
939963
});
940964
} catch (e) {}
941965
}
@@ -1412,7 +1436,7 @@ function updateVisibleEdgeOutlinePriceLabel() {
14121436

14131437
const theme = (w.CONFIG && w.CONFIG.theme) || {};
14141438
const upColor = theme.successColor || '#0C9F76';
1415-
const lineColor = theme.lineColor || upColor;
1439+
const lineColor = getThemeLineColor(theme) || upColor;
14161440
const downColor = theme.errorColor || '#E06470';
14171441
let outlineColor = ct === 2 ? lineColor : upColor;
14181442
if (ct === 1) {
@@ -2163,8 +2187,7 @@ function handleSetChartType(payload) {
21632187
let ac = window.chartWidget.activeChart();
21642188
ac.setChartType(type);
21652189

2166-
const color =
2167-
window.CONFIG.theme.lineColor || window.CONFIG.theme.successColor;
2190+
const color = getThemeLineColor();
21682191
let series = ac.getSeries();
21692192
if (type === 2) {
21702193
series.setChartStyleProperties(2, {
@@ -2318,7 +2341,7 @@ function handleSetPositionLines(payload) {
23182341
}
23192342

23202343
// ============================================
2321-
// Last close: green dashed horizontal_line (showPrice:false) + DOM pill (#last-close-price-label,
2344+
// Last close: dashed horizontal_line (showPrice:false) + DOM pill (#last-close-price-label,
23222345
// same styles as crosshair labels in AdvancedChartTemplate)
23232346
// ============================================
23242347
window.lastPriceShapeId = null;
@@ -2374,8 +2397,7 @@ function createLastPriceLine() {
23742397

23752398
let lastBar = window.ohlcvData[window.ohlcvData.length - 1];
23762399
let chart = window.chartWidget.activeChart();
2377-
let color =
2378-
window.CONFIG.theme.currentPriceColor || window.CONFIG.theme.successColor;
2400+
let color = getThemeLastPriceLineColor();
23792401
let candlePt = getLineEndDotTimeAndPriceFromSeries(chart);
23802402
let candlePrice =
23812403
candlePt && isFinite(candlePt.price) ? candlePt.price : lastBar.close;
@@ -2460,10 +2482,7 @@ function createLineLastPriceLine() {
24602482

24612483
let lastBar = window.ohlcvData[window.ohlcvData.length - 1];
24622484
let chart = window.chartWidget.activeChart();
2463-
const color =
2464-
window.CONFIG.theme.currentPriceColor ||
2465-
window.CONFIG.theme.lineColor ||
2466-
window.CONFIG.theme.successColor;
2485+
const color = getThemeLastPriceLineColor();
24672486
let seriesPt = resolveLineEndOverlayPoint(chart);
24682487
let linePrice =
24692488
seriesPt && isFinite(seriesPt.price) ? seriesPt.price : lastBar.close;
@@ -3245,8 +3264,7 @@ function refreshLineEndDot() {
32453264
return;
32463265
}
32473266

3248-
const color =
3249-
window.CONFIG.theme.lineColor || window.CONFIG.theme.successColor;
3267+
const color = getThemeLineColor();
32503268

32513269
function placeLineEndIcon() {
32523270
if (placementGen !== window.__lineEndDotPlacementGen) {
@@ -4515,7 +4533,7 @@ function initChart() {
45154533
'mainSeriesProperties.candleStyle.wickUpColor': theme.successColor,
45164534
'mainSeriesProperties.candleStyle.wickDownColor': theme.errorColor,
45174535
},
4518-
getSeriesColorOverrides(theme.lineColor || theme.successColor),
4536+
getSeriesColorOverrides(getThemeLineColor(theme)),
45194537
),
45204538

45214539
loading_screen: {

‎app/components/UI/Charts/AdvancedChart/webview/chartLogicString.ts‎

Lines changed: 37 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -827,6 +827,31 @@ function generatePaletteShades(hex) {
827827
return shades;
828828
}
829829
830+
// ============================================
831+
// Theme color helpers (series line, last-price overlays, end dot)
832+
// ============================================
833+
834+
/**
835+
* Ambient / series stroke color: line chart, filled last-close pill, line-end dot.
836+
* Falls back to successColor when lineColorOverride is unset (ambient feature off).
837+
* @param {object} [theme] — defaults to window.CONFIG.theme
838+
*/
839+
function getThemeLineColor(theme) {
840+
const t = theme || (window.CONFIG && window.CONFIG.theme) || {};
841+
return t.lineColor || t.successColor;
842+
}
843+
844+
/**
845+
* Custom dashed last-price horizontal_line. Honors currentPriceLineColorOverride when set,
846+
* else matches series line / filled pill via {@link getThemeLineColor}.
847+
* @param {object} [theme] — defaults to window.CONFIG.theme
848+
*/
849+
function getThemeLastPriceLineColor(theme) {
850+
const t = theme || (window.CONFIG && window.CONFIG.theme) || {};
851+
const lineColor = getThemeLineColor(t);
852+
return t.currentPriceColor || lineColor;
853+
}
854+
830855
function getSeriesColorOverrides(color) {
831856
return {
832857
'mainSeriesProperties.lineStyle.color': color,
@@ -857,8 +882,7 @@ function getSeriesColorOverrides(color) {
857882
*/
858883
function applySeriesColors() {
859884
if (!window.chartWidget) return;
860-
const color =
861-
window.CONFIG.theme.lineColor || window.CONFIG.theme.successColor;
885+
const color = getThemeLineColor();
862886
try {
863887
window.chartWidget.applyOverrides(getSeriesColorOverrides(color));
864888
let series = window.chartWidget.activeChart().getSeries();
@@ -904,8 +928,8 @@ function handleSetThemeColors(payload) {
904928
applySeriesColors();
905929
906930
let chart = window.chartWidget.activeChart();
907-
let lineColor = theme.lineColor || theme.successColor;
908-
let currentPriceColor = theme.currentPriceColor || lineColor;
931+
let lineColor = getThemeLineColor(theme);
932+
let lastPriceLineColor = getThemeLastPriceLineColor(theme);
909933
910934
// Update volume study colors if present
911935
if (window.volumeStudyId) {
@@ -936,15 +960,15 @@ function handleSetThemeColors(payload) {
936960
if (window.lastPriceShapeId) {
937961
try {
938962
chart.getShapeById(window.lastPriceShapeId).setProperties({
939-
linecolor: theme.currentPriceColor || theme.successColor,
963+
linecolor: lastPriceLineColor,
940964
});
941965
} catch (e) {}
942966
}
943967
944968
if (window.lineLastPriceShapeId) {
945969
try {
946970
chart.getShapeById(window.lineLastPriceShapeId).setProperties({
947-
linecolor: currentPriceColor,
971+
linecolor: lastPriceLineColor,
948972
});
949973
} catch (e) {}
950974
}
@@ -1421,7 +1445,7 @@ function updateVisibleEdgeOutlinePriceLabel() {
14211445
14221446
const theme = (w.CONFIG && w.CONFIG.theme) || {};
14231447
const upColor = theme.successColor || '#0C9F76';
1424-
const lineColor = theme.lineColor || upColor;
1448+
const lineColor = getThemeLineColor(theme) || upColor;
14251449
const downColor = theme.errorColor || '#E06470';
14261450
let outlineColor = ct === 2 ? lineColor : upColor;
14271451
if (ct === 1) {
@@ -2172,8 +2196,7 @@ function handleSetChartType(payload) {
21722196
let ac = window.chartWidget.activeChart();
21732197
ac.setChartType(type);
21742198
2175-
const color =
2176-
window.CONFIG.theme.lineColor || window.CONFIG.theme.successColor;
2199+
const color = getThemeLineColor();
21772200
let series = ac.getSeries();
21782201
if (type === 2) {
21792202
series.setChartStyleProperties(2, {
@@ -2327,7 +2350,7 @@ function handleSetPositionLines(payload) {
23272350
}
23282351
23292352
// ============================================
2330-
// Last close: green dashed horizontal_line (showPrice:false) + DOM pill (#last-close-price-label,
2353+
// Last close: dashed horizontal_line (showPrice:false) + DOM pill (#last-close-price-label,
23312354
// same styles as crosshair labels in AdvancedChartTemplate)
23322355
// ============================================
23332356
window.lastPriceShapeId = null;
@@ -2383,8 +2406,7 @@ function createLastPriceLine() {
23832406
23842407
let lastBar = window.ohlcvData[window.ohlcvData.length - 1];
23852408
let chart = window.chartWidget.activeChart();
2386-
let color =
2387-
window.CONFIG.theme.currentPriceColor || window.CONFIG.theme.successColor;
2409+
let color = getThemeLastPriceLineColor();
23882410
let candlePt = getLineEndDotTimeAndPriceFromSeries(chart);
23892411
let candlePrice =
23902412
candlePt && isFinite(candlePt.price) ? candlePt.price : lastBar.close;
@@ -2469,10 +2491,7 @@ function createLineLastPriceLine() {
24692491
24702492
let lastBar = window.ohlcvData[window.ohlcvData.length - 1];
24712493
let chart = window.chartWidget.activeChart();
2472-
const color =
2473-
window.CONFIG.theme.currentPriceColor ||
2474-
window.CONFIG.theme.lineColor ||
2475-
window.CONFIG.theme.successColor;
2494+
const color = getThemeLastPriceLineColor();
24762495
let seriesPt = resolveLineEndOverlayPoint(chart);
24772496
let linePrice =
24782497
seriesPt && isFinite(seriesPt.price) ? seriesPt.price : lastBar.close;
@@ -3254,8 +3273,7 @@ function refreshLineEndDot() {
32543273
return;
32553274
}
32563275
3257-
const color =
3258-
window.CONFIG.theme.lineColor || window.CONFIG.theme.successColor;
3276+
const color = getThemeLineColor();
32593277
32603278
function placeLineEndIcon() {
32613279
if (placementGen !== window.__lineEndDotPlacementGen) {
@@ -4524,7 +4542,7 @@ function initChart() {
45244542
'mainSeriesProperties.candleStyle.wickUpColor': theme.successColor,
45254543
'mainSeriesProperties.candleStyle.wickDownColor': theme.errorColor,
45264544
},
4527-
getSeriesColorOverrides(theme.lineColor || theme.successColor),
4545+
getSeriesColorOverrides(getThemeLineColor(theme)),
45284546
),
45294547
45304548
loading_screen: {

0 commit comments

Comments
 (0)