Add maintenance mode through Amplitude#2666
Conversation
…x/adjust-unfunded-acct-errors
…r/freighter into feature/add-maintenance-mode
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
|
All alerts resolved. Learn more about Socket for GitHub. This PR previously contained dependency changes with security issues that have been resolved, removed, or ignored. |
There was a problem hiding this comment.
Pull request overview
Implements “maintenance mode” in the extension popup driven by Amplitude Experiment feature flags, enabling both a non-blocking banner and a full-screen blocking maintenance screen (plus supporting debug/testing tooling).
Changes:
- Add Amplitude Experiment client initialization + Redux “remoteConfig” duck to fetch and expose maintenance flags/content.
- Introduce
MaintenanceBanner(Account header) andMaintenanceScreen(app-wide gate) with localized payload parsing. - Expand Debug tooling and add unit + Playwright E2E tests (including test-only Redux store injection).
Reviewed changes
Copilot reviewed 35 out of 36 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| yarn.lock | Adds Amplitude Experiment dependencies and transitive packages. |
| extension/package.json | Adds @amplitude/experiment-js-client dependency. |
| extension/webpack.extension.js | Adds Experiment deployment key env wiring; defaults env param. |
| extension/webpack.dev.js | Adds Experiment deployment key to dev env merge. |
| extension/webpack.common.js | Defines AMPLITUDE_EXPERIMENT_DEPLOYMENT_KEY at build time. |
| extension/.env.example | Documents new Experiment deployment key env var. |
| extension/src/constants/env.ts | Exposes AMPLITUDE_EXPERIMENT_DEPLOYMENT_KEY constant. |
| extension/src/helpers/experimentClient.ts | Adds Experiment client init/get helper. |
| extension/src/helpers/metrics.ts | Initializes Experiment client after analytics init; refactors constants. |
| extension/src/popup/ducks/remoteConfig.ts | Adds remote config slice + thunk + maintenance selectors. |
| extension/src/popup/ducks/tests/remoteConfig.test.ts | Unit tests for remoteConfig thunk/selectors. |
| extension/src/popup/helpers/hooks/useRemoteConfig.ts | Hook to fetch flags once per popup open. |
| extension/src/popup/helpers/maintenance/types.ts | Defines maintenance payload/content types and banner themes. |
| extension/src/popup/helpers/maintenance/parseMaintenanceContent.ts | Parses and localizes maintenance payloads safely. |
| extension/src/popup/helpers/maintenance/tests/parseMaintenanceContent.test.ts | Unit tests for maintenance payload parsing. |
| extension/src/popup/components/MaintenanceBanner/index.tsx | Implements maintenance banner + optional modal interaction. |
| extension/src/popup/components/MaintenanceBanner/styles.scss | Styles for banner strip and modal content. |
| extension/src/popup/components/MaintenanceScreen/index.tsx | Implements full-screen maintenance overlay. |
| extension/src/popup/components/MaintenanceScreen/styles.scss | Styles for the full-screen overlay. |
| extension/src/popup/components/SlideupModal/index.tsx | Adds optional full-screen backdrop support. |
| extension/src/popup/components/SlideupModal/styles.scss | Uses visibility toggling to improve Playwright visibility assertions. |
| extension/src/popup/components/account/AccountHeader/index.tsx | Renders MaintenanceBanner as header top content. |
| extension/src/popup/basics/layout/View/index.tsx | Adds topContent slot to View.AppHeader. |
| extension/src/popup/basics/layout/View/styles.scss | Adds styling hook for View__topContent. |
| extension/src/popup/App.tsx | Adds MaintenanceGate and dev-only window.__store exposure for E2E. |
| extension/src/popup/testHelpers/index.tsx | Registers remoteConfig reducer in dummy store. |
| extension/src/popup/views/Debug.tsx | Adds feature flag debug section + layout changes. |
| extension/src/popup/views/Debug/styles.scss | Adds side-by-side layout and increases max width. |
| extension/src/popup/styles/global.scss | Adds z-index CSS var for maintenance overlay. |
| extension/src/popup/components/tests/maintenanceMode.test.tsx | Unit tests for banner + screen components. |
| extension/src/popup/locales/en/translation.json | Adds new strings for flags/debug + maintenance banner. |
| extension/src/popup/locales/pt/translation.json | Adds Portuguese translations for new strings. |
| extension/e2e-tests/helpers/stubs.ts | Adds helpers to inject maintenance state via window.__store. |
| extension/e2e-tests/maintenanceMode.test.ts | Adds Playwright E2E coverage for banner/screen behavior. |
| eslint.config.js | Updates TypeScript import resolver to include extension tsconfig. |
| config/jest/setupTests.tsx | Adds globals for env constants used in tests and formats mocked middleware. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| onKeyDown={ | ||
| isClickable | ||
| ? (e) => { | ||
| if (e.key === "Enter" || e.key === " ") handleClick(); |
There was a problem hiding this comment.
The banner strip is rendered as a <div> with role="button", but the keyboard handler doesn’t preventDefault() for the Space key. On Space, browsers typically scroll the page; for custom buttons you generally need to prevent default to match native button behavior. Consider using a real <button type="button"> for the clickable banner (or at least call e.preventDefault() when e.key === " ") to improve accessibility and keyboard parity.
| if (e.key === "Enter" || e.key === " ") handleClick(); | |
| if (e.key === "Enter") { | |
| handleClick(); | |
| } else if (e.key === " ") { | |
| e.preventDefault(); | |
| handleClick(); | |
| } |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 35 out of 36 changed files in this pull request and generated 7 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| await expect(page.getByTestId("maintenance-banner")).toBeVisible(); | ||
| await expect( | ||
| page | ||
| .getByTestId("maintenance-banner") | ||
| .locator(".MaintenanceBanner__alert--warning"), | ||
| ).toBeVisible(); | ||
| }); |
There was a problem hiding this comment.
This locator is scoped to the maintenance-banner element and then searches for a descendant with .MaintenanceBanner__alert--warning, but that class is on the banner element itself (not a child). locator() won’t match the scope element, so this assertion will fail. Assert the class directly on the banner element (e.g. via toHaveClass) or use a :scope selector.
|
@leofelix077 the font size here seems a bit bigger than what we have on Figma. (left is Figma - right is PR) Is this intended? |
| * String flags — variant value is used directly. | ||
| * Empty for now; add flag names here as new string flags are introduced. | ||
| */ | ||
| const STRING_FLAGS = [] as const; |
There was a problem hiding this comment.
on mobile we don't have STRING_FLAGS, we actually have VERSION_FLAGS which receive a special treatment where we parse 1_2_3 => 1.2.3
There was a problem hiding this comment.
I'd suggest updating it to VERSION_FLAGSn + special parsing so we don't need to worry about it whenever we need to use a version flag with extension so we keep same behavior on both platforms for consistency
| return { ...initialState, isInitialized: true }; | ||
| } | ||
|
|
||
| await client.fetch(); |
There was a problem hiding this comment.
I think we need to fetch the flags passing user_properties + bundleId to rest assure it'll fetch the flags when users have opt out of analytics, the same we do on mobile:
await experimentClient.fetch({
user_properties: {
// We need to explicitly pass the "Bundle Id" here to make sure the
// correct flag values will be assigned regardless if (iOS) users
// have ever enabled tracking permission or not.
// Other common properties like "Platform" and "Version" appear to be
// always forwarded regardless of tracking permission.
[ANALYTICS_CONFIG.BUNDLE_ID_KEY]: getBundleId(),
},
});
There was a problem hiding this comment.
could you please apply this change and test what happens when a user opt-out of analytics? the analytics metrics should stop being sent to Amplitude but we should still be able to fetch the feature flags with no issues
| # To test Amplitude locally, add the line below with your Amplitude key, then run: yarn start | ||
| # | ||
| # AMPLITUDE_KEY= | ||
| # AMPLITUDE_EXPERIMENT_DEPLOYMENT_KEY= |
There was a problem hiding this comment.
I think we need to polish the comments on lines 4-5 a little bit to account for this new variable
| function resolveLocalizedArray(map: LocalizedStringArray): string[] { | ||
| const lang = getLang(); | ||
| return map[lang] ?? map.en ?? []; | ||
| } |
There was a problem hiding this comment.
on mobile we do some filtering here to take only the string values from the array. E.g.: if the Amplitude payload contains ["text", 42, null], mobile filters to ["text"] but extension would pass all values through.
I think extension should add the same .filter() guard for consistency.
This is there I saw it on mobile:
const getLocalizedStringArray = (
obj: LocalizedStringArray | undefined,
fallback: string[],
): string[] => {
if (!obj) return fallback;
const lang = getDeviceLanguage();
const value = obj[lang] ?? obj.en;
if (!Array.isArray(value)) return fallback;
return value.filter(
(v): v is string => typeof v === "string" && v.trim().length > 0,
); // ========================> filter out non-string values
};
|
@CassioMG finished adjusting the UI + open comments from review
|
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 37 out of 38 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // the user has opted out of analytics (mirrors mobile implementation). | ||
| await client.fetch({ | ||
| user_properties: { | ||
| "Bundle Id": `extension.${BUILD_TYPE}`, |
There was a problem hiding this comment.
could we extract "Bundle Id" to a const and extension.${BUILD_TYPE} to a getBundleId helper? I think we use it in multiple places now
There was a problem hiding this comment.
oh yes. done 👌
…llar/freighter into feature/add-maintenance-mode
…ature/add-maintenance-mode





Closes #2531