-
Notifications
You must be signed in to change notification settings - Fork 502
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
New feature: Add custom ad banner #1202
Changes from 7 commits
2f0d608
2e9c7f4
07cc436
a266d1a
e0fac6c
20b7b8f
3ee3677
2869118
1e285de
9613380
5b41dc1
88b86d1
3ece9af
449699d
815b67d
4131076
a0c59fa
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -228,9 +228,27 @@ This feature is **enabled by default** with the `slise` ads provider. To switch | |||||
|
||||||
| Variable | Type| Description | Compulsoriness | Default value | Example value | | ||||||
| --- | --- | --- | --- | --- | --- | | ||||||
| NEXT_PUBLIC_AD_BANNER_PROVIDER | `slise` \| `adbutler` \| `coinzilla` \| `none` | Ads provider | - | `slise` | `coinzilla` | | ||||||
| NEXT_PUBLIC_AD_BANNER_PROVIDER | `slise` \| `adbutler` \| `coinzilla` \| `custom` \| `none` | Ads provider | - | `slise` | `coinzilla` | | ||||||
| NEXT_PUBLIC_AD_ADBUTLER_CONFIG_DESKTOP | `{ id: string; width: string; height: string }` | Placement config for desktop Adbutler banner | - | - | `{'id':'123456','width':'728','height':'90'}` | | ||||||
| NEXT_PUBLIC_AD_ADBUTLER_CONFIG_MOBILE | `{ id: string; width: number; height: number }` | Placement config for mobile Adbutler banner | - | - | `{'id':'654321','width':'300','height':'100'}` | | ||||||
| NEXT_PUBLIC_AD_CUSTOM_CONFIG_URL | `string` | URL of configuration file (.json format only) which contains settings and list of custom banners that will be shown in the home page and token detail page. See below list of available properties for particular banner | - | - | `https://example.com/ad_custom_config.json` | | ||||||
|
||||||
#### Configuration properties | ||||||
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.
Suggested change
just a little bit more specific |
||||||
| Variable | Type | Description | Compulsoriness | Default value | Example value | | ||||||
| --- | --- | --- | --- | --- | --- | | ||||||
| banners | `array` | List of banners with their properties. Refer to the "Custom banners configuration properties" section below. | Required | - | See below | | ||||||
| interval | `number` | Duration (in milliseconds) for how long each banner will be displayed. | - | 60000 | `6000` | | ||||||
|
||||||
| ||||||
|
||||||
#### Custom banners configuration properties | ||||||
|
||||||
| Variable | Type | Description | Compulsoriness | Default value | Example value | | ||||||
| --- | --- | --- | --- | --- | --- | | ||||||
| text | `string` | Tooltip text displayed when the mouse is moved over the banner. | - | - | - | | ||||||
| url | `string` | Link that opens when clicking on the banner. | Required | - | `https://example.com` | | ||||||
| desktop | `string` | Banner image (both .png, .jpg, and .gif are acceptable) used when the screen width is greater than 1000px. | Required | - | `https://example.com/configs/ad-custom-banners/desktop/example.gif` | | ||||||
| mobile | `string` | Banner image (both .png, .jpg, and .gif are acceptable) used when the screen width is less than 1000px. | Required | - | `https://example.com/configs/ad-custom-banners/mobile/example.gif` | | ||||||
jasonzysun marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
|
||||||
| ||||||
|
||||||
|
@@ -315,7 +333,7 @@ This feature is **always enabled**, but you can configure its behavior by passin | |||||
|
||||||
#### Marketplace app configuration properties | ||||||
|
||||||
| Property | Type | Description | Compulsoriness | Example value | ||||||
| Property | Type | Description | Compulsoriness | Example value | | ||||||
| --- | --- | --- | --- | --- | | ||||||
| id | `string` | Used as slug for the app. Must be unique in the app list. | Required | `'app'` | | ||||||
| external | `boolean` | `true` means that the application opens in a new window, but not in an iframe. | - | `true` | | ||||||
|
jasonzysun marked this conversation as resolved.
Show resolved
Hide resolved
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,7 +1,24 @@ | ||
import type { ArrayElement } from 'types/utils'; | ||
|
||
export const SUPPORTED_AD_BANNER_PROVIDERS = [ 'slise', 'adbutler', 'coinzilla', 'none' ] as const; | ||
export const SUPPORTED_AD_BANNER_PROVIDERS = [ 'slise', 'adbutler', 'coinzilla', 'custom', 'none' ] as const; | ||
export type AdBannerProviders = ArrayElement<typeof SUPPORTED_AD_BANNER_PROVIDERS>; | ||
|
||
export const SUPPORTED_AD_TEXT_PROVIDERS = [ 'coinzilla', 'none' ] as const; | ||
export type AdTextProviders = ArrayElement<typeof SUPPORTED_AD_TEXT_PROVIDERS>; | ||
|
||
export type AdButlerConfig = { | ||
id: string; | ||
width: string; | ||
height: string; | ||
} | ||
export type AdCustomBannerConfig = { | ||
text: string; | ||
url: string; | ||
desktop: string; | ||
mobile: string; | ||
} | ||
|
||
export type AdCustomConfig = { | ||
banners: Array<AdCustomBannerConfig>; | ||
interval: number; | ||
} |
This file was deleted.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,61 @@ | ||
import { Flex, chakra, Tooltip, Image } from '@chakra-ui/react'; | ||
import { useQuery } from '@tanstack/react-query'; | ||
import React, { useState, useEffect } from 'react'; | ||
|
||
import type { AdCustomConfig } from 'types/client/ad'; | ||
|
||
import config from 'configs/app'; | ||
import type { ResourceError } from 'lib/api/resources'; | ||
import useFetch from 'lib/hooks/useFetch'; | ||
|
||
import useIsMobile from '../../../lib/hooks/useIsMobile'; | ||
jasonzysun marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
const CustomAdBanner = ({ className }: { className?: string }) => { | ||
const isMobile = useIsMobile(); | ||
|
||
const feature = config.features.adsBanner; | ||
const configUrl = (feature.isEnabled && feature.provider === 'custom') ? feature.configUrl : ''; | ||
|
||
const apiFetch = useFetch(); | ||
const { data: adConfig } = useQuery<unknown, ResourceError<unknown>, AdCustomConfig>( | ||
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 need to manage the following states of the component as well:
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. Good idea, I will add these two states |
||
[ 'custom-configs' ], | ||
jasonzysun marked this conversation as resolved.
Show resolved
Hide resolved
|
||
async() => apiFetch(configUrl), | ||
{ | ||
enabled: feature.isEnabled && feature.provider === 'custom', | ||
staleTime: Infinity, | ||
}); | ||
const interval = adConfig?.interval || 60000; | ||
jasonzysun marked this conversation as resolved.
Show resolved
Hide resolved
|
||
const banners = adConfig?.banners || []; | ||
|
||
const [ currentBannerIndex, setCurrentBannerIndex ] = useState(0); | ||
jasonzysun marked this conversation as resolved.
Show resolved
Hide resolved
|
||
useEffect(() => { | ||
if (banners.length === 0) { | ||
return; | ||
} | ||
const timer = setInterval(() => { | ||
setCurrentBannerIndex((prevIndex) => (prevIndex + 1) % banners.length); | ||
}, interval); | ||
|
||
return () => { | ||
clearInterval(timer); | ||
}; | ||
}, [ interval, banners.length ]); | ||
|
||
if (banners.length === 0) { | ||
jasonzysun marked this conversation as resolved.
Show resolved
Hide resolved
|
||
return null; | ||
} | ||
|
||
const currentBanner = banners[currentBannerIndex]; | ||
|
||
return ( | ||
<Flex className={ className } h="90px"> | ||
<Tooltip label={ currentBanner.text } aria-label={ currentBanner.text }> | ||
<a href={ currentBanner.url } target="_blank" rel="noopener noreferrer"> | ||
<Image src={ isMobile ? currentBanner.mobile : currentBanner.desktop } alt={ currentBanner.text } height="100%" width="auto" borderRadius="md"/> | ||
</a> | ||
</Tooltip> | ||
</Flex> | ||
); | ||
}; | ||
|
||
export default chakra(CustomAdBanner); |
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.
Have you checked the schema validation localy as it is described in docs?
I guess we are missing
.json()
here, so the yup can parse the string to the actual JSON.Also in
deploy/tools/envs-validator/index.ts
you need to replace ENV value with the content of the json-file in order to make the validator work.Can you please temporarily add this config ENV to the
.env.main
preset file to demonstrate that everything works as expected? I will run the validator in our CI then. After that, you can comment it out. I know it is not the best workflow, but we are trying to improve it.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.
@jasonzysun I have just merged some changes in the validation script, now it should be easier to add tests for the new variables. I've described all the necessary steps in the contribution guide. Check it out and let me know if you have any questions.
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.
That's great to hear. I haven't been able to address this section of the content lately, but I'll take care of it in the coming days.