[LWD] feat(desktop): upgrade analytics consent QA screen - #20266
Conversation
Web Tools Build Status
|
There was a problem hiding this comment.
Pull request overview
Upgrades the Ledger Live Desktop analytics consent QA/debug screen to align with the shared @features/flow-analytics-consent/debug scenarios and inspection helpers, matching the mobile QA tool behavior while keeping the Lumen Web UI.
Changes:
- Replaces the legacy desktop QA screen logic with a dedicated MVVM-style view-model backed by shared consent-debug helpers (scenarios, verdicts, baselines, reasons).
- Reworks the desktop QA screen UI to add scenario presets, an Inspect tab (stored state + remote config + user prefs), feature-flag override, and reset.
- Removes the now-unused
analyticsConsentOptInQai18n keys from all desktop locales and drops deprecated QA-only settings actions.
Reviewed changes
Copilot reviewed 17 out of 17 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| apps/ledger-live-desktop/static/i18n/zh/app.json | Removes analyticsConsentOptInQa locale strings (hardcoded English copy now used). |
| apps/ledger-live-desktop/static/i18n/tr/app.json | Removes analyticsConsentOptInQa locale strings. |
| apps/ledger-live-desktop/static/i18n/th/app.json | Removes analyticsConsentOptInQa locale strings. |
| apps/ledger-live-desktop/static/i18n/ru/app.json | Removes analyticsConsentOptInQa locale strings. |
| apps/ledger-live-desktop/static/i18n/pt-BR/app.json | Removes analyticsConsentOptInQa locale strings. |
| apps/ledger-live-desktop/static/i18n/ko/app.json | Removes analyticsConsentOptInQa locale strings. |
| apps/ledger-live-desktop/static/i18n/ja/app.json | Removes analyticsConsentOptInQa locale strings. |
| apps/ledger-live-desktop/static/i18n/fr/app.json | Removes analyticsConsentOptInQa locale strings. |
| apps/ledger-live-desktop/static/i18n/es/app.json | Removes analyticsConsentOptInQa locale strings. |
| apps/ledger-live-desktop/static/i18n/en/app.json | Removes analyticsConsentOptInQa locale strings. |
| apps/ledger-live-desktop/static/i18n/de/app.json | Removes analyticsConsentOptInQa locale strings. |
| apps/ledger-live-desktop/src/renderer/screens/settings/sections/Developer/AnalyticsConsentOptInDevTool/useAnalyticsConsentOptInQaViewModel.ts | New shared-scenarios view-model (verdict/phase, baseline handling, overrides, reset, inspect fields). |
| apps/ledger-live-desktop/src/renderer/screens/settings/sections/Developer/AnalyticsConsentOptInDevTool/index.tsx | Updates the developer settings row to hardcoded copy and uses a Link to open the QA screen. |
| apps/ledger-live-desktop/src/renderer/screens/settings/sections/Developer/AnalyticsConsentOptInDevTool/AnalyticsConsentOptInDevScreen.tsx | Rebuilds the QA screen UI (scenario cards + inspect tab + reset + portfolio preview navigation). |
| apps/ledger-live-desktop/src/renderer/screens/settings/sections/Developer/AnalyticsConsentOptInDevTool/tests/useAnalyticsConsentOptInQaViewModel.test.ts | Adds view-model tests covering key scenario/reset/flag-off behaviors. |
| apps/ledger-live-desktop/src/renderer/actions/settings.ts | Removes deprecated QA-only analytics consent mutation helpers in favor of normal actions. |
| .changeset/calm-clouds-debug.md | Adds a patch changeset for the desktop package. |
Rsdoctor Bundle Diff Analysis
Found 7 projects in monorepo, 2 projects with changes. 📊 Quick Summary
📋 Detailed Reports (Click to expand)📁 desktop-rendererPath:
📦 Download Diff Report: desktop-renderer Bundle Diff 📁 mobilePath:
📦 Download Diff Report: mobile Bundle Diff Generated by Rsdoctor GitHub Action |
95dfa06 to
ccae87c
Compare
0c7a38e to
77b6694
Compare
❌ Action Required: Monitored Files ChangedThe following files in monitored folders have been modified:
Action Required: Please rebase your branch against git rebase origin/develop |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 17 out of 17 changed files in this pull request and generated no new comments.
Suppressed comments (10)
apps/ledger-live-desktop/static/i18n/de/app.json:5072
- Per repo translation workflow guidelines, avoid editing non-English locale files (only
apps/ledger-live-desktop/static/i18n/en/app.jsonshould be changed). Please revert this locale change and let the localization sync handle key removals.
"themeConsole" : {
"title" : "Designumschalter",
"desc" : "Zeigt ein schwebendes Overlay an, mit dem du schnell zwischen Designs umschalten kannst."
},
"devTools" : {
apps/ledger-live-desktop/static/i18n/es/app.json:5072
- Per repo translation workflow guidelines, avoid editing non-English locale files (only
apps/ledger-live-desktop/static/i18n/en/app.jsonshould be changed). Please revert this locale change and let the localization sync handle key removals.
"themeConsole" : {
"title" : "Conmutador de tema",
"desc" : "Muestra una superposición flotante con la que cambiar de tema rápidamente."
},
"devTools" : {
apps/ledger-live-desktop/static/i18n/ko/app.json:5072
- Per repo translation workflow guidelines, avoid editing non-English locale files (only
apps/ledger-live-desktop/static/i18n/en/app.jsonshould be changed). Please revert this locale change and let the localization sync handle key removals.
"themeConsole" : {
"title" : "테마 토글",
"desc" : "테마를 빠르게 전환할 수 있도록 플로팅 오버레이를 표시합니다."
},
"devTools" : {
apps/ledger-live-desktop/static/i18n/pt-BR/app.json:5072
- Per repo translation workflow guidelines, avoid editing non-English locale files (only
apps/ledger-live-desktop/static/i18n/en/app.jsonshould be changed). Please revert this locale change and let the localization sync handle key removals.
"themeConsole" : {
"title" : "Alternar tema",
"desc" : "Exibir botão flutuante para alternar temas rapidamente."
},
"devTools" : {
apps/ledger-live-desktop/static/i18n/th/app.json:5072
- Per repo translation workflow guidelines, avoid editing non-English locale files (only
apps/ledger-live-desktop/static/i18n/en/app.jsonshould be changed). Please revert this locale change and let the localization sync handle key removals.
"themeConsole" : {
"title" : "ปุ่มสลับธีม",
"desc" : "แสดงโอเวอร์เลย์แบบลอยตัวเพื่อเปลี่ยนธีมได้อย่างรวดเร็ว"
},
"devTools" : {
apps/ledger-live-desktop/static/i18n/fr/app.json:5072
- Per repo translation workflow guidelines, avoid editing non-English locale files (only
apps/ledger-live-desktop/static/i18n/en/app.jsonshould be changed). Please revert this locale change and let the localization sync handle key removals.
"themeConsole" : {
"title" : "Sélecteur de thème",
"desc" : "Affichez une fenêtre flottante pour changer de thème rapidement."
},
"devTools" : {
apps/ledger-live-desktop/static/i18n/ja/app.json:5072
- Per repo translation workflow guidelines, avoid editing non-English locale files (only
apps/ledger-live-desktop/static/i18n/en/app.jsonshould be changed). Please revert this locale change and let the localization sync handle key removals.
"themeConsole" : {
"title" : "テーマ切り替え",
"desc" : "テーマを素早く切り替えるためのフローティングオーバーレイを表示します。"
},
"devTools" : {
apps/ledger-live-desktop/static/i18n/ru/app.json:5072
- Per repo translation workflow guidelines, avoid editing non-English locale files (only
apps/ledger-live-desktop/static/i18n/en/app.jsonshould be changed). Please revert this locale change and let the localization sync handle key removals.
"themeConsole" : {
"title" : "Переключение темы",
"desc" : "Отображать плавающий тумблер для быстрого переключения тем."
},
"devTools" : {
apps/ledger-live-desktop/static/i18n/tr/app.json:5072
- Per repo translation workflow guidelines, avoid editing non-English locale files (only
apps/ledger-live-desktop/static/i18n/en/app.jsonshould be changed). Please revert this locale change and let the localization sync handle key removals.
"themeConsole" : {
"title" : "Tema Butonu",
"desc" : "Temalar arasında hızlıca geçiş yapmak için bir küçük pencere gösterin."
},
"devTools" : {
apps/ledger-live-desktop/static/i18n/zh/app.json:5072
- Per repo translation workflow guidelines, avoid editing non-English locale files (only
apps/ledger-live-desktop/static/i18n/en/app.jsonshould be changed). Please revert this locale change and let the localization sync handle key removals.
"themeConsole" : {
"title" : "主题切换",
"desc" : "显示可快速切换主题的浮动覆盖。"
},
"devTools" : {
ccae87c to
7821700
Compare
77b6694 to
1dcc85e
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 17 out of 17 changed files in this pull request and generated no new comments.
Suppressed comments (10)
apps/ledger-live-desktop/static/i18n/zh/app.json:5072
- This PR edits a non-English locale file. Repo review guidance says desktop translations should only be edited in
apps/ledger-live-desktop/static/i18n/en/app.json(see.agents/agents/code-reviewer.md:43), with other locales handled by the localization workflow. Please revert this change (restore the removed key) and keep the functional change in code/hardcoded strings.
"themeConsole" : {
"title" : "主题切换",
"desc" : "显示可快速切换主题的浮动覆盖。"
},
"devTools" : {
apps/ledger-live-desktop/static/i18n/tr/app.json:5072
- This PR edits a non-English locale file. Repo review guidance says desktop translations should only be edited in
apps/ledger-live-desktop/static/i18n/en/app.json(see.agents/agents/code-reviewer.md:43), with other locales handled by the localization workflow. Please revert this change (restore the removed key) and keep the functional change in code/hardcoded strings.
"themeConsole" : {
"title" : "Tema Butonu",
"desc" : "Temalar arasında hızlıca geçiş yapmak için bir küçük pencere gösterin."
},
"devTools" : {
apps/ledger-live-desktop/static/i18n/th/app.json:5072
- This PR edits a non-English locale file. Repo review guidance says desktop translations should only be edited in
apps/ledger-live-desktop/static/i18n/en/app.json(see.agents/agents/code-reviewer.md:43), with other locales handled by the localization workflow. Please revert this change (restore the removed key) and keep the functional change in code/hardcoded strings.
"themeConsole" : {
"title" : "ปุ่มสลับธีม",
"desc" : "แสดงโอเวอร์เลย์แบบลอยตัวเพื่อเปลี่ยนธีมได้อย่างรวดเร็ว"
},
"devTools" : {
apps/ledger-live-desktop/static/i18n/ru/app.json:5072
- This PR edits a non-English locale file. Repo review guidance says desktop translations should only be edited in
apps/ledger-live-desktop/static/i18n/en/app.json(see.agents/agents/code-reviewer.md:43), with other locales handled by the localization workflow. Please revert this change (restore the removed key) and keep the functional change in code/hardcoded strings.
"themeConsole" : {
"title" : "Переключение темы",
"desc" : "Отображать плавающий тумблер для быстрого переключения тем."
},
"devTools" : {
apps/ledger-live-desktop/static/i18n/pt-BR/app.json:5072
- This PR edits a non-English locale file. Repo review guidance says desktop translations should only be edited in
apps/ledger-live-desktop/static/i18n/en/app.json(see.agents/agents/code-reviewer.md:43), with other locales handled by the localization workflow. Please revert this change (restore the removed key) and keep the functional change in code/hardcoded strings.
"themeConsole" : {
"title" : "Alternar tema",
"desc" : "Exibir botão flutuante para alternar temas rapidamente."
},
"devTools" : {
apps/ledger-live-desktop/static/i18n/ko/app.json:5072
- This PR edits a non-English locale file. Repo review guidance says desktop translations should only be edited in
apps/ledger-live-desktop/static/i18n/en/app.json(see.agents/agents/code-reviewer.md:43), with other locales handled by the localization workflow. Please revert this change (restore the removed key) and keep the functional change in code/hardcoded strings.
"themeConsole" : {
"title" : "테마 토글",
"desc" : "테마를 빠르게 전환할 수 있도록 플로팅 오버레이를 표시합니다."
},
"devTools" : {
apps/ledger-live-desktop/static/i18n/ja/app.json:5072
- This PR edits a non-English locale file. Repo review guidance says desktop translations should only be edited in
apps/ledger-live-desktop/static/i18n/en/app.json(see.agents/agents/code-reviewer.md:43), with other locales handled by the localization workflow. Please revert this change (restore the removed key) and keep the functional change in code/hardcoded strings.
"themeConsole" : {
"title" : "テーマ切り替え",
"desc" : "テーマを素早く切り替えるためのフローティングオーバーレイを表示します。"
},
"devTools" : {
apps/ledger-live-desktop/static/i18n/fr/app.json:5072
- This PR edits a non-English locale file. Repo review guidance says desktop translations should only be edited in
apps/ledger-live-desktop/static/i18n/en/app.json(see.agents/agents/code-reviewer.md:43), with other locales handled by the localization workflow. Please revert this change (restore the removed key) and keep the functional change in code/hardcoded strings.
"themeConsole" : {
"title" : "Sélecteur de thème",
"desc" : "Affichez une fenêtre flottante pour changer de thème rapidement."
},
"devTools" : {
apps/ledger-live-desktop/static/i18n/es/app.json:5072
- This PR edits a non-English locale file. Repo review guidance says desktop translations should only be edited in
apps/ledger-live-desktop/static/i18n/en/app.json(see.agents/agents/code-reviewer.md:43), with other locales handled by the localization workflow. Please revert this change (restore the removed key) and keep the functional change in code/hardcoded strings.
"themeConsole" : {
"title" : "Conmutador de tema",
"desc" : "Muestra una superposición flotante con la que cambiar de tema rápidamente."
},
"devTools" : {
apps/ledger-live-desktop/static/i18n/de/app.json:5072
- This PR edits a non-English locale file. Repo review guidance says desktop translations should only be edited in
apps/ledger-live-desktop/static/i18n/en/app.json(see.agents/agents/code-reviewer.md:43), with other locales handled by the localization workflow. Please revert this change (restore the removed key) and keep the functional change in code/hardcoded strings.
"themeConsole" : {
"title" : "Designumschalter",
"desc" : "Zeigt ein schwebendes Overlay an, mit dem du schnell zwischen Designs umschalten kannst."
},
"devTools" : {
1dcc85e to
9058265
Compare
7821700 to
ba6c8ba
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 17 out of 17 changed files in this pull request and generated no new comments.
Suppressed comments (4)
apps/ledger-live-desktop/src/renderer/screens/settings/sections/Developer/AnalyticsConsentOptInDevTool/useAnalyticsConsentOptInQaViewModel.ts:94
formatBareValuecurrently returns the string "null" for bothnullandundefined, which makes the inspector output ambiguous (and can hide when a remote param is actually missing). Consider renderingundefineddistinctly and falling back whenJSON.stringifyreturnsundefined.
export function formatBareValue(value: unknown): string {
if (value === null || value === undefined) return "null";
return JSON.stringify(value);
}
apps/ledger-live-desktop/src/renderer/screens/settings/sections/Developer/AnalyticsConsentOptInDevTool/AnalyticsConsentOptInDevScreen.tsx:25
TONE_TEXTis typed asRecord<string, string>, which weakens type-safety (any key becomes allowed). Since the keys are meant to match the existing tone union, tighten this toRecord<InspectorField["status"]["tone"], string>so missing/extra tones are caught by TypeScript.
const TONE_TEXT: Record<string, string> = {
error: "text-error",
warning: "text-warning",
success: "text-success",
gray: "text-muted",
};
apps/ledger-live-desktop/src/renderer/screens/settings/sections/Developer/AnalyticsConsentOptInDevTool/useAnalyticsConsentOptInQaViewModel.ts:4
- New Desktop feature code is being added under
src/renderer/...(e.g. this new ViewModel hook). Our MVVM architecture guidance states that new code forledger-live-desktopshould go undersrc/mvvm/(see.agents/skills/mvvm-architecture/SKILL.md:10-13). Consider moving this QA tool to an MVVM feature folder (or documenting why this is an exception).
This issue also appears on line 91 of the same file.
import { useEffect, useState } from "react";
import { useNavigate } from "react-router";
import { useFeature } from "@features/platform-feature-flags";
import { featureFlagsOverridesSelector, setOverride } from "@shared/feature-flags";
apps/ledger-live-desktop/src/renderer/screens/settings/sections/Developer/AnalyticsConsentOptInDevTool/index.tsx:3
- This row switches to
Linkfromreact-router, but internal navigation in Settings developer rows is typically handled viauseNavigate/onClick(and there are no otherLinkimports fromreact-routerin the desktop app). To keep navigation patterns consistent (and avoid coupling this row to a specific router component API), consider reverting to anonClick+navigatehandler.
import React from "react";
import { Link } from "react-router";
import { Button } from "@ledgerhq/lumen-ui-react";
|
Closing — desktop/prune work deferred; stack ends at #20212 for now. |
8185884 to
0ca37d4
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 19 out of 20 changed files in this pull request and generated no new comments.
Files not reviewed (1)
- pnpm-lock.yaml: Generated file
Suppressed comments (3)
apps/ledger-live-desktop/src/renderer/screens/settings/sections/Developer/AnalyticsConsentOptInDevTool/useAnalyticsConsentOptInQaViewModel.ts:95
formatBareValuecurrently returns "null" for bothnullandundefined, and can also returnundefinedwhenJSON.stringifyyieldsundefined(e.g. functions). This makes the inspector output ambiguous and breaks the function’s declaredstringreturn type.
export function formatBareValue(value: unknown): string {
if (value === null || value === undefined) return "null";
return JSON.stringify(value);
}
apps/ledger-live-desktop/src/renderer/screens/settings/sections/Developer/AnalyticsConsentOptInDevTool/useAnalyticsConsentOptInQaViewModel.ts:260
consentInfo.consentDateis checked using a truthy condition, so an empty string (""), which is a plausible invalid stored value, would be displayed asconsentDate: nullin the raw inspector output. Using an explicit null check preserves the actual stored value for debugging.
raw: consentInfo.consentDate
? `consentDate: ${formatBareValue(consentInfo.consentDate)}`
: "consentDate: null",
status:
apps/ledger-live-desktop/src/renderer/screens/settings/sections/Developer/AnalyticsConsentOptInDevTool/AnalyticsConsentOptInDevScreen.tsx:26
TONE_TEXTis typed asRecord<string, string>, which makes it easy to accidentally use unsupported tones without a type error. Since this mapping is meant to coverFieldTone, type it accordingly so missing/extra keys are caught at compile time.
const TONE_TEXT: Record<string, string> = {
error: "text-error",
warning: "text-warning",
success: "text-success",
gray: "text-muted",
};
9d4e5e4 to
f2da8aa
Compare
0ca37d4 to
4bf3d6d
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 19 out of 19 changed files in this pull request and generated no new comments.
Suppressed comments (2)
apps/ledger-live-desktop/src/renderer/screens/settings/sections/Developer/AnalyticsConsentOptInDevTool/index.tsx:14
- This row now hardcodes title/description/button label, which regresses i18n and is inconsistent with other Developer settings rows that use
react-i18next(e.g.OpenDevToolsRow.tsx:16-22usest("settings.developer.*")). Consider restoringuseTranslation()+ translation keys (and keeping the per-locale entries) so the Settings UI remains localized.
<SettingsSectionRow
title="Analytics consent QA"
desc="Policy bumps, consent state, drawer preview"
>
<Button asChild size="sm" appearance="accent">
<Link to="/settings/developer/analytics-consent-opt-in-qa">Open</Link>
</Button>
apps/ledger-live-desktop/src/renderer/screens/settings/sections/Developer/AnalyticsConsentOptInDevTool/useAnalyticsConsentOptInQaViewModel.ts:95
formatBareValuecurrently prints"null"for bothnullandundefined, which makes the inspector'srawvalues ambiguous (missing param vs explicitly null). Showing"undefined"forundefinedwould make QA/debug output clearer and avoid misleading reads.
export function formatBareValue(value: unknown): string {
if (value === null || value === undefined) return "null";
return JSON.stringify(value);
}
f2da8aa to
a009c3e
Compare
4bf3d6d to
a71c587
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 19 out of 19 changed files in this pull request and generated no new comments.
Suppressed comments (5)
apps/ledger-live-desktop/src/renderer/screens/settings/sections/Developer/AnalyticsConsentOptInDevTool/AnalyticsConsentOptInDevScreen.tsx:100
- This screen introduces several user-visible strings as hardcoded English (e.g., header/title/section labels). Since the PR also removes the existing i18n keys, this QA screen will no longer be localized in non-English locales. Even for developer tools, keeping strings behind i18n keys avoids regressions when running the app in other locales.
<div className="flex min-w-0 justify-start">
<Button size="sm" appearance="no-background" onClick={vm.onBack} icon={ArrowLeft}>
Back
</Button>
</div>
<span className="heading-2-semi-bold max-w-[min(100vw-8rem,28rem)] text-center text-base">
Analytics consent QA
</span>
<div aria-hidden className="min-w-0" />
apps/ledger-live-desktop/src/renderer/screens/settings/sections/Developer/AnalyticsConsentOptInDevTool/useAnalyticsConsentOptInQaViewModel.ts:95
formatBareValuereturns the string "null" for bothnullandundefined, which makes the inspector output misleading (e.g., an absentpolicyVersionwill render asnull). Distinguishingundefinedhelps QA interpret whether a value is missing vs explicitly cleared.
export function formatBareValue(value: unknown): string {
if (value === null || value === undefined) return "null";
return JSON.stringify(value);
}
apps/ledger-live-desktop/src/renderer/screens/settings/sections/Developer/AnalyticsConsentOptInDevTool/AnalyticsConsentOptInDevScreen.tsx:26
TONE_TEXTis typed asRecord<string, string>, which hides missing/extra tone keys and forces a runtime fallback later. Since the tones are a closed set, it’s safer to type this map against the ViewModel’sheadlineToneunion so TS enforces completeness.
This issue also appears on line 92 of the same file.
const TONE_TEXT: Record<string, string> = {
error: "text-error",
warning: "text-warning",
success: "text-success",
gray: "text-muted",
};
apps/ledger-live-desktop/src/mvvm/features/AnalyticsConsentDialog/hooks/useAnalyticsConsentDialogViewModel.ts:96
- PR description mentions previewing the dialog “via
forceOffer”, but there’s noforceOffermechanism in the codebase (and the implementation here instead removes the route-focus guard entirely). If the intent is to keep the dialog restricted to Portfolio in production, consider adding an explicit override (e.g., aforceOfferparam/prop used only by the QA screen) rather than broadening the default behavior for all mounts.
useEffect(() => {
if (!shouldOffer) {
setPhase(current => {
if (current !== "closed") {
track("drawer_closed", dialogClosedPayload);
}
return "closed";
});
return;
}
setPhase(current => {
if (current === "preferences") return current;
return resolveAnalyticsConsentPhase(current, decision, shareAnalytics);
});
}, [shouldOffer, decision, shareAnalytics]);
apps/ledger-live-desktop/src/renderer/screens/settings/sections/Developer/AnalyticsConsentOptInDevTool/index.tsx:14
- This dev-tool row is now hardcoded in English (and the corresponding i18n keys were removed). That makes the Settings → Developer menu inconsistent with the rest of the app’s i18n and removes previously available translations for non-English locales. Consider restoring
react-i18nextusage here (and reintroducing the removedsettings.developer.analyticsConsentOptInQa.*keys, or replacing them with new keys) so the title/description and button label remain localized.
<SettingsSectionRow
title="Analytics consent QA"
desc="Policy bumps, consent state, drawer preview"
>
<Button asChild size="sm" appearance="accent">
<Link to="/settings/developer/analytics-consent-opt-in-qa">Open</Link>
</Button>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 19 out of 19 changed files in this pull request and generated no new comments.
Suppressed comments (1)
apps/ledger-live-desktop/src/renderer/screens/settings/sections/Developer/AnalyticsConsentOptInDevTool/useAnalyticsConsentOptInQaViewModel.ts:259
rawuses a truthy check onconsentInfo.consentDate, so an empty string (invalid but possible persisted value) would be displayed asconsentDate: null, losing debugging signal. Use an explicitnullcheck so invalid strings remain visible.
enabled,
params: feature?.params ?? {},
},
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 19 out of 19 changed files in this pull request and generated no new comments.
Suppressed comments (2)
apps/ledger-live-desktop/static/i18n/zh/app.json:5075
- Repo translation guideline: desktop PRs should only edit
apps/ledger-live-desktop/static/i18n/en/app.json(other locales are managed separately). This PR removessettings.developer.analyticsConsentOptInQafrom multiple non-en locale files (including this one), which should be reverted; keep the change inen/app.jsononly (or keep the key in code).
"themeConsole" : {
"title" : "主题切换",
"desc" : "显示可快速切换主题的浮动覆盖。"
},
"devTools" : {
"rowTitle" : "开发工具",
"rowDesc" : "打开 DevTools 开发工具界面(功能标志和开发者工具面板)。"
},
apps/ledger-live-desktop/src/renderer/screens/settings/sections/Developer/AnalyticsConsentOptInDevTool/useAnalyticsConsentOptInQaViewModel.ts:5
- New Desktop feature/UI logic is being introduced under
src/renderer/screens/...(this new ViewModel file). Per the repo MVVM standard, new screens/components/hooks for ledger-live-desktop should be added underapps/ledger-live-desktop/src/mvvm/(FeatureName/{screens,hooks,components,utils}) and the view should receive data via props from a ViewModel hook.
import { useEffect, useState } from "react";
import { useNavigate } from "react-router";
import { useFeature } from "@features/platform-feature-flags";
import { featureFlagsOverridesSelector, setOverride } from "@shared/feature-flags";
import {
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 19 out of 19 changed files in this pull request and generated no new comments.
Suppressed comments (4)
apps/ledger-live-desktop/src/renderer/screens/settings/sections/Developer/AnalyticsConsentOptInDevTool/index.tsx:14
- This Settings row was switched from i18n keys to hard-coded English strings, and the related translation entries were removed from multiple locales. That makes the Developer settings screen inconsistent with the rest of Settings rows (which use react-i18next) and regresses localization for non-English users.
<SettingsSectionRow
title="Analytics consent QA"
desc="Policy bumps, consent state, drawer preview"
>
<Button asChild size="sm" appearance="accent">
<Link to="/settings/developer/analytics-consent-opt-in-qa">Open</Link>
</Button>
apps/ledger-live-desktop/src/renderer/screens/settings/sections/Developer/AnalyticsConsentOptInDevTool/useAnalyticsConsentOptInQaViewModel.ts:182
- This PR introduces a new ViewModel hook under
src/renderer/screens/...rather than thesrc/mvvm/structure used for new desktop feature code. If we want to follow the MVVM migration rules, this logic should live undersrc/mvvm/features/...with the corresponding View component consuming it via props.
export function useAnalyticsConsentOptInQaViewModel() {
const navigate = useNavigate();
const dispatch = useDispatch();
const feature = useFeature(FLAG_KEY);
const rawPolicyVersion = feature?.params?.policyVersion;
apps/ledger-live-desktop/src/renderer/screens/settings/sections/Developer/AnalyticsConsentOptInDevTool/AnalyticsConsentOptInDevScreen.tsx:91
- PR description mentions preview-in-place via a
forceOffermechanism, but the QA screen preview currently just mounts<AnalyticsConsentDialog />normally. Since the dialog still relies onshouldOffer, this is not a forced preview API. Either update the PR description to match the implementation, or implement an explicit override prop/API if forced preview is required.
<Box grow shrink className="p-8 pb-16">
{vm.isPreviewMounted ? <AnalyticsConsentDialog key={vm.previewKey} /> : null}
<header className="mb-14 grid grid-cols-[1fr_auto_1fr] items-center gap-x-3 py-6">
apps/ledger-live-desktop/src/renderer/screens/settings/sections/Developer/AnalyticsConsentOptInDevTool/AnalyticsConsentOptInDevScreen.tsx:152
SegmentedControl'sonSelectedChangecallback provides a string, but passing the React state setter directly weakens type-safety (it can accept values outside the intended "scenarios" | "inspect" union). Wrap it with a narrow cast handler (similar to other usages like MarketCategoryBar).
<SegmentedControl
selectedValue={vm.tab}
onSelectedChange={vm.setTab}
tabLayout="fit"
aria-label="Analytics consent QA sections"
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 19 out of 19 changed files in this pull request and generated no new comments.
Suppressed comments (2)
apps/ledger-live-desktop/src/renderer/screens/settings/sections/Developer/AnalyticsConsentOptInDevTool/index.tsx:14
- This row switches from the established pattern (Button + useNavigate) to
Linkimported fromreact-router. There are no other occurrences ofimport { Link } from "react-router"in the desktop codebase, while other QA rows (e.g.LargeScreenUpsellDevTool/index.tsx:12-23) useuseNavigate. To avoid a potentially invalid import and keep navigation consistent, useuseNavigatehere as well.
import React from "react";
import { Link } from "react-router";
import { Button } from "@ledgerhq/lumen-ui-react";
import { SettingsSectionRow } from "~/renderer/screens/settings/SettingsSection";
export default function AnalyticsConsentOptInDevTool() {
return (
<SettingsSectionRow
title="Analytics consent QA"
desc="Policy bumps, consent state, drawer preview"
>
<Button asChild size="sm" appearance="accent">
<Link to="/settings/developer/analytics-consent-opt-in-qa">Open</Link>
</Button>
apps/ledger-live-desktop/src/mvvm/features/AnalyticsConsentDialog/hooks/useAnalyticsConsentDialogViewModel.ts:96
- Removing the route-focus guard means
drawer_closedis no longer tracked when the dialog is implicitly dismissed by navigation/unmount while still open (previously covered by the deleted tests). If this event is still expected for analytics completeness, consider adding an unmount/route-change close path (or cleanup) that emitsdrawer_closedwhenphase !== "closed".
useEffect(() => {
if (!shouldOffer) {
setPhase(current => {
if (current !== "closed") {
track("drawer_closed", dialogClosedPayload);
}
return "closed";
});
return;
}
setPhase(current => {
if (current === "preferences") return current;
return resolveAnalyticsConsentPhase(current, decision, shareAnalytics);
});
}, [shouldOffer, decision, shareAnalytics]);
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 9 out of 9 changed files in this pull request and generated 1 comment.
Suppressed comments (3)
apps/ledger-live-desktop/static/i18n/en/app.json:5042
- The
analyticsConsentOptInQatranslation block was removed from the English source file, but it still exists in other locale JSONs (e.g.static/i18n/fr/app.json). If the intent is to remove this row’s translation keys, consider removing the same block from the other locales too to avoid keeping stale/unused translations.
"themeConsole": {
"title": "Theme Toggle",
"desc": "Display a floating overlay to quickly switch themes."
},
"devTools": {
"rowTitle": "DevTools",
"rowDesc": "Open the DevTools shell (feature flags and dev tools panel)."
},
apps/ledger-live-desktop/src/renderer/screens/settings/sections/Developer/AnalyticsConsentOptInDevTool/index.tsx:14
- This row was switched from i18n keys to hardcoded English strings. In the Developer settings section, other rows consistently use
react-i18nextkeys (e.g.apps/ledger-live-desktop/src/renderer/screens/settings/sections/Developer/index.tsx:57+), so this change breaks localization consistency and makes the QA row non-translatable.
<SettingsSectionRow
title="Analytics consent QA"
desc="Policy bumps, consent state, drawer preview"
>
<Button asChild size="sm" appearance="accent">
<Link to="/settings/developer/analytics-consent-opt-in-qa">Open</Link>
</Button>
apps/ledger-live-desktop/src/mvvm/features/AnalyticsConsentDialog/hooks/useAnalyticsConsentDialogViewModel.ts:96
- With the portfolio-route focus check removed,
drawer_closedis only tracked when the view model explicitly transitions toclosed. If the dialog unmounts while open (navigation, or QA preview being unmounted), the close event won’t be tracked. Adding an unmount cleanup to emitdrawer_closedwhenphase !== "closed"preserves the previous behavior without reintroducing route coupling.
useEffect(() => {
if (!shouldOffer) {
setPhase(current => {
if (current !== "closed") {
track("drawer_closed", dialogClosedPayload);
}
return "closed";
});
return;
}
setPhase(current => {
if (current === "preferences") return current;
return resolveAnalyticsConsentPhase(current, decision, shareAnalytics);
});
}, [shouldOffer, decision, shareAnalytics]);
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 9 out of 9 changed files in this pull request and generated no new comments.
Suppressed comments (2)
apps/ledger-live-desktop/src/renderer/screens/settings/sections/Developer/AnalyticsConsentOptInDevTool/useAnalyticsConsentOptInQaViewModel.ts:162
- This uses a truthiness check for
consentInfo.consentDate, so an empty string (or any other falsy string) would be displayed asconsentDate: null, hiding the real raw value. Since this is an inspector, it should distinguishnullfrom other invalid string values.
raw: consentInfo.consentDate
? `consentDate: ${formatBareValue(consentInfo.consentDate)}`
: "consentDate: null",
status: consentDateStatus,
apps/ledger-live-desktop/src/renderer/screens/settings/sections/Developer/AnalyticsConsentOptInDevTool/useAnalyticsConsentOptInQaViewModel.ts:98
formatBareValuecurrently returns "null" for bothnullandundefined, which makes the inspector raw values misleading (e.g. an actually-undefined remotepolicyVersionwill be shown asnull).
This issue also appears on line 159 of the same file.
export function formatBareValue(value: unknown): string {
if (value === null || value === undefined) return "null";
return JSON.stringify(value);
}
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 14 out of 14 changed files in this pull request and generated no new comments.
Suppressed comments (2)
apps/ledger-live-desktop/src/renderer/actions/settings.ts:76
setAnalyticsConsentInfonow acceptsPartial<AnalyticsConsentInfo>, but the reducer merges the payload with a spread ({ ...state.analyticsConsentInfo, ...payload }). WithexactOptionalPropertyTypesnot enabled for the desktop app, callers can accidentally passundefinedfields (e.g.{ consentDate: undefined }), which would overwrite previously valid values and break invariants (these fields are expected to bestring | null). Since all current call sites provide both fields, consider keeping this action helper strict (AnalyticsConsentInfo) and rely on callers to pass the existing value when they want to keep it unchanged (as the QA view-model already does).
export const setAnalyticsConsentInfo = (info: Partial<AnalyticsConsentInfo>) =>
saveAnalyticsConsentInfo(info);
apps/ledger-live-desktop/src/renderer/screens/settings/sections/Developer/AnalyticsConsentOptInDevTool/useAnalyticsConsentOptInQaViewModel.ts:98
formatBareValuecurrently rendersundefinedas"null", which can mislead when inspecting raw remote-config values (e.g. missingpolicyVersion). It also doesn't guard against values that stringify toundefined(e.g.Symbol) or throw (BigInt). Consider distinguishingundefinedexplicitly and falling back toString(value)when JSON serialization isn't usable.
export function formatBareValue(value: unknown): string {
if (value === null || value === undefined) return "null";
return JSON.stringify(value);
}
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 14 out of 14 changed files in this pull request and generated no new comments.
Suppressed comments (1)
apps/ledger-live-desktop/src/renderer/screens/settings/sections/Developer/AnalyticsConsentOptInDevTool/useAnalyticsConsentOptInQaViewModel.ts:5
- This new QA screen ViewModel is introduced under
apps/ledger-live-desktop/src/renderer/..., but the repo MVVM guidelines state that new desktop code should live underapps/ledger-live-desktop/src/mvvm/(see.agents/skills/mvvm-architecture/SKILL.md:10-13). To stay aligned with the migration, this hook (and ideally the QA screen) should be moved into an appropriatesrc/mvvm/features/...folder (with a View/Container split if needed) rather than adding more new code in the legacyrenderer/tree.
import { useEffect, useState } from "react";
import { useNavigate } from "react-router";
import { useFeature } from "@features/platform-feature-flags";
import { featureFlagsOverridesSelector, setOverride } from "@shared/feature-flags";
import {
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 9 out of 9 changed files in this pull request and generated no new comments.
Suppressed comments (2)
apps/ledger-live-desktop/src/renderer/screens/settings/sections/Developer/AnalyticsConsentOptInDevTool/useAnalyticsConsentOptInQaViewModel.ts:162
rawconsent-date display uses a truthy check, so an invalid-but-present value like an empty string will be shown asconsentDate: null, which misrepresents what is actually stored. Use an explicit null check so invalid strings still display their raw value.
raw: consentInfo.consentDate
? `consentDate: ${formatBareValue(consentInfo.consentDate)}`
: "consentDate: null",
status: consentDateStatus,
apps/ledger-live-desktop/src/renderer/screens/settings/sections/Developer/AnalyticsConsentOptInDevTool/useAnalyticsConsentOptInQaViewModel.ts:98
formatBareValuecurrently returns the string "null" for bothnullandundefined, which hides the distinction in the inspector output (and can mislead debugging). It’s safer to renderundefinedexplicitly and fall back when JSON.stringify throws.
This issue also appears on line 159 of the same file.
export function formatBareValue(value: unknown): string {
if (value === null || value === undefined) return "null";
return JSON.stringify(value);
}
|


✅ Checklist
npx changesetwas attached.- Developer → Analytics consent QA screen (desktop Settings → Developer)
- Dialog preview mounts in place on desktop QA screen (no portfolio navigation)
- Collapses deprecated
DANGEROUSLY_*QA settings helpers intosetAnalyticsConsentInfo📝 Description
Upgrade the desktop analytics consent QA debug screen to scenario parity with mobile (scenario groups, inspector fields, verdicts, preview-in-place via
forceOffer).Depends on shared inspector helper in #20330.
debug.desktop.mp4
❓ Context
🧐 Checklist for the PR Reviewers