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.

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

Feat: white label branding + domain checks #4593

Open
wants to merge 12 commits into
base: dev
Choose a base branch
from
Open

Feat: white label branding + domain checks #4593

wants to merge 12 commits into from

Conversation

katspaugh
Copy link
Member

@katspaugh katspaugh commented Dec 3, 2024

What it solves

To show or hide trademark elements like the Safe Wallet logo, legal pages etc, do a runtime check of the current domain in addition to an env var.

Previously, we were only checking an env variable which apparently wasn't enough as some forks are still using Safe{Wallet} branding. I've also extended this check to the logo in the header.

How does it solve it

I've added two new environment variables:

  • NEXT_PUBLIC_BRAND_NAME – set to Safe{Wallet} for the official distribution, falls back to Wallet fork.
  • NEXT_PUBLIC_BRAND_LOGO – takes the URL of the main logo in the header

@katspaugh katspaugh requested a review from usame-algan December 3, 2024 10:03
Copy link

github-actions bot commented Dec 3, 2024

Copy link

github-actions bot commented Dec 3, 2024

📦 Next.js Bundle Analysis for safe-wallet-web

This analysis was generated by the Next.js Bundle Analysis action. 🤖

⚠️ Global Bundle Size Increased

Page Size (compressed)
global 1 MB (🟡 +228 B)
Details

The global bundle is the javascript bundle that loads alongside every page. It is in its own category because its impact is much higher - an increase to its size means that every page on your website loads slower, and a decrease means every page loads faster.

Any third party scripts you have added directly to your app using the <script> tag are not accounted for in this analysis

If you want further insight into what is behind the changes, give @next/bundle-analyzer a try!

Thirty-nine Pages Changed Size

The following pages changed size from the code in this PR compared to its base branch:

Page Size (compressed) First Load
/_offline 1.1 KB (🟡 +9 B) 1 MB
/addOwner 564 B (🟡 +9 B) 1 MB
/address-book 23.07 KB (🟢 -6 B) 1.03 MB
/apps 33.27 KB (🟡 +15 B) 1.04 MB
/apps/bookmarked 475 B (🟡 +11 B) 1 MB
/apps/custom 31.33 KB (🟡 +16 B) 1.03 MB
/apps/open 53.65 KB (-1 B) 1.06 MB
/balances 29.81 KB (-1 B) 1.03 MB
/balances/nfts 9.63 KB (🟢 -2 B) 1.01 MB
/bridge 2.56 KB (🟡 +12 B) 1.01 MB
/cookie 8.77 KB (🟡 +27 B) 1.01 MB
/home 58.74 KB (🟡 +2 B) 1.06 MB
/imprint 1.41 KB (🟡 +25 B) 1 MB
/licenses 2.46 KB (🟡 +24 B) 1.01 MB
/new-safe/advanced-create 26.45 KB (🟡 +3 B) 1.03 MB
/new-safe/create 25.58 KB (🟡 +9 B) 1.03 MB
/new-safe/load 6.06 KB (🟡 +9 B) 1.01 MB
/privacy 14.57 KB (🟡 +29 B) 1.02 MB
/settings 475 B (🟡 +9 B) 1 MB
/settings/appearance 2.33 KB (🟡 +10 B) 1.01 MB
/settings/cookies 1.96 KB (🟡 +8 B) 1 MB
/settings/environment-variables 3.36 KB (🟡 +3 B) 1.01 MB
/settings/modules 4.13 KB (🟡 +4 B) 1.01 MB
/settings/notifications 21.39 KB (🟡 +4 B) 1.02 MB
/settings/safe-apps 17.83 KB (🟢 -2 B) 1.02 MB
/settings/security 2.43 KB (🟡 +14 B) 1.01 MB
/settings/setup 30.8 KB (🟡 +4 B) 1.03 MB
/share/safe-app 7.57 KB (🟡 +3 B) 1.01 MB
/stake 619 B (🟡 +10 B) 1 MB
/swap 761 B (🟡 +12 B) 1 MB
/terms 12.93 KB (🟡 +23 B) 1.02 MB
/transactions 96.88 KB (🟡 +10 B) 1.1 MB
/transactions/history 96.84 KB (🟡 +10 B) 1.1 MB
/transactions/messages 58.39 KB (🟡 +1 B) 1.06 MB
/transactions/msg 54.56 KB (🟡 +9 B) 1.06 MB
/transactions/queue 47.52 KB (🟡 +8 B) 1.05 MB
/transactions/tx 46.78 KB (🟡 +9 B) 1.05 MB
/welcome 6.82 KB (🟡 +16 B) 1.01 MB
/welcome/accounts 409 B (🟡 +8 B) 1 MB
Details

Only the gzipped size is provided here based on an expert tip.

First Load is the size of the global bundle plus the bundle for the individual page. If a user were to show up to your website and land on a given page, the first load size represents the amount of javascript that user would need to download. If next/link is used, subsequent page loads would only need to download that page's bundle (the number in the "Size" column), since the global bundle has already been downloaded.

Any third party scripts you have added directly to your app using the <script> tag are not accounted for in this analysis

Next to the size is how much the size has increased or decreased compared with the base branch of this PR. If this percentage has increased by 20% or more, there will be a red status indicator applied, indicating that special attention should be given to this.

Copy link

github-actions bot commented Dec 3, 2024

Coverage report

St.
Category Percentage Covered / Total
🟡 Statements
73.72% (+0.01% 🔼)
14394/19525
🔴 Branches
51.08% (+0.01% 🔼)
3421/6697
🔴 Functions
56.4% (-0.03% 🔻)
2028/3596
🟡 Lines
75.29% (+0.01% 🔼)
13061/17347
Show new covered files 🐣
St.
File Statements Branches Functions Lines
🟡
... / useIsOfficialHost.ts
50% 0% 0% 60%
Show files with reduced coverage 🔻
St.
File Statements Branches Functions Lines
🟢
... / constants.ts
100%
97.5% (-2.5% 🔻)
100% 100%
🟡
... / index.tsx
72.73% (-4.55% 🔻)
75% (-25% 🔻)
60%
71.43% (-4.76% 🔻)
🟢
... / index.tsx
89.36% (+0.73% 🔼)
66.67% (-3.92% 🔻)
50%
93.18% (+0.5% 🔼)
🟡
... / index.tsx
65.22% (-1.45% 🔻)
0% 0%
71.43% (-2.26% 🔻)

Test suite run success

1666 tests passing in 227 suites.

Report generated by 🧪jest coverage report action from 4c54e2b

@katspaugh katspaugh changed the title Feat: check page hostname to toggle trademark elements Feat: white label branding + domain checks Dec 3, 2024
Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Code review by ChatGPT

@@ -23,6 +23,9 @@ Here's the list of all the environment variables:

| Env variable | Description |
| ------------------------------------------------- | ----------------------------------------------------------------------------------------------------------------------------------------------------------------------- |
| `NEXT_PUBLIC_BRAND_NAME` | The name of the app, defaults to "Wallet fork" |
| `NEXT_PUBLIC_BRAND_LOGO` | The URL of the app logo displayed in the header |

| `NEXT_PUBLIC_INFURA_TOKEN` | [Infura](https://docs.infura.io/infura/networks/ethereum/how-to/secure-a-project/project-id) RPC API token |
| `NEXT_PUBLIC_SAFE_APPS_INFURA_TOKEN` | Infura token for Safe Apps, falls back to `NEXT_PUBLIC_INFURA_TOKEN` |
| `NEXT_PUBLIC_IS_PRODUCTION` | Set to `true` to build a minified production app |
Copy link

Choose a reason for hiding this comment

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

No significant issues found in the added environment variables. Ensure that the naming convention aligns with existing variables and verify that the new variables are correctly utilized in the application to avoid future integration problems.

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Code review by ChatGPT

@@ -31,3 +31,5 @@ jobs:
package-manager: yarn
test-script: yarn test:ci
github-token: ${{ secrets.GITHUB_TOKEN }}
env:
NEXT_PUBLIC_IS_OFFICIAL_HOST: true
Copy link

Choose a reason for hiding this comment

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

  1. Environment Variable Scope: Ensure that NEXT_PUBLIC_IS_OFFICIAL_HOST is necessary as a public environment variable. Public environment variables expose their values to the client-side JavaScript. If the variable contains sensitive information, consider an alternative solution.

  2. Hardcoded Values: The value true for NEXT_PUBLIC_IS_OFFICIAL_HOST is hardcoded. If this value needs to be dynamic or might change based on the environment, consider refactoring to extract this as a configurable parameter. Use environment-specific configuration or secrets management where applicable.

@katspaugh katspaugh marked this pull request as draft December 9, 2024 11:28
@katspaugh katspaugh marked this pull request as ready for review December 9, 2024 14:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Ready for QA
Development

Successfully merging this pull request may close these issues.

1 participant