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.

Sign up for GitHub

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 opt-in props for pingback #330

Closed
wants to merge 6 commits into from
Closed

Add opt-in props for pingback #330

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
b15a6e7
Add prop to opt-in to pingback in Gif and Video
LBBO Nov 28, 2022
d8dacfd
Add prop to opt-in to carousel and grid
LBBO Nov 29, 2022
6c9574b
Add documentation about opt-in
LBBO Nov 29, 2022
f9b5a41
Add opt-in to normal components
LBBO Nov 29, 2022
5195d87
Enable global `npm test`
LBBO Nov 29, 2022
4bd9772
Cherrypick changes from #331
LBBO Nov 29, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
61 changes: 29 additions & 32 deletions .travis.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,51 +1,48 @@
language: node_js
os: linux
node_js:
- '14'
- '14'

env:
# skip Puppeteer download, only needed for Netlify build
- PUPPETEER_SKIP_DOWNLOAD=1
# skip Puppeteer download, only needed for Netlify build
- PUPPETEER_SKIP_DOWNLOAD=1

addons:
apt:
packages:
- libgconf-2-4
apt:
packages:
- libgconf-2-4

cache:
yarn: true
directories:
- 'node_modules'
- '~/.cache'
yarn: true
directories:
- 'node_modules'
- '~/.cache'

before_script:
- yarn global add license-checker
- yarn run lerna bootstrap
- git checkout .
- yarn run cy:verify
- yarn run cy:info
- yarn global add license-checker
- yarn run lerna bootstrap
- git checkout .
- yarn run cy:verify
- yarn run cy:info

script:
- yarn run check-licenses
- yarn run dependency-cruiser
- yarn run lint
- yarn test
# All Percy snapshots except SearchBar have been moved to Cypress, to avoid confusion disable the old "snapshot" command.
# When the Cypress setuisbe verified, this command might be removed.
# - yarn run snapshot
- yarn run cy:run --record --key ${CYPRESS_RECORD_KEY}
# after all tests finish running we need
# to kill all background jobs (like "npm start &")
# this avoids flake in Travis jobs
- kill $(jobs -p) || true
- yarn run check-licenses
- yarn run dependency-cruiser
- yarn run lint
- yarn test
# All Percy snapshots except SearchBar have been moved to Cypress, to avoid confusion disable the old "snapshot" command.
# When the Cypress setuisbe verified, this command might be removed.
# - yarn run snapshot
- yarn run cy:run --record --key ${CYPRESS_RECORD_KEY}

before_deploy:
- echo "//registry.npmjs.org/:_authToken=\${NPM_TOKEN}" >> $HOME/.npmrc
- echo "//registry.npmjs.org/:_authToken=\${NPM_TOKEN}" >> $HOME/.npmrc

