-
Notifications
You must be signed in to change notification settings - Fork 105
feat: add vip telegram notification for high-value users #1901
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -45,9 +45,12 @@ import { CloseAllPositionsNotification } from '@/views/notifications/CloseAllPos | |
| import { OrderCancelNotification } from '@/views/notifications/OrderCancelNotification'; | ||
| import { OrderStatusNotification } from '@/views/notifications/OrderStatusNotification'; | ||
| import { TradeNotification } from '@/views/notifications/TradeNotification'; | ||
| import { VipTelegramNotification } from '@/views/notifications/VipTelegramNotification'; | ||
|
|
||
| import { getUserWalletAddress } from '@/state/accountInfoSelectors'; | ||
| import { | ||
| getIsAccountConnected, | ||
| getSubaccountEquity, | ||
| selectOrphanedTriggerOrders, | ||
| selectReclaimableChildSubaccountFunds, | ||
| selectShouldAccountRebalanceUsdc, | ||
|
|
@@ -85,6 +88,7 @@ import { useAppSelectorWithArgs } from './useParameterizedSelector'; | |
| import { useAllStatsigDynamicConfigValues } from './useStatsig'; | ||
| import { useStringGetter } from './useStringGetter'; | ||
| import { useURLConfigs } from './useURLConfigs'; | ||
| import { useLoadedVaultAccount } from './vaultsHooks'; | ||
|
|
||
| export const notificationTypes: NotificationTypeConfig[] = [ | ||
| { | ||
|
|
@@ -1175,6 +1179,73 @@ export const notificationTypes: NotificationTypeConfig[] = [ | |
| }; | ||
| }, | ||
| }, | ||
| { | ||
| type: NotificationType.VipTelegram, | ||
| useTrigger: ({ trigger }) => { | ||
| const stringGetter = useStringGetter(); | ||
| const isConnected = useAppSelector(getIsAccountConnected); | ||
| const equity = useAppSelector(getSubaccountEquity, shallowEqual); | ||
| const { data: vaultAccount } = useLoadedVaultAccount(); | ||
| const vaultBalance = vaultAccount?.balanceUsdc; | ||
|
|
||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. After following steps in https://github.com/dydxprotocol/v4-web/pull/1901/files/50fdb651b95914e94f9965d01573902e02f13b37#r2383062316 You can use
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The reason we want to go through the trouble of adding it to the env config is so third party deployers that fork this repo are not forced to use these links |
||
| useEffect(() => { | ||
| if (!isConnected) { | ||
| return; | ||
| } | ||
|
|
||
| if (sessionStorage.getItem('vip-telegram-notification-shown')) { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. we can use normal notification behavior to not re-show this, I think this is entirely unnecessary |
||
| return; | ||
| } | ||
|
|
||
| const totalValue = equity != null ? equity + (vaultBalance ?? 0) : undefined; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. could probably just be (equity ?? 0) + (vaultBalance ?? 0) |
||
| if (totalValue == null || totalValue < 50000) { | ||
| return; | ||
| } | ||
|
|
||
| const title = stringGetter({ | ||
| key: STRING_KEYS.YOURE_INVITED, | ||
| fallback: "You're Invited", | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. probably don't need fallbacks here either |
||
| }); | ||
| const body = stringGetter({ | ||
| key: STRING_KEYS.VIP_TELEGRAM_BODY, | ||
| fallback: | ||
| 'Private Telegram Group for VIP traders. Enjoy white glove service, special incentives and competitions', | ||
| }); | ||
|
|
||
| if (!title || !body) { | ||
| return; | ||
| } | ||
|
|
||
| sessionStorage.setItem('vip-telegram-notification-shown', 'true'); | ||
|
|
||
| trigger({ | ||
| id: `vip-telegram-invitation-${Date.now()}`, | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. why include date in the id?
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think you need to remove it to get the notification localstorage behavior working |
||
| displayData: { | ||
| title, | ||
| body, | ||
| renderCustomBody: ({ isToast, notification }) => ( | ||
| <VipTelegramNotification | ||
| isToast={isToast} | ||
| notification={notification} | ||
| portfolioValue={totalValue} | ||
| telegramUrl="https://t.me/+NLOpf5jiAEVjODFh" | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. weird that this is hardcoded in two places |
||
| /> | ||
| ), | ||
| groupKey: NotificationType.VipTelegram, | ||
| toastSensitivity: 'foreground', | ||
| toastDuration: 15000, | ||
| }, | ||
| updateKey: ['vip-telegram-invitation'], | ||
| }); | ||
| }, [equity, isConnected, stringGetter, trigger, vaultBalance]); | ||
| }, | ||
|
|
||
| useNotificationAction: () => { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. can probably just remove this or remove the comment at least |
||
| return () => { | ||
| // Optional: track analytics when user clicks | ||
| }; | ||
| }, | ||
| }, | ||
| ]; | ||
|
|
||
| const $Icon = tw.img`h-1.5 w-1.5`; | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -83,8 +83,8 @@ export const MarketsStats = (props: MarketsStatsProps) => { | |
| <$FreeDepositBanner> | ||
| <div tw="flex items-center justify-between gap-0.75"> | ||
| <div tw="relative z-10 mr-auto flex max-w-10 flex-col"> | ||
| <span tw="mb-0.75 leading-[1.2] font-extra-large-bold"> | ||
| <span tw="mr-0.25 rounded-0 bg-color-layer-1 px-0.25 text-color-accent"> | ||
| <span tw="mb-0.75 !leading-tight font-extra-large-bold"> | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. why is ! required?
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why are these changes here?
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't think that the |
||
| <span tw="mr-0.25 rounded-0 bg-color-text-2 px-0.25 text-color-accent"> | ||
| {stringGetter({ key: STRING_KEYS.FREE_DEPOSIT_BANNER_TITLE_FREE })} | ||
| </span> | ||
| <span tw="text-color-text-2"> | ||
|
|
@@ -97,7 +97,7 @@ export const MarketsStats = (props: MarketsStatsProps) => { | |
| type={ButtonType.Button} | ||
| tw="relative z-10 w-full border-none bg-color-layer-0 text-color-text-2" | ||
| onClick={handleFreeDepositClick} | ||
| state={{ isDisabled: isOnboardingDisabled }} | ||
| state={{ isDisabled: Boolean(isOnboardingDisabled) }} | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't think this is necessary either |
||
| > | ||
| {stringGetter({ key: STRING_KEYS.FREE_DEPOSIT_BANNER_CTA })} | ||
| </Button> | ||
|
|
@@ -110,7 +110,7 @@ export const MarketsStats = (props: MarketsStatsProps) => { | |
| <img | ||
| src="/free-deposit-hedgie.png" | ||
| alt="free deposit hedgie" | ||
| tw="absolute bottom-0 left-7 z-0 h-14 object-contain mobile:hidden" | ||
| tw="absolute bottom-0 left-6 z-0 h-full object-contain mobile:hidden" | ||
| /> | ||
| </div> | ||
| </$FreeDepositBanner> | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,77 @@ | ||
| import styled from 'styled-components'; | ||
| import tw from 'twin.macro'; | ||
|
|
||
| import { ButtonAction, ButtonType } from '@/constants/buttons'; | ||
| import { STRING_KEYS } from '@/constants/localization'; | ||
|
|
||
| import { useStringGetter } from '@/hooks/useStringGetter'; | ||
|
|
||
| import { Button } from '@/components/Button'; | ||
| // eslint-disable-next-line import/no-cycle | ||
| import { Notification, type NotificationProps } from '@/components/Notification'; | ||
|
|
||
| type ElementProps = { | ||
| portfolioValue: number; | ||
| telegramUrl?: string; | ||
| }; | ||
|
|
||
| export type VipTelegramNotificationProps = NotificationProps & ElementProps; | ||
|
|
||
| export const VipTelegramNotification = ({ | ||
| isToast, | ||
| notification, | ||
| portfolioValue: _portfolioValue, | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. why start with _?
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can this be removed? |
||
| telegramUrl = 'https://t.me/+NLOpf5jiAEVjODFh', | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @jaredvu do you think we should make this a config of some sort? Any risk here?
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. can we remove the default
then you have to update |
||
| }: VipTelegramNotificationProps) => { | ||
| const stringGetter = useStringGetter(); | ||
|
|
||
| const title = stringGetter({ | ||
| key: STRING_KEYS.YOURE_INVITED, | ||
| fallback: "You're Invited", | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. let's remove fallbacks for now, it might actually be a good idea but it's best to follow the practices in the rest of the app |
||
| }); | ||
|
|
||
| const description = stringGetter({ | ||
| key: STRING_KEYS.VIP_TELEGRAM_BODY, | ||
| fallback: | ||
| 'Private Telegram Group for VIP traders. Enjoy white glove service, special incentives and competitions', | ||
| }); | ||
|
|
||
| const handleViewClick = () => { | ||
| window.open(telegramUrl, '_blank', 'noopener,noreferrer'); | ||
| }; | ||
|
|
||
| return ( | ||
| <Notification | ||
| isToast={isToast} | ||
| notification={notification} | ||
| slotIcon={<$Icon src="/telegram-noti.svg" alt="Telegram" />} | ||
| slotTitle={title} | ||
| slotCustomContent={ | ||
| <$Content> | ||
| <$Description>{description}</$Description> | ||
| <Button | ||
| action={ButtonAction.Primary} | ||
| type={ButtonType.Button} | ||
| onClick={handleViewClick} | ||
| tw="flex-shrink-0 rounded-0.5 border-none bg-color-layer-6 text-small text-color-text-button font-base-medium hover:bg-color-layer-5" | ||
| > | ||
| {stringGetter({ key: STRING_KEYS.VIEW })} | ||
| </Button> | ||
| </$Content> | ||
| } | ||
| /> | ||
| ); | ||
| }; | ||
|
|
||
| const $Icon = styled.img` | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think we should just inline all of these with tw='' |
||
| ${tw`h-3 w-3 flex-shrink-0`}; | ||
| `; | ||
|
|
||
| const $Content = styled.div` | ||
| ${tw`mt-0.5 flex items-center justify-between gap-1`}; | ||
| `; | ||
|
|
||
| const $Description = styled.p` | ||
| ${tw`m-0 text-color-text-0 font-small-book`}; | ||
| line-height: 110%; | ||
| `; | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
was this failing lint or something?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Probably best to undo this
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
forcedTurnkey will never be nullish