fix(ohlc): fix toggle race condition (#933)

dydxprotocol · Aug 19, 2024 · 4e97f92 · 4e97f92
1 parent b1e5697
commit 4e97f92
Show file tree

Hide file tree

Showing 6 changed files with 111 additions and 65 deletions.
diff --git a/src/constants/charts.ts b/src/constants/charts.ts
@@ -3,6 +3,8 @@ import { OrderSide } from '@dydxprotocol/v4-client-js';
 
 import { FundingDirection } from './markets';
 
+export const TOGGLE_ACTIVE_CLASS_NAME = 'toggle-active';
+
 // ------ Depth Chart ------ //
 export enum DepthChartSeries {
   Asks = 'Asks',

diff --git a/src/hooks/tradingView/useBuySellMarks.ts b/src/hooks/tradingView/useBuySellMarks.ts
@@ -1,5 +1,6 @@
-import { Dispatch, SetStateAction, useEffect } from 'react';
+import { useEffect } from 'react';
 
+import { TOGGLE_ACTIVE_CLASS_NAME } from '@/constants/charts';
 import { TvWidget } from '@/constants/tvchart';
 
 import { getMarketFills } from '@/state/accountSelectors';
@@ -14,13 +15,11 @@ import { useAppThemeAndColorModeContext } from '../useAppThemeAndColorMode';
 export function useBuySellMarks({
   buySellMarksToggle,
   buySellMarksToggleOn,
-  setBuySellMarksToggleOn,
   tvWidget,
   isChartReady,
 }: {
   buySellMarksToggle: HTMLElement | null;
   buySellMarksToggleOn: boolean;
-  setBuySellMarksToggleOn: Dispatch<SetStateAction<boolean>>;
   tvWidget: TvWidget | null;
   isChartReady: boolean;
 }) {
@@ -30,13 +29,6 @@ export function useBuySellMarks({
 
   const theme = useAppThemeAndColorModeContext();
 
-  useEffect(() => {
-    // Initialize onClick for Buys/Sells toggle
-    if (isChartReady && buySellMarksToggle) {
-      buySellMarksToggle.onclick = () => setBuySellMarksToggleOn((prev) => !prev);
-    }
-  }, [isChartReady, buySellMarksToggle, setBuySellMarksToggleOn]);
-
   useEffect(
     // Update marks on toggle and on new fills and on display preference changes
     () => {
@@ -45,10 +37,10 @@ export function useBuySellMarks({
       tvWidget.onChartReady(() => {
         tvWidget.headerReady().then(() => {
           if (buySellMarksToggleOn) {
-            buySellMarksToggle?.classList?.add('toggle-active');
+            buySellMarksToggle?.classList?.add(TOGGLE_ACTIVE_CLASS_NAME);
             tvWidget.activeChart().refreshMarks();
           } else {
-            buySellMarksToggle?.classList?.remove('toggle-active');
+            buySellMarksToggle?.classList?.remove(TOGGLE_ACTIVE_CLASS_NAME);
             tvWidget.activeChart().clearMarks();
           }
         });

diff --git a/src/hooks/tradingView/useChartLines.ts b/src/hooks/tradingView/useChartLines.ts
@@ -1,8 +1,9 @@
-import { Dispatch, SetStateAction, useCallback, useEffect, useRef, useState } from 'react';
+import { useCallback, useEffect, useRef, useState } from 'react';
 
 import { shallowEqual } from 'react-redux';
 
 import { ORDER_SIDES, SubaccountOrder } from '@/constants/abacus';
+import { TOGGLE_ACTIVE_CLASS_NAME } from '@/constants/charts';
 import { STRING_KEYS } from '@/constants/localization';
 import { ORDER_TYPE_STRINGS, type OrderType } from '@/constants/trade';
 import type { ChartLine, PositionLineType, TvWidget } from '@/constants/tvchart';
@@ -33,13 +34,11 @@ export const useChartLines = ({
   orderLineToggle,
   isChartReady,
   orderLinesToggleOn,
-  setOrderLinesToggleOn,
 }: {
   tvWidget: TvWidget | null;
   orderLineToggle: HTMLElement | null;
   isChartReady: boolean;
   orderLinesToggleOn: boolean;
-  setOrderLinesToggleOn: Dispatch<SetStateAction<boolean>>;
 }) => {
   const [initialWidget, setInitialWidget] = useState<TvWidget | null>(null);
   const [lastMarket, setLastMarket] = useState<string | undefined>(undefined);
@@ -262,22 +261,15 @@ export const useChartLines = ({
 
   // Effects
 
-  useEffect(() => {
-    // Initialize onClick for order line toggle
-    if (isChartReady && orderLineToggle) {
-      orderLineToggle.onclick = () => setOrderLinesToggleOn((prev) => !prev);
-    }
-  }, [isChartReady, orderLineToggle, setOrderLinesToggleOn]);
-
   useEffect(
     // Update display button on toggle
     () => {
       if (isChartReady) {
         runOnChartReady(() => {
           if (orderLinesToggleOn) {
-            orderLineToggle?.classList?.add('toggle-active');
+            orderLineToggle?.classList?.add(TOGGLE_ACTIVE_CLASS_NAME);
           } else {
-            orderLineToggle?.classList?.remove('toggle-active');
+            orderLineToggle?.classList?.remove(TOGGLE_ACTIVE_CLASS_NAME);
           }
         });
       }

diff --git a/src/hooks/tradingView/useOrderbookCandles.ts b/src/hooks/tradingView/useOrderbookCandles.ts
@@ -1,5 +1,6 @@
-import { Dispatch, SetStateAction, useEffect } from 'react';
+import { useEffect } from 'react';
 
+import { TOGGLE_ACTIVE_CLASS_NAME } from '@/constants/charts';
 import { TvWidget } from '@/constants/tvchart';
 
 import abacusStateManager from '@/lib/abacus';
@@ -11,22 +12,13 @@ export const useOrderbookCandles = ({
   orderbookCandlesToggle,
   isChartReady,
   orderbookCandlesToggleOn,
-  setOrderbookCandlesToggleOn,
   tvWidget,
 }: {
   orderbookCandlesToggle: HTMLElement | null;
   isChartReady: boolean;
   orderbookCandlesToggleOn: boolean;
-  setOrderbookCandlesToggleOn: Dispatch<SetStateAction<boolean>>;
   tvWidget: TvWidget | null;
 }) => {
-  useEffect(() => {
-    // Initialize onClick for orderbook candles toggle
-    if (isChartReady && orderbookCandlesToggle) {
-      orderbookCandlesToggle.onclick = () => setOrderbookCandlesToggleOn((prev) => !prev);
-    }
-  }, [isChartReady, orderbookCandlesToggle, setOrderbookCandlesToggleOn]);
-
   useEffect(
     // Update orderbookCandles button on toggle
     () => {
@@ -35,9 +27,9 @@ export const useOrderbookCandles = ({
       tvWidget.onChartReady(() => {
         tvWidget.headerReady().then(() => {
           if (orderbookCandlesToggleOn) {
-            orderbookCandlesToggle?.classList?.add('toggle-active');
+            orderbookCandlesToggle?.classList?.add(TOGGLE_ACTIVE_CLASS_NAME);
           } else {
-            orderbookCandlesToggle?.classList?.remove('toggle-active');
+            orderbookCandlesToggle?.classList?.remove(TOGGLE_ACTIVE_CLASS_NAME);
           }
           abacusStateManager.toggleOrderbookCandles(orderbookCandlesToggleOn);
         });

diff --git a/src/hooks/tradingView/useTradingView.ts b/src/hooks/tradingView/useTradingView.ts
@@ -1,4 +1,4 @@
-import React, { useEffect, useState } from 'react';
+import React, { Dispatch, SetStateAction, useCallback, useEffect, useState } from 'react';
 
 import BigNumber from 'bignumber.js';
 import isEmpty from 'lodash/isEmpty';
@@ -10,6 +10,7 @@ import {
 import { shallowEqual } from 'react-redux';
 
 import { DEFAULT_RESOLUTION } from '@/constants/candles';
+import { TOGGLE_ACTIVE_CLASS_NAME } from '@/constants/charts';
 import { LocalStorageKey } from '@/constants/localStorage';
 import { STRING_KEYS, SUPPORTED_LOCALE_BASE_TAGS } from '@/constants/localization';
 import { tooltipStrings } from '@/constants/tooltips';
@@ -39,21 +40,32 @@ import { useURLConfigs } from '../useURLConfigs';
 export const useTradingView = ({
   tvWidgetRef,
   orderLineToggleRef,
+  orderLinesToggleOn,
+  setOrderLinesToggleOn,
   orderbookCandlesToggleRef,
   orderbookCandlesToggleOn,
+  setOrderbookCandlesToggleOn,
   buySellMarksToggleRef,
+  buySellMarksToggleOn,
+  setBuySellMarksToggleOn,
   setIsChartReady,
 }: {
   tvWidgetRef: React.MutableRefObject<TvWidget | null>;
   orderLineToggleRef: React.MutableRefObject<HTMLElement | null>;
+  orderLinesToggleOn: boolean;
+  setOrderLinesToggleOn: Dispatch<SetStateAction<boolean>>;
   orderbookCandlesToggleRef: React.MutableRefObject<HTMLElement | null>;
   orderbookCandlesToggleOn: boolean;
+  setOrderbookCandlesToggleOn: Dispatch<SetStateAction<boolean>>;
   buySellMarksToggleRef: React.MutableRefObject<HTMLElement | null>;
+  buySellMarksToggleOn: boolean;
+  setBuySellMarksToggleOn: Dispatch<SetStateAction<boolean>>;
   setIsChartReady: React.Dispatch<React.SetStateAction<boolean>>;
 }) => {
   const stringGetter = useStringGetter();
   const urlConfigs = useURLConfigs();
   const featureFlags = useAllStatsigGateValues();
+
   const { isOhlcEnabled } = useEnvFeatures();
   const { group, decimal } = useLocaleSeparators();
 
@@ -79,6 +91,35 @@ export const useTradingView = ({
   const hasMarkets = marketIds.length > 0;
   const hasPriceScaleInfo = initialPriceScale !== null || hasMarkets;
 
+  const initializeToggle = useCallback(
+    ({
+      toggleRef,
+      tvWidget,
+      isOn,
+      setToggleOn,
+      label,
+      tooltip,
+    }: {
+      toggleRef: React.MutableRefObject<HTMLElement | null>;
+      tvWidget: TvWidget;
+      isOn: boolean;
+      setToggleOn: Dispatch<SetStateAction<boolean>>;
+      label: string;
+      tooltip: string;
+    }) => {
+      if (toggleRef) {
+        toggleRef.current = tvWidget.createButton();
+        toggleRef.current.innerHTML = `<span>${label}</span> <div class="toggle"></div>`;
+        toggleRef.current.setAttribute('title', tooltip);
+        if (isOn) {
+          toggleRef.current.classList.add(TOGGLE_ACTIVE_CLASS_NAME);
+        }
+        toggleRef.current.onclick = () => setToggleOn((prev) => !prev);
+      }
+    },
+    []
+  );
+
   useEffect(() => {
     // we only need tick size from current market for the price scale settings
     // if markets haven't been loaded via abacus, get the current market info from indexer
@@ -122,17 +163,19 @@ export const useTradingView = ({
       tvWidgetRef.current?.onChartReady(() => {
         tvWidgetRef.current?.headerReady().then(() => {
           if (tvWidgetRef.current) {
-            if (orderLineToggleRef) {
-              orderLineToggleRef.current = tvWidgetRef.current.createButton();
-              orderLineToggleRef.current.innerHTML = `<span>${stringGetter({
+            initializeToggle({
+              toggleRef: orderLineToggleRef,
+              tvWidget: tvWidgetRef.current,
+              isOn: orderLinesToggleOn,
+              setToggleOn: setOrderLinesToggleOn,
+              label: stringGetter({
                 key: STRING_KEYS.ORDER_LINES,
-              })}</span> <div class="toggle"></div>`;
-              orderLineToggleRef.current.setAttribute(
-                'title',
-                stringGetter({ key: STRING_KEYS.ORDER_LINES_TOOLTIP })
-              );
-            }
-            if (isOhlcEnabled && orderbookCandlesToggleRef) {
+              }),
+              tooltip: stringGetter({
+                key: STRING_KEYS.ORDER_LINES_TOOLTIP,
+              }),
+            });
+            if (isOhlcEnabled) {
               const getOhlcTooltipString = tooltipStrings.ohlc;
               const { title: ohlcTitle, body: ohlcBody } = getOhlcTooltipString({
                 stringGetter,
@@ -141,18 +184,27 @@ export const useTradingView = ({
                 featureFlags,
               });
 
-              orderbookCandlesToggleRef.current = tvWidgetRef.current.createButton();
-              orderbookCandlesToggleRef.current.innerHTML = `<span>${`${ohlcTitle}*`}</span> <div class="toggle"></div>`;
-              orderbookCandlesToggleRef.current.setAttribute('title', ohlcBody as string);
-            }
-            if (buySellMarksToggleRef) {
-              buySellMarksToggleRef.current = tvWidgetRef.current.createButton();
-              buySellMarksToggleRef.current.innerHTML = `<span>${stringGetter({ key: STRING_KEYS.BUYS_SELLS_TOGGLE })}</span> <div class="toggle"></div>`;
-              buySellMarksToggleRef.current.setAttribute(
-                'title',
-                stringGetter({ key: STRING_KEYS.BUYS_SELLS_TOGGLE_TOOLTIP })
-              );
+              initializeToggle({
+                toggleRef: orderbookCandlesToggleRef,
+                tvWidget: tvWidgetRef.current,
+                isOn: orderbookCandlesToggleOn,
+                setToggleOn: setOrderbookCandlesToggleOn,
+                label: `${ohlcTitle}*`,
+                tooltip: ohlcBody as string,
+              });
             }
+            initializeToggle({
+              toggleRef: buySellMarksToggleRef,
+              tvWidget: tvWidgetRef.current,
+              isOn: buySellMarksToggleOn,
+              setToggleOn: setBuySellMarksToggleOn,
+              label: stringGetter({
+                key: STRING_KEYS.BUYS_SELLS_TOGGLE,
+              }),
+              tooltip: stringGetter({
+                key: STRING_KEYS.BUYS_SELLS_TOGGLE_TOOLTIP,
+              }),
+            });
           }
         });
 
@@ -175,7 +227,20 @@ export const useTradingView = ({
       tvWidgetRef.current = null;
       setIsChartReady(false);
     };
-  }, [selectedLocale, selectedNetwork, !!marketId, hasPriceScaleInfo, orderbookCandlesToggleOn]);
+  }, [
+    selectedLocale,
+    selectedNetwork,
+    !!marketId,
+    hasPriceScaleInfo,
+    orderLineToggleRef,
+    orderbookCandlesToggleRef,
+    buySellMarksToggleRef,
+    setBuySellMarksToggleOn,
+    setOrderLinesToggleOn,
+    setOrderbookCandlesToggleOn,
+    orderbookCandlesToggleOn,
+    tvWidgetRef,
+  ]);
 
   return { savedResolution };
 };
diff --git a/src/views/charts/TvChart.tsx b/src/views/charts/TvChart.tsx
@@ -26,25 +26,31 @@ export const TvChart = () => {
 
   const orderLineToggleRef = useRef<HTMLElement | null>(null);
   const orderLineToggle = orderLineToggleRef.current;
+
   const orderbookCandlesToggleRef = useRef<HTMLElement | null>(null);
   const orderbookCandlesToggle = orderbookCandlesToggleRef.current;
   const buySellMarksToggleRef = useRef<HTMLElement | null>(null);
   const buySellMarksToggle = buySellMarksToggleRef.current;
 
   const {
-    orderbookCandlesToggleOn,
-    setOrderbookCandlesToggleOn,
     orderLinesToggleOn,
     setOrderLinesToggleOn,
+    orderbookCandlesToggleOn,
+    setOrderbookCandlesToggleOn,
     setBuySellMarksToggleOn,
     buySellMarksToggleOn,
   } = useTradingViewToggles();
   const { savedResolution } = useTradingView({
     tvWidgetRef,
     orderLineToggleRef,
+    orderLinesToggleOn,
+    setOrderLinesToggleOn,
     orderbookCandlesToggleRef,
     orderbookCandlesToggleOn,
+    setOrderbookCandlesToggleOn,
     buySellMarksToggleRef,
+    buySellMarksToggleOn,
+    setBuySellMarksToggleOn,
     setIsChartReady,
   });
   useChartMarketAndResolution({
@@ -57,23 +63,20 @@ export const TvChart = () => {
     orderLineToggle,
     isChartReady,
     orderLinesToggleOn,
-    setOrderLinesToggleOn,
   });
   useOrderbookCandles({
     orderbookCandlesToggle,
     isChartReady,
     orderbookCandlesToggleOn,
-    setOrderbookCandlesToggleOn,
     tvWidget,
   });
-  useTradingViewTheme({ tvWidget, isWidgetReady, chartLines });
   useBuySellMarks({
     buySellMarksToggle,
     buySellMarksToggleOn,
-    setBuySellMarksToggleOn,
     tvWidget,
     isChartReady,
   });
+  useTradingViewTheme({ tvWidget, isWidgetReady, chartLines });
 
   return (
     <$PriceChart isChartReady={isChartReady}>