From f0b484e93a0ba86a2035512a7dc76693c6d4662d Mon Sep 17 00:00:00 2001 From: Bill Date: Tue, 30 Jul 2024 10:29:19 -0700 Subject: [PATCH] feat: deposit/withdraw UX improvements (#857) --- package.json | 2 +- pnpm-lock.yaml | 8 +- src/components/Button.tsx | 13 +- src/components/Loading/LoadingDots.tsx | 8 +- src/constants/notifications.ts | 2 + src/hooks/useLocalNotifications.tsx | 195 ++++++++++-------- src/hooks/useNotificationTypes.tsx | 4 +- .../AccountManagementForms/DepositForm.tsx | 29 ++- .../DepositForm/DepositButtonAndReceipt.tsx | 16 +- .../AccountManagementForms/WithdrawForm.tsx | 56 +++-- .../TransferStatusNotification/index.tsx | 28 ++- 11 files changed, 228 insertions(+), 133 deletions(-) diff --git a/package.json b/package.json index 4b6a236de..635cd2c0d 100644 --- a/package.json +++ b/package.json @@ -53,7 +53,7 @@ "@cosmjs/tendermint-rpc": "^0.32.1", "@dydxprotocol/v4-abacus": "1.8.63", "@dydxprotocol/v4-client-js": "^1.1.27", - "@dydxprotocol/v4-localization": "^1.1.164", + "@dydxprotocol/v4-localization": "^1.1.165", "@ethersproject/providers": "^5.7.2", "@hugocxl/react-to-image": "^0.0.9", "@js-joda/core": "^5.5.3", diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index cd9bc99c8..79997067d 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -36,8 +36,8 @@ dependencies: specifier: ^1.1.27 version: 1.1.27 '@dydxprotocol/v4-localization': - specifier: ^1.1.164 - version: 1.1.164 + specifier: ^1.1.165 + version: 1.1.165 '@ethersproject/providers': specifier: ^5.7.2 version: 5.7.2 @@ -3239,8 +3239,8 @@ packages: - utf-8-validate dev: false - /@dydxprotocol/v4-localization@1.1.164: - resolution: {integrity: sha512-qModKXbxuxto6iDhNtzpnLPw4qopbkO5aDAjoUu1CF763ipJFLjwsrij1aaI8ivAhwzJAnKE8n51vrWryFmwNQ==} + /@dydxprotocol/v4-localization@1.1.165: + resolution: {integrity: sha512-db8gVr8P5Lsf/H6Efpue3EOj1NUxGW7XdatPqSjO43ekE12jO214WV8e7ffQX31oNRYm1SmXKUaiaiqsnnJf0A==} dev: false /@dydxprotocol/v4-proto@5.0.0-dev.0: diff --git a/src/components/Button.tsx b/src/components/Button.tsx index b14458ac8..d71721b0a 100644 --- a/src/components/Button.tsx +++ b/src/components/Button.tsx @@ -27,6 +27,7 @@ type ElementProps = { slotLeft?: React.ReactNode; slotRight?: React.ReactNode; state?: ButtonState | ButtonStateConfig; + withContentOnLoading?: boolean; }; type StyleProps = { @@ -48,6 +49,7 @@ export const Button = forwardRef {state[ButtonState.Loading] ? ( - + <> + {withContentOnLoading && slotLeft} + {withContentOnLoading && children} + {withContentOnLoading && slotRight} + <$LoadingDots size={3} /> + ) : ( <> {slotLeft} @@ -170,3 +177,7 @@ const StyledBaseButton = styled(BaseButton)` ${state[ButtonState.Disabled] && buttonStateVariants(action)[ButtonState.Disabled]} `} `; + +const $LoadingDots = styled(LoadingDots)` + padding-top: 0.5em; +`; diff --git a/src/components/Loading/LoadingDots.tsx b/src/components/Loading/LoadingDots.tsx index dac3d7681..a4d9da7dc 100644 --- a/src/components/Loading/LoadingDots.tsx +++ b/src/components/Loading/LoadingDots.tsx @@ -3,11 +3,15 @@ import styled, { css } from 'styled-components'; // Types/constants export type LoadingDotsProps = { size?: number; + className?: string; }; // Component -export const LoadingDots: React.FC = ({ size = 1 }: LoadingDotsProps) => ( - +export const LoadingDots: React.FC = ({ + size = 1, + className, +}: LoadingDotsProps) => ( + diff --git a/src/constants/notifications.ts b/src/constants/notifications.ts index e604f8259..a3a3432e3 100644 --- a/src/constants/notifications.ts +++ b/src/constants/notifications.ts @@ -191,6 +191,7 @@ export enum TransferNotificationTypes { } export type TransferNotifcation = { + id?: string; txHash: string; type?: TransferNotificationTypes; toChainId?: string; @@ -203,6 +204,7 @@ export type TransferNotifcation = { isExchange?: boolean; requestId?: string; tracked?: boolean; + isDummy?: boolean; }; export enum ReleaseUpdateNotificationIds { diff --git a/src/hooks/useLocalNotifications.tsx b/src/hooks/useLocalNotifications.tsx index b9eb9a3c4..399bca7ac 100644 --- a/src/hooks/useLocalNotifications.tsx +++ b/src/hooks/useLocalNotifications.tsx @@ -83,10 +83,33 @@ const useLocalNotificationsContext = () => { [setAllTransferNotifications, dydxAddress, allTransferNotifications] ); - const addTransferNotification = useCallback( + useEffect(() => { + // whip out all dummy notifications on startup + if (!dydxAddress) return; + setAllTransferNotifications((currentAllNotifications) => { + const updatedNotifications = { ...currentAllNotifications }; + updatedNotifications[dydxAddress] = (updatedNotifications[dydxAddress] ?? []).filter( + (n) => !n.isDummy + ); + return updatedNotifications; + }); + }, [dydxAddress]); + + const addOrUpdateTransferNotification = useCallback( (notification: TransferNotifcation) => { - const { txHash, triggeredAt, toAmount, type, fromChainId } = notification; - setTransferNotifications([...transferNotifications, notification]); + const { id, txHash, triggeredAt, toAmount, type, fromChainId } = notification; + // replace notification if id or txhash already exists + const existingNotificationIndex = transferNotifications.findIndex( + (n) => n.id === id || n.txHash === txHash + ); + if (existingNotificationIndex > -1) { + const updatedNotifications = [...transferNotifications]; + updatedNotifications[existingNotificationIndex] = notification; + setTransferNotifications(updatedNotifications); + } else { + setTransferNotifications([...transferNotifications, notification]); + } + trackSkipTxWithTenacity({ transactionHash: txHash, chainId: fromChainId, @@ -104,7 +127,7 @@ const useLocalNotificationsContext = () => { }) ); }, - [transferNotifications] + [transferNotifications, setTransferNotifications, skip] ); useQuery({ @@ -114,91 +137,93 @@ const useLocalNotificationsContext = () => { transferNotificationsInner: TransferNotifcation[] ) => { const newTransferNotifications = await Promise.all( - transferNotificationsInner.map(async (transferNotification) => { - const { - txHash, - toChainId, - fromChainId, - triggeredAt, - isCctp, - errorCount, - status: currentStatus, - isExchange, - requestId, - tracked, - } = transferNotification; - - const hasErrors = - // @ts-ignore status.errors is not in the type definition but can be returned - // also error can some time come back as an empty object so we need to ignore for that - !!currentStatus?.errors || - (currentStatus?.error && Object.keys(currentStatus.error).length !== 0); - - if ( - !isExchange && - !hasErrors && - (!currentStatus?.squidTransactionStatus || - currentStatus?.squidTransactionStatus === 'ongoing') - ) { - try { - const skipParams = { - transactionHash: txHash, - chainId: fromChainId, - baseUrl: skip, - }; - if (!tracked && useSkip) { - const { tx_hash: trackedTxHash } = await trackSkipTx(skipParams); - // if no tx hash was returned, transfer has not yet been tracked - if (!trackedTxHash) return transferNotification; - transferNotification.tracked = true; - } - const status = await fetchTransferStatus({ - transactionId: txHash, - toChainId, - fromChainId, - isCctp, - requestId, - baseUrl: skip, - useSkip, - }); - if (status) { - transferNotification.status = status; - if (status.squidTransactionStatus === 'success') { - track( - AnalyticsEvents.TransferNotification({ - triggeredAt, - timeSpent: triggeredAt ? Date.now() - triggeredAt : undefined, - toAmount: transferNotification.toAmount, - status: 'success', - type: transferNotification.type, - txHash, - }) - ); + transferNotificationsInner + .filter((n) => !n.isDummy) + .map(async (transferNotification) => { + const { + txHash, + toChainId, + fromChainId, + triggeredAt, + isCctp, + errorCount, + status: currentStatus, + isExchange, + requestId, + tracked, + } = transferNotification; + + const hasErrors = + // @ts-ignore status.errors is not in the type definition but can be returned + // also error can some time come back as an empty object so we need to ignore for that + !!currentStatus?.errors || + (currentStatus?.error && Object.keys(currentStatus.error).length !== 0); + + if ( + !isExchange && + !hasErrors && + (!currentStatus?.squidTransactionStatus || + currentStatus?.squidTransactionStatus === 'ongoing') + ) { + try { + const skipParams = { + transactionHash: txHash, + chainId: fromChainId, + baseUrl: skip, + }; + if (!tracked && useSkip) { + const { tx_hash: trackedTxHash } = await trackSkipTx(skipParams); + // if no tx hash was returned, transfer has not yet been tracked + if (!trackedTxHash) return transferNotification; + transferNotification.tracked = true; } - } - } catch (error) { - if (!triggeredAt || Date.now() - triggeredAt > STATUS_ERROR_GRACE_PERIOD) { - if (errorCount && errorCount > ERROR_COUNT_THRESHOLD) { - transferNotification.status = error; - track( - AnalyticsEvents.TransferNotification({ - triggeredAt, - timeSpent: triggeredAt ? Date.now() - triggeredAt : undefined, - toAmount: transferNotification.toAmount, - status: 'error', - type: transferNotification.type, - txHash, - }) - ); - } else { - transferNotification.errorCount = errorCount ? errorCount + 1 : 1; + const status = await fetchTransferStatus({ + transactionId: txHash, + toChainId, + fromChainId, + isCctp, + requestId, + baseUrl: skip, + useSkip, + }); + if (status) { + transferNotification.status = status; + if (status.squidTransactionStatus === 'success') { + track( + AnalyticsEvents.TransferNotification({ + triggeredAt, + timeSpent: triggeredAt ? Date.now() - triggeredAt : undefined, + toAmount: transferNotification.toAmount, + status: 'success', + type: transferNotification.type, + txHash, + }) + ); + } + } + } catch (error) { + if (!triggeredAt || Date.now() - triggeredAt > STATUS_ERROR_GRACE_PERIOD) { + if (errorCount && errorCount > ERROR_COUNT_THRESHOLD) { + transferNotification.status = error; + track( + AnalyticsEvents.TransferNotification({ + triggeredAt, + timeSpent: triggeredAt ? Date.now() - triggeredAt : undefined, + toAmount: transferNotification.toAmount, + status: 'error', + type: transferNotification.type, + txHash, + }) + ); + } else { + transferNotification.errorCount = errorCount ? errorCount + 1 : 1; + } } } } - } - return transferNotification; - }) + return transferNotification; + }) ); return newTransferNotifications; @@ -212,6 +237,6 @@ const useLocalNotificationsContext = () => { return { // Transfer notifications transferNotifications, - addTransferNotification, + addOrUpdateTransferNotification, }; }; diff --git a/src/hooks/useNotificationTypes.tsx b/src/hooks/useNotificationTypes.tsx index 99743f2e0..34b30e14f 100644 --- a/src/hooks/useNotificationTypes.tsx +++ b/src/hooks/useNotificationTypes.tsx @@ -209,7 +209,7 @@ export const notificationTypes: NotificationTypeConfig[] = [ useEffect(() => { // eslint-disable-next-line no-restricted-syntax for (const transfer of transferNotifications) { - const { fromChainId, status, txHash, toAmount, type, isExchange } = transfer; + const { id, fromChainId, status, txHash, toAmount, type, isExchange } = transfer; const isFinished = (Boolean(status) && status?.squidTransactionStatus !== 'ongoing') || isExchange; const icon = ; @@ -241,7 +241,7 @@ export const notificationTypes: NotificationTypeConfig[] = [ }); trigger( - txHash, + id ?? txHash, { icon, title, diff --git a/src/views/forms/AccountManagementForms/DepositForm.tsx b/src/views/forms/AccountManagementForms/DepositForm.tsx index 327119e6c..771e2eb70 100644 --- a/src/views/forms/AccountManagementForms/DepositForm.tsx +++ b/src/views/forms/AccountManagementForms/DepositForm.tsx @@ -70,11 +70,18 @@ type DepositFormProps = { onError?: () => void; }; +enum DepositSteps { + Initial = 'initial', + Approval = 'approval', + Confirm = 'confirm', +} + export const DepositForm = ({ onDeposit, onError }: DepositFormProps) => { const stringGetter = useStringGetter(); const dispatch = useAppDispatch(); const [error, setError] = useState(null); const [isLoading, setIsLoading] = useState(false); + const [depositStep, setDepositStep] = useState(DepositSteps.Initial); const [requireUserActionInWallet, setRequireUserActionInWallet] = useState(false); const selectedDydxChainId = useAppSelector(getSelectedDydxChainId); const { hasAcknowledgedTerms } = useAppSelector(getOnboardingGuards); @@ -88,7 +95,7 @@ export const DepositForm = ({ onDeposit, onError }: DepositFormProps) => { saveHasAcknowledgedTerms, } = useAccounts(); - const { addTransferNotification } = useLocalNotifications(); + const { addOrUpdateTransferNotification } = useLocalNotifications(); const { requestPayload, @@ -246,6 +253,7 @@ export const DepositForm = ({ onDeposit, onError }: DepositFormProps) => { const sourceAmountBN = parseUnits(debouncedAmount, sourceToken.decimals); if (sourceAmountBN > (allowance as bigint)) { + setDepositStep(DepositSteps.Approval); const simulateApprove = async (abi: Abi) => publicClientWagmi.simulateContract({ account: evmAddress, @@ -267,7 +275,7 @@ export const DepositForm = ({ onDeposit, onError }: DepositFormProps) => { hash: approveTx, }); } - }, [signerWagmi, sourceToken, sourceChain, requestPayload, publicClientWagmi]); + }, [signerWagmi, publicClientWagmi, sourceToken, requestPayload, evmAddress, debouncedAmount]); const onSubmit = useCallback( async (e: FormEvent) => { @@ -299,10 +307,11 @@ export const DepositForm = ({ onDeposit, onError }: DepositFormProps) => { gasLimit: BigInt(requestPayload.gasLimit || DEFAULT_GAS_LIMIT), value: requestPayload.routeType !== 'SEND' ? BigInt(requestPayload.value) : undefined, }; + setDepositStep(DepositSteps.Confirm); const txHash = await signerWagmi.sendTransaction(tx); if (txHash) { - addTransferNotification({ + addOrUpdateTransferNotification({ txHash, toChainId: !isCctp ? selectedDydxChainId : getNobleChainId(), fromChainId: chainIdStr || undefined, @@ -333,6 +342,7 @@ export const DepositForm = ({ onDeposit, onError }: DepositFormProps) => { setError(err); } finally { setIsLoading(false); + setDepositStep(DepositSteps.Initial); } }, [requestPayload, signerWagmi, chainId, sourceToken, sourceChain] @@ -441,6 +451,18 @@ export const DepositForm = ({ onDeposit, onError }: DepositFormProps) => { debouncedAmountBN, ]); + const depositCTAString = useMemo(() => { + if (depositStep === DepositSteps.Approval) { + return stringGetter({ key: STRING_KEYS.PENDING_TOKEN_APPROVAL }); + } + if (depositStep === DepositSteps.Confirm) { + return stringGetter({ key: STRING_KEYS.PENDING_DEPOSIT_CONFIRMATION }); + } + return hasAcknowledgedTerms + ? stringGetter({ key: STRING_KEYS.DEPOSIT_FUNDS }) + : stringGetter({ key: STRING_KEYS.ACKNOWLEDGE_TERMS_AND_DEPOSIT }); + }, [depositStep, stringGetter, hasAcknowledgedTerms]); + const isDisabled = Boolean(errorMessage) || !sourceToken || @@ -503,6 +525,7 @@ export const DepositForm = ({ onDeposit, onError }: DepositFormProps) => { )} <$Footer> void; sourceToken?: TransferInputTokenResource; + buttonLabel: string; }; export const DepositButtonAndReceipt = ({ @@ -63,12 +60,12 @@ export const DepositButtonAndReceipt = ({ isDisabled, isLoading, setRequireUserActionInWallet, + buttonLabel, }: ElementProps) => { const [isEditingSlippage, setIsEditingSlipapge] = useState(false); const stringGetter = useStringGetter(); const canAccountTrade = useAppSelector(calculateCanAccountTrade, shallowEqual); - const { hasAcknowledgedTerms } = useAppSelector(getOnboardingGuards); const { connectWallet, isConnectedWagmi } = useWalletConnection(); const { connectionError } = useApiState(); @@ -291,12 +288,9 @@ export const DepositButtonAndReceipt = ({ isDisabled: !isFormValid, isLoading: (isFormValid && !requestPayload) || isLoading, }} + withContentOnLoading > - {stringGetter({ - key: hasAcknowledgedTerms - ? STRING_KEYS.DEPOSIT_FUNDS - : STRING_KEYS.ACKNOWLEDGE_TERMS_AND_DEPOSIT, - })} + {buttonLabel} )} diff --git a/src/views/forms/AccountManagementForms/WithdrawForm.tsx b/src/views/forms/AccountManagementForms/WithdrawForm.tsx index bdbcba32a..894bbc11d 100644 --- a/src/views/forms/AccountManagementForms/WithdrawForm.tsx +++ b/src/views/forms/AccountManagementForms/WithdrawForm.tsx @@ -69,6 +69,8 @@ import { log } from '@/lib/telemetry'; import { TokenSelectMenu } from './TokenSelectMenu'; import { WithdrawButtonAndReceipt } from './WithdrawForm/WithdrawButtonAndReceipt'; +const DUMMY_TX_HASH = 'withdraw_dummy_tx_hash'; + export const WithdrawForm = () => { const stringGetter = useStringGetter(); const [error, setError] = useState(); @@ -105,7 +107,7 @@ export const WithdrawForm = () => { [token, resources] ); - const { addTransferNotification } = useLocalNotifications(); + const { addOrUpdateTransferNotification } = useLocalNotifications(); // Async Data const debouncedAmountBN = useMemo(() => MustBigNumber(debouncedAmount), [debouncedAmount]); @@ -159,6 +161,8 @@ export const WithdrawForm = () => { const onSubmit = useCallback( async (e: FormEvent) => { + const notificationId = crypto?.randomUUID() ?? Date.now().toString(); + try { e.preventDefault(); @@ -190,28 +194,41 @@ export const WithdrawForm = () => { }) ); } else { + const nobleChainId = getNobleChainId(); + const toChainId = exchange ? nobleChainId : chainIdStr || undefined; + + const notificationParams = { + id: notificationId, + // DUMMY_TX_HASH is a place holder before we get the real txHash + txHash: DUMMY_TX_HASH, + type: TransferNotificationTypes.Withdrawal, + fromChainId: !isCctp ? selectedDydxChainId : nobleChainId, + toChainId, + toAmount: debouncedAmountBN.toNumber(), + triggeredAt: Date.now(), + isCctp, + isExchange: Boolean(exchange), + requestId: requestPayload.requestId ?? undefined, + }; + + if (isCctp) { + // we want to trigger a dummy notification first since CCTP withdraws can take + // up to 30s to generate a txHash, set isDummy to true + addOrUpdateTransferNotification({ ...notificationParams, isDummy: true }); + } + const txHash = await sendSquidWithdraw( debouncedAmountBN.toNumber(), requestPayload.data, isCctp ); - const nobleChainId = getNobleChainId(); - const toChainId = exchange ? nobleChainId : chainIdStr || undefined; + if (txHash && toChainId) { - addTransferNotification({ - txHash, - type: TransferNotificationTypes.Withdrawal, - fromChainId: !isCctp ? selectedDydxChainId : nobleChainId, - toChainId, - toAmount: debouncedAmountBN.toNumber(), - triggeredAt: Date.now(), - isCctp, - isExchange: Boolean(exchange), - requestId: requestPayload.requestId ?? undefined, - }); abacusStateManager.clearTransferInputValues(); setWithdrawAmount(''); + addOrUpdateTransferNotification({ ...notificationParams, txHash, isDummy: false }); + track( AnalyticsEvents.TransferWithdraw({ chainId: toChainId, @@ -226,6 +243,8 @@ export const WithdrawForm = () => { toAmountMin: summary?.toAmountMin || undefined, }) ); + } else { + throw new Error('No transaction hash returned'); } } } catch (err) { @@ -244,6 +263,14 @@ export const WithdrawForm = () => { : stringGetter({ key: STRING_KEYS.SOMETHING_WENT_WRONG }) ); } + if (isCctp) { + // if error update the dummy notification with error + addOrUpdateTransferNotification({ + id: notificationId, + txHash: DUMMY_TX_HASH, + status: { error: stringGetter({ key: STRING_KEYS.SOMETHING_WENT_WRONG }) }, + }); + } } finally { setIsLoading(false); } @@ -258,6 +285,7 @@ export const WithdrawForm = () => { toToken, screenAddresses, stringGetter, + addOrUpdateTransferNotification, ] ); diff --git a/src/views/notifications/TransferStatusNotification/index.tsx b/src/views/notifications/TransferStatusNotification/index.tsx index d7e9fc81a..f14e17b93 100644 --- a/src/views/notifications/TransferStatusNotification/index.tsx +++ b/src/views/notifications/TransferStatusNotification/index.tsx @@ -122,16 +122,24 @@ export const TransferStatusNotification = ({ slotIcon={isToast && slotIcon} slotTitle={slotTitle} slotCustomContent={ - !status && !isExchange ? ( - - ) : ( - <$BridgingStatus> - {content} - {!isToast && !isComplete && !hasError && ( - <$TransferStatusSteps status={status} type={type} /> - )} - - ) + <$BridgingStatus> + {!status && !isExchange ? ( + <> + {!isComplete &&
{stringGetter({ key: STRING_KEYS.KEEP_WINDOW_OPEN })}
} +
+ +
+ + ) : ( + <> + {content} + {!isComplete &&
{stringGetter({ key: STRING_KEYS.KEEP_WINDOW_OPEN })}
} + {!isToast && !isComplete && !hasError && ( + <$TransferStatusSteps status={status} type={type} /> + )} + + )} + } slotAction={ isToast &&