deploy:
if: branch = master AND tag IS present
provider: script
- provider: script
script: 'yarn run lerna:publish'
cleanup: false
skip_cleanup: true
on:
tags: true
condition: $TRAVIS_COMMIT_MESSAGE =~ ^Publish
branch: master
condition: $TRAVIS_COMMIT_MESSAGE =~ ^Publish
3 changes: 2 additions & 1 deletion packages/brand/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,8 @@
"clean": "rm -rf ./dist",
"dev": "parcel public/test.html",
"build": "tsc",
"prepublish": "npm run clean && npm run build"
"prepublish": "npm run clean && npm run build",
"test": "tsc --noEmit"
},
"devDependencies": {
"parcel-bundler": "latest",
Expand Down
50 changes: 27 additions & 23 deletions packages/components/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ _renderGrid options_
| gutter | `number` | 6 | The space between columns and rows |
| noResultsMessage | `string \| element` | undefined | Customise the "No results" message |
| noLink | `boolean` | false | Use a `div` instead of an `a` tag for the Gif component, user defines functionality with `onGifClick` |
| optInToTelemetry | `boolean` | false | Opt-in to sending telemetry events to Giphy on certain user interactions |
| [hideAttribution](#attribution-overlay) | `boolean` | false | Hide the user attribution that appears over a |
| [Gif Events](#gif-events) | \* | \* | see below |

Expand Down Expand Up @@ -91,16 +92,17 @@ grid.remove()

_renderCarousel options_

| property | type | default | description |
| --------------------------------------- | ---------------------------------------- | --------- | ----------------------------------------------------------------------------------------------------- |
| gifHeight | `number` | undefined | The height of the gifs and the carousel |
| gifWidth | `number` | undefined | The width of the gifs and the carousel (you may want to set Gif.imgClassName to have object-fit: cover to avoid stretching) |
| fetchGifs | `(offset:number) => Promise<GifsResult>` | undefined | A function that returns a Promise<GifsResult>. Use `@giphy/js-fetch-api` |
| gutter | `number` | 6 | The space between columns and rows |
| noResultsMessage | `string \| element` | undefined | Customise the "No results" message |
| [hideAttribution](#attribution-overlay) | `boolean` | false | Hide the user attribution that appears over a |
| noLink | `boolean` | false | Use a `div` instead of an `a` tag for the Gif component, user defines functionality with `onGifClick` |
| [Gif Events](#gif-events) | \* | \* | see below |
| property | type | default | description |
| --------------------------------------- | ---------------------------------------- | --------- | --------------------------------------------------------------------------------------------------------------------------- |
| gifHeight | `number` | undefined | The height of the gifs and the carousel |
| gifWidth | `number` | undefined | The width of the gifs and the carousel (you may want to set Gif.imgClassName to have object-fit: cover to avoid stretching) |
| fetchGifs | `(offset:number) => Promise<GifsResult>` | undefined | A function that returns a Promise<GifsResult>. Use `@giphy/js-fetch-api` |
| gutter | `number` | 6 | The space between columns and rows |
| noResultsMessage | `string \| element` | undefined | Customise the "No results" message |
| noLink | `boolean` | false | Use a `div` instead of an `a` tag for the Gif component, user defines functionality with `onGifClick` |
| optInToTelemetry | `boolean` | false | Opt-in to sending telemetry events to Giphy on certain user interactions |
| [hideAttribution](#attribution-overlay) | `boolean` | false | Hide the user attribution that appears over a |
| [Gif Events](#gif-events) | \* | \* | see below |

```typescript
import { renderCarousel } from '@giphy/js-components'
Expand Down Expand Up @@ -133,8 +135,9 @@ _Gif props_
| gif | `IGif` | undefined | The gif to display |
| width | `number` | undefined | The width of the gif |
| backgroundColor | `string` | random giphy color | The background of the gif before it loads |
| [hideAttribution](#attribution-overlay) | `boolean` | false | Hide the user attribution that appears over a GIF |
| noLink | `boolean` | false | Use a `div` instead of an `a` tag for the Gif component, user defines functionality with `onGifClick` |
| optInToTelemetry | `boolean` | false | Opt-in to sending telemetry events to Giphy on certain user interactions |
| [hideAttribution](#attribution-overlay) | `boolean` | false | Hide the user attribution that appears over a GIF |
| [Gif Events](#gif-events) | \* | \* | see below |

```typescript
Expand All @@ -160,17 +163,18 @@ If you want controls for the video player, use the `controls` property.

_Video props_

| _prop_ | _type_ | _default_ | _description_ |
| ------------------ | -------------------------- | --------- | ------------------------------------------------ |
| gif | `IGif` | undefined | The gif to display that contains video data |
| width | `number` | undefined | The width of the video |
| height | `number` | undefined | The height of the video |
| controls | `boolean` | undefined | Show transport controls |
| hideProgressBar | `boolean` | undefined | if controls is true, hides progress bar |
| hideMute | `boolean` | undefined | if controls is true, hides the mute button |
| hidePlayPause | `boolean` | undefined | if controls is true, hides the play/pause button |
| persistentControls | `boolean` | undefined | don't hide controls when hovering away |
| onUserMuted | `(muted: boolean) => void` | undefined | fired when the user toggles the mute state |
| _prop_ | _type_ | _default_ | _description_ |
| ------------------ | -------------------------- | --------- | ------------------------------------------------------------------------ |
| gif | `IGif` | undefined | The gif to display that contains video data |
| width | `number` | undefined | The width of the video |
| height | `number` | undefined | The height of the video |
| controls | `boolean` | undefined | Show transport controls |
| hideProgressBar | `boolean` | undefined | if controls is true, hides progress bar |
| hideMute | `boolean` | undefined | if controls is true, hides the mute button |
| hidePlayPause | `boolean` | undefined | if controls is true, hides the play/pause button |
| persistentControls | `boolean` | undefined | don't hide controls when hovering away |
| onUserMuted | `(muted: boolean) => void` | undefined | fired when the user toggles the mute state |
| optInToTelemetry | `boolean` | false | Opt-in to sending telemetry events to Giphy on certain user interactions |

```typescript
import { renderVideo } from '@giphy/js-components'
Expand Down Expand Up @@ -206,4 +210,4 @@ If a GIF has an associated user, an overlay with their avatar and display name w

```

To stop fonts from loading set the environment variable `GIPHY_SDK_NO_FONTS=true`, this is not recommended as it could cause inconsistencies in the ui components
To stop fonts from loading set the environment variable `GIPHY_SDK_NO_FONTS=true`, this is not recommended as it could cause inconsistencies in the ui components
3 changes: 2 additions & 1 deletion packages/components/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,8 @@
"types": "tsc ./src/index.tsx -d --emitDeclarationOnly -declarationDir ./dist",
"dev": "parcel public/test.html",
"build": "tsc",
"prepublish": "npm run clean && tsc"
"prepublish": "npm run clean && tsc",
"test": "tsc --noEmit"
},
"devDependencies": {
"@types/bricks.js": "^1.8.1",
Expand Down
4 changes: 4 additions & 0 deletions packages/components/src/components/carousel.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,8 @@ type Props = {
noLink?: boolean
tabIndex?: number
borderRadius?: number
// Enables telemetry; opt-in
optInToTelemetry?: boolean
} & EventProps

const defaultProps = Object.freeze({ gutter: 6, user: {} })
Expand Down Expand Up @@ -121,6 +123,7 @@ class Carousel extends Component<Props, State> {
noLink,
tabIndex = 0,
borderRadius,
optInToTelemetry = false,
}: Props,
{ gifs }: State
) {
Expand Down Expand Up @@ -155,6 +158,7 @@ class Carousel extends Component<Props, State> {
hideAttribution={hideAttribution}
noLink={noLink}
borderRadius={borderRadius}
optInToTelemetry={optInToTelemetry}
/>
)
})}
Expand Down
23 changes: 16 additions & 7 deletions packages/components/src/components/gif.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,8 @@ type GifProps = {
noLink?: boolean
borderRadius?: number
tabIndex?: number
// Enables telemetry; opt-in
optInToTelemetry?: boolean
}

export type Props = GifProps & EventProps
Expand All @@ -91,6 +93,7 @@ const Gif = ({
noLink = false,
borderRadius = 4,
tabIndex,
optInToTelemetry = false,
}: Props) => {
// only fire seen once per gif id
const [hasFiredSeen, setHasFiredSeen] = useState(false)
Expand Down Expand Up @@ -118,9 +121,11 @@ const Gif = ({
const onMouseOver = (e: Event) => {
clearTimeout(hoverTimeout.current!)
setHovered(true)
hoverTimeout.current = window.setTimeout(() => {
pingback.onGifHover(gif, user?.id, e.target as HTMLElement, attributes)
}, hoverTimeoutDelay)
if (optInToTelemetry) {
hoverTimeout.current = window.setTimeout(() => {
pingback.onGifHover(gif, user?.id, e.target as HTMLElement, attributes)
}, hoverTimeoutDelay)
}
}

const onMouseLeave = () => {
Expand All @@ -129,8 +134,10 @@ const Gif = ({
}

const onClick = (e: Event) => {
// fire pingback
pingback.onGifClick(gif, user?.id, e.target as HTMLElement, attributes)
if (optInToTelemetry) {
// fire pingback
pingback.onGifClick(gif, user?.id, e.target as HTMLElement, attributes)
}
onGifClick(gif, e)
}

Expand All @@ -143,8 +150,10 @@ const Gif = ({
// flag so we don't observe any more
setHasFiredSeen(true)
Logger.debug(`GIF ${gif.id} seen. ${gif.title}`)
// fire pingback
pingback.onGifSeen(gif, user?.id, entry.boundingClientRect, attributes)
if (optInToTelemetry) {
// fire pingback
pingback.onGifSeen(gif, user?.id, entry.boundingClientRect, attributes)
}
// fire custom onGifSeen
onGifSeen?.(gif, entry.boundingClientRect)
// disconnect
Expand Down
4 changes: 4 additions & 0 deletions packages/components/src/components/grid.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,8 @@ type Props = {
noLink?: boolean
tabIndex?: number
borderRadius?: number
// Enables telemetry; opt-in
optInToTelemetry?: boolean
} & EventProps
const defaultProps = Object.freeze({ gutter: 6, user: {} })

Expand Down Expand Up @@ -158,6 +160,7 @@ class Grid extends Component<Props, State> {
noLink,
tabIndex = 0,
borderRadius,
optInToTelemetry = false,
}: Props,
{ gifWidth, gifs, isError, isDoneFetching }: State
) {
Expand All @@ -183,6 +186,7 @@ class Grid extends Component<Props, State> {
hideAttribution={hideAttribution}
noLink={noLink}
borderRadius={borderRadius}
optInToTelemetry={optInToTelemetry}
/>
))}
{!showLoader && gifs.length === 0 && noResultsMessage}
Expand Down
7 changes: 5 additions & 2 deletions packages/components/src/components/video/video.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,8 @@ type Props = {
height?: number
volume?: number
className?: string
// Enables telemetry; opt-in
optInToTelemetry?: boolean
}
const Video = ({
muted,
Expand All @@ -59,6 +61,7 @@ const Video = ({
height: height_,
volume = 0.7,
className,
optInToTelemetry = false,
}: Props) => {
const height = height_ || getGifHeight(gif, width)

Expand Down Expand Up @@ -123,12 +126,12 @@ const Video = ({
onStateChange?.('playing')
if (!hasPlayingFired.current) {
hasPlayingFired.current = true
if (gif.analytics_response_payload) {
if (optInToTelemetry && gif.analytics_response_payload) {
pingback({ actionType: 'START', analyticsResponsePayload: gif.analytics_response_payload })
}
onFirstPlay?.(Date.now() - mountTime.current)
}
}, [onFirstPlay, onStateChange])
}, [onFirstPlay, onStateChange, optInToTelemetry])
const _onPaused = useCallback(() => onStateChange?.('paused'), [onStateChange])
const _onTimeUpdate = useCallback(() => {
const el = videoEl.current
Expand Down
Loading