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

Jump to bottom

Remove adroll #4029

Merged
merged 4 commits into from
Sep 24, 2024
Merged

Remove adroll #4029

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
859531b
Remove adroll
piekczyk Sep 23, 2024
925538c
related stuff removal
piekczyk Sep 23, 2024
7438b75
bring back cookie info
piekczyk Sep 23, 2024
a61cb9d
Update public/locales/cn/common.json
piekczyk Sep 24, 2024
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
31 changes: 0 additions & 31 deletions analytics/adroll.ts
View file
Open in desktop

This file was deleted.

6 changes: 4 additions & 2 deletions pages/_app.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ import { CacheProvider, Global } from '@emotion/core'
import { Web3OnboardProvider } from '@web3-onboard/react'
import type { AbstractConnector } from '@web3-react/abstract-connector'
import { Web3ReactProvider } from '@web3-react/core'
import { adRollPixelScript } from 'analytics/adroll'
import { COOKIE_NAMES_LOCASTORAGE_KEY } from 'analytics/common'
import { mixpanelInit } from 'analytics/mixpanel'
import { trackingEvents } from 'analytics/trackingEvents'
Expand Down Expand Up @@ -155,7 +154,10 @@ function App({ Component, pageProps }: AppProps & CustomAppProps) {
<meta name="viewport" content="width=device-width, initial-scale=1" />
</Head>
{cookiesValue?.enabledCookies?.marketing && (
<Script dangerouslySetInnerHTML={{ __html: adRollPixelScript }} async />
// Use when there is a need to load extra script required for marketing
// Example usage:
// <Script dangerouslySetInnerHTML={{ __html: scriptToBeLoaded }} async />
<></>
Comment on lines +157 to +160
Copy link
Contributor

Choose a reason for hiding this comment

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

Tip

Codebase Verification

Remaining AdRoll References Detected

The AdRoll-related code has not been fully removed. Please address the following instances to ensure complete elimination:

  • runtime.config.js

    • adRollAdvId: process.env.ADROLL_ADV_ID,
    • adRollPixId: process.env.ADROLL_PIX_ID,

  • public/locales/en/common.json & public/locales/cn/common.json

    • Descriptions referencing AdRoll cookies.

  • features/content/cookie/en.tsx

    • Multiple <td>Adroll</td> entries.
    • Usage of __adroll_fpc.

  • analytics/manageCookie.ts

    • Comments related to AdRoll instance management.

  • README.md

    • References to ADROLL_PIX_ID and ADROLL_ADV_ID.
    • Descriptions mentioning AdRoll Pixel.

  • Dockerfile.production

    • Environment variables for ADROLL_ADV_ID and ADROLL_PIX_ID.

Please ensure that all AdRoll-related configurations, comments, and documentation are thoroughly removed to maintain codebase integrity.

Analysis chain

Approve AdRoll removal and suggest minor improvement

The removal of the AdRoll script is correctly implemented, aligning with the PR objective. The comment provides clear guidance for future marketing script additions, which is helpful.

However, we can make a small improvement:

Consider removing the empty fragment on line 160 as it's unnecessary:

  {cookiesValue?.enabledCookies?.marketing && (
    // Use when there is a need to load extra script required for marketing
    // Example usage:
    // <Script dangerouslySetInnerHTML={{ __html: scriptToBeLoaded }} async />
-   <></>
  )}

This change addresses the static analysis hint and slightly improves code cleanliness.

Let's verify the complete removal of AdRoll-related code:

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check for any remaining AdRoll-related code

# Test 1: Search for 'adroll' or 'AdRoll' in all files
echo "Searching for 'adroll' or 'AdRoll' in all files:"
rg -i 'adroll'

# Test 2: Check for any remaining imports from 'analytics/adroll'
echo "Checking for imports from 'analytics/adroll':"
rg "from 'analytics/adroll'"

# Test 3: Check if the 'analytics/adroll.ts' file still exists
echo "Checking if 'analytics/adroll.ts' file exists:"
fd 'adroll.ts' analytics

Length of output: 1606

Tools
Biome

[error] 160-160: Avoid using unnecessary Fragment.

A fragment is redundant if it contains only one child, or if it is the child of a html element, and is not a keyed fragment.

(lint/complexity/noUselessFragments)

)}
<ThemeUIProvider theme={theme}>
<CacheProvider value={cache}>
Expand Down
Loading