Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add input_amount to events #451

Merged
merged 13 commits into from
Feb 20, 2025
6 changes: 5 additions & 1 deletion src/components/Nabla/useSwapForm.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import { storageService } from '../../services/storage/local';
import schema, { SwapFormValues } from './schema';
import { getCaseSensitiveNetwork } from '../../helpers/networks';
import { useNetwork } from '../../contexts/network';
import { useFormStoreActions } from '../../stores/formStore';

type SwapSettings = {
from: string;
Expand All @@ -40,6 +41,7 @@ export const useSwapForm = () => {
const [isTokenSelectModalVisible, setIsTokenSelectModalVisible] = useState(false);
const [tokenSelectModalType, setTokenModalType] = useState<TokenSelectType>('from');
const { selectedNetwork, setSelectedNetwork } = useNetwork();
const { setFromAmount } = useFormStoreActions();

const initialState = useMemo(() => {
const searchParams = new URLSearchParams(window.location.search);
Expand Down Expand Up @@ -129,7 +131,9 @@ export const useSwapForm = () => {

const fromAmount: Big | undefined = useMemo(() => {
try {
return new Big(fromAmountString);
const fromAmount = new Big(fromAmountString);
setFromAmount(fromAmount);
return fromAmount;
} catch {
return undefined;
}
Expand Down
7 changes: 7 additions & 0 deletions src/contexts/events.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import { getNetworkId, isNetworkEVM, Networks } from '../helpers/networks';
import { LocalStorageKeys } from '../hooks/useLocalStorage';
import { storageService } from '../services/storage/local';
import { useNetwork } from './network';
import { useFromAmount } from '../stores/formStore';

declare global {
interface Window {
Expand All @@ -33,6 +34,7 @@ const UNIQUE_EVENT_TYPES: TrackableEvent['event'][] = [

export interface AmountTypeEvent {
event: `amount_type`;
input_amount: string;
}

export interface ClickDetailsEvent {
Expand All @@ -42,6 +44,7 @@ export interface ClickDetailsEvent {
export interface WalletConnectEvent {
event: 'wallet_connect';
wallet_action: 'connect' | 'disconnect' | 'change';
input_amount?: string;
account_address?: string;
}

Expand Down Expand Up @@ -104,6 +107,7 @@ export interface NetworkChangeEvent {

export interface FormErrorEvent {
event: 'form_error';
input_amount: string;
error_message:
| 'insufficient_balance'
| 'insufficient_liquidity'
Expand Down Expand Up @@ -148,6 +152,7 @@ const useEvents = () => {
const previousChainId = useRef<number | undefined>(undefined);
const firstRender = useRef(true);
const { selectedNetwork } = useNetwork();
const fromAmount = useFromAmount();

const scheduledQuotes = useRef<
| {
Expand Down Expand Up @@ -276,12 +281,14 @@ const useEvents = () => {
event: 'wallet_connect',
wallet_action: 'disconnect',
account_address: previous,
input_amount: fromAmount ? fromAmount.toString() : '0',
});
} else if (wasChanged) {
trackEvent({
event: 'wallet_connect',
wallet_action: wasConnected ? 'change' : 'connect',
account_address: address,
input_amount: fromAmount ? fromAmount.toString() : '0',
});
}

Expand Down
6 changes: 5 additions & 1 deletion src/hooks/nabla/useTokenAmountOut.ts
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,11 @@ export function useTokenOutAmount({
},
parseError: (error) => {
const insufficientLiquidityMessage = () => {
trackEvent({ event: 'form_error', error_message: 'insufficient_liquidity' });
trackEvent({
event: 'form_error',
error_message: 'insufficient_liquidity',
input_amount: amountIn ? amountIn : '0',
});
return 'Insufficient liquidity for this exchange. Please try a smaller amount or try again later.';
};

Expand Down
34 changes: 28 additions & 6 deletions src/pages/swap/index.tsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import Big from 'big.js';
import { useEffect, useMemo, useRef, useState, useCallback, FormEvent } from 'react';
import { useEffect, useMemo, useRef, useState, useCallback, FormEvent, useDeferredValue } from 'react';
import { ApiPromise } from '@polkadot/api';
import { motion } from 'framer-motion';

Expand Down Expand Up @@ -69,6 +69,7 @@ import satoshipayLogo from '../../assets/logo/satoshipay.svg';
export const SwapPage = () => {
const formRef = useRef<HTMLDivElement | null>(null);
const feeComparisonRef = useRef<FeeComparisonRef>(null);
const fromAmountRef = useRef<Big | undefined>(undefined);
const pendulumNode = usePendulumNode();
const [api, setApi] = useState<ApiPromise | null>(null);
const { isDisconnected, address } = useVortexAccount();
Expand Down Expand Up @@ -266,6 +267,10 @@ export const SwapPage = () => {
[toToken.fiat.assetIcon, toToken.fiat.symbol, form, tokenOutAmount.isLoading, openTokenSelectModal],
);

useEffect(() => {
fromAmountRef.current = fromAmount;
}, [fromAmount]);

const WithdrawNumericInput = useMemo(
() => (
<>
Expand All @@ -276,22 +281,31 @@ export const SwapPage = () => {
onClick={() => openTokenSelectModal('from')}
onChange={() => {
// User interacted with the input field
trackEvent({ event: 'amount_type' });
setTimeout(() => {
trackEvent({
event: 'amount_type',
input_amount: fromAmountRef.current ? fromAmountRef.current.toString() : '0',
Copy link
Member

Choose a reason for hiding this comment

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

Any reason we are using a ref, and especially using it only here? Alternatively, we could change the onChange prop of the AssetNumericInput component and pass teh new value in it. So you could access it right away. Here, you would then be able to do

onChange={(newValue: string) => {
   trackEvent({...})
}}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Well, the idea of the ref (and timeout) is because the onChange will always be triggered right on first modification. We need to wait, somehow, for the actual stable amount.

If we just send the event on change, the value registered will be always 0 or whatever default value is on the form as soon as the user started typing.

I think we will have the same issue on the AssetNumericInput. The other alternative I can think of, is to track it when the output is stable, here. Seems more cumbersome.

Copy link
Member

Choose a reason for hiding this comment

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

So the ref is just used in order not to track the first (prefilled) amount?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, it is to store the value. I tried using the "current" fromAmount value at the time of the first onChange, but the timeout function will take the value as soon as it is triggered (the initial one), which will be again the default.

Since the ref updates from the useEffect above, at the moment of triggering the event, it will read the last value typed.

Copy link
Member

Choose a reason for hiding this comment

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

I pushed a new commit with a suggested change. I thought it might be easier to show an alternative approach instead of describing it. What do you think about using the ref only for tracking if the user touched the field but then debounce the actual value to avoid tracking too many amounts?

I also learned that useDeferredValue is not necessarily suited to debounce values like for user amounts as the timeout is not configurable.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I saw you replaced the ref and timeout with a debounced value. Did it worked? Because I think this would send the current debouncedValueat the moment of the first onChange triggering.

Because here, since fromAmountFieldTouched is a dependency, it would execute the effect right away.

Yes useDeferred was a loose import.

Copy link
Member

Choose a reason for hiding this comment

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

I fixed that by adding another statement to the effect hook. I also removed the amount_type event from the list of unique events. Everything seems to work as expected now.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So we will capture all user quotes? Makes sense, only we will be flooded by events. But I suppose GA can handle such things.

Copy link
Member

Choose a reason for hiding this comment

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

I personally understood that the valuable outcome of this change is that marketing is able to see all amounts that the user tried. If it's just about the final amount, we can check the input amount on the other events after all. So yes, 'flooding' would be intended I think.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

All good then!

});
}, 3000);
}}
id="fromAmount"
/>
<UserBalance token={fromToken} onClick={(amount: string) => form.setValue('fromAmount', amount)} />
</>
),
[form, fromToken, openTokenSelectModal, trackEvent],
[form, fromAmount, fromToken, openTokenSelectModal, trackEvent],
);

function getCurrentErrorMessage() {
if (isDisconnected) return;

if (typeof userInputTokenBalance === 'string') {
if (Big(userInputTokenBalance).lt(fromAmount ?? 0) && walletAccount) {
trackEvent({ event: 'form_error', error_message: 'insufficient_balance' });
trackEvent({
event: 'form_error',
error_message: 'insufficient_balance',
input_amount: fromAmount ? fromAmount.toString() : '0',
});
return `Insufficient balance. Your balance is ${userInputTokenBalance} ${fromToken?.assetSymbol}.`;
}
}
Expand All @@ -303,14 +317,22 @@ export const SwapPage = () => {
const minAmountUnits = multiplyByPowerOfTen(Big(toToken.minWithdrawalAmountRaw), -toToken.decimals);

if (maxAmountUnits.lt(amountOut)) {
trackEvent({ event: 'form_error', error_message: 'more_than_maximum_withdrawal' });
trackEvent({
event: 'form_error',
error_message: 'more_than_maximum_withdrawal',
input_amount: fromAmount ? fromAmount.toString() : '0',
});
return `Maximum withdrawal amount is ${stringifyBigWithSignificantDecimals(maxAmountUnits, 2)} ${
toToken.fiat.symbol
}.`;
}

if (!config.test.overwriteMinimumTransferAmount && minAmountUnits.gt(amountOut)) {
trackEvent({ event: 'form_error', error_message: 'less_than_minimum_withdrawal' });
trackEvent({
event: 'form_error',
error_message: 'less_than_minimum_withdrawal',
input_amount: fromAmount ? fromAmount.toString() : '0',
});
return `Minimum withdrawal amount is ${stringifyBigWithSignificantDecimals(minAmountUnits, 2)} ${
toToken.fiat.symbol
}.`;
Expand Down
37 changes: 37 additions & 0 deletions src/stores/formStore.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
import { create } from 'zustand';
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@pendulum-chain/devs let me know if you like the addition of this state, if you prefer it to be more minimal (only fromAmount) or to avoid this altogether.

Although it adds extra code, I found it less confusing than passing fromAmount to the event context.

Copy link
Member

Choose a reason for hiding this comment

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

I'm fine with keeping it the way you implemented it 👍

import Big from 'big.js';
import { InputTokenDetails, OutputTokenDetails } from '../constants/tokenConfig';

interface FormState {
fromAmount?: Big;
fromToken?: InputTokenDetails;
toToken?: OutputTokenDetails;
}

interface FormStoreActions {
setFromAmount: (amount?: Big) => void;
setFromToken: (token?: InputTokenDetails) => void;
setToToken: (token?: OutputTokenDetails) => void;
}

type FormStore = FormState & {
actions: FormStoreActions;
};

const useFormStore = create<FormStore>((set) => ({
fromAmount: undefined,
fromToken: undefined,
toToken: undefined,

actions: {
setFromAmount: (amount?: Big) => set({ fromAmount: amount }),
setFromToken: (token?: InputTokenDetails) => set({ fromToken: token }),
setToToken: (token?: OutputTokenDetails) => set({ toToken: token }),
},
}));

export const useFromAmount = () => useFormStore((state) => state.fromAmount);
export const useFromToken = () => useFormStore((state) => state.fromToken);
export const useToToken = () => useFormStore((state) => state.toToken);

export const useFormStoreActions = () => useFormStore((state) => state.actions);
Loading