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

πŸ’„ Ledger Sync copy and UX improvements #8916

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
6 changes: 6 additions & 0 deletions .changeset/itchy-camels-sin.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
"ledger-live-desktop": minor
"live-mobile": minor
---

LS - Copy and UX improvements
Original file line number Diff line number Diff line change
Expand Up @@ -15,12 +15,6 @@ import { openModal } from "~/renderer/actions/modals";
import { useHideEmptyTokenAccounts } from "~/renderer/actions/settings";
import { Icons } from "@ledgerhq/react-ui";
import { useFeature } from "@ledgerhq/live-common/featureFlags/index";
import { setDrawerVisibility as setLedgerSyncDrawerVisibility } from "~/renderer/actions/walletSync";
import { useFlows } from "LLD/features/WalletSync/hooks/useFlows";
import {
AnalyticsPage,
useLedgerSyncAnalytics,
} from "LLD/features/WalletSync/hooks/useLedgerSyncAnalytics";

const Separator = styled.div`
background-color: ${p => p.theme.colors.palette.divider};
Expand Down Expand Up @@ -53,15 +47,7 @@ const OptionsButton = () => {

const lldLedgerSyncFF = useFeature("lldWalletSync");
const isLedgerSyncEnabled = lldLedgerSyncFF?.enabled;
const { goToWelcomeScreenWalletSync } = useFlows();

const { onClickTrack } = useLedgerSyncAnalytics();

const openLedgerSyncDrawer = () => {
goToWelcomeScreenWalletSync();
onClickTrack({ button: "Access Ledger Sync", page: AnalyticsPage.Accounts });
dispatch(setLedgerSyncDrawerVisibility(true));
};
const items: ItemType[] = [
{
key: "exportOperations",
Expand All @@ -70,14 +56,7 @@ const OptionsButton = () => {
onClick: () => onOpenModal("MODAL_EXPORT_OPERATIONS"),
},
...(isLedgerSyncEnabled
? [
{
key: "exportAccounts",
label: t("accounts.optionsMenu.ledgerSync"),
icon: <Icons.Refresh size="S" />,
onClick: openLedgerSyncDrawer,
},
]
? []
: [
{
key: "exportAccounts",
Expand Down
4 changes: 2 additions & 2 deletions apps/ledger-live-desktop/static/i18n/en/app.json
Original file line number Diff line number Diff line change
Expand Up @@ -6743,15 +6743,15 @@
"description": "Sync with the Ledger Live app on another phone",
"steps": {
"step1": "Open the Ledger Live app you want to sync",
"step2": "Go to <0>Settings</0> > <0>General</0> > <0>Ledger Sync</0> > <0>Turn on sync</0> > <0>Show QR</0>",
"step2": "Go to <0>Settings</0> > <0>General</0> > <0>Ledger Sync</0> > <0>I already turned it on</0> > <0>Show QR</0>",
"step3": "Scan QR code"
}
},
"desktop": {
"description": "Synchronize your accounts with another Ledger Live app",
"steps": {
"step1": "Open the Ledger Live app you want to sync",
"step2": "Go to <0>Settings</0> > <0>General</0> > <0>Ledger Sync</0> > <0>Turn on sync</0> > <0>Use your Ledger</0>",
"step2": "Go to <0>Settings</0> > <0>General</0> > <0>Ledger Sync</0> > <0>I already turned it on</0> > <0>Use your Ledger</0>",
"step3": "Connect your Ledger"
}
}
Expand Down
4 changes: 2 additions & 2 deletions apps/ledger-live-mobile/src/locales/en/common.json
Original file line number Diff line number Diff line change
Expand Up @@ -7031,7 +7031,7 @@
"title": "Scan and synchronize your accounts using another Ledger Live app",
"steps": {
"step1": "Open the Ledger Live app you want to sync",
"step2": "Go to <0>Settings</0> <1>></1> <0>General</0> <1>></1> <0>Ledger Sync</0> <1>></1> <0>Turn on Ledger Sync</0> <1>></1> <0>Scan QR code</0>",
"step2": "Go to <0>Settings</0> <1>></1> <0>General</0> <1>></1> <0>Ledger Sync</0> <1>></1> <0>I already turned it on</0> <1>></1> <0>Scan QR code</0>",
"step3": "Scan QR code"
}
}
Expand All @@ -7043,7 +7043,7 @@
"title": "To sync with Ledger Live on another phone or computer...",
"steps": {
"step1": "Open the Ledger Live you want to sync with.",
"step2": "Go to <0>Settings</0> <1>></1> <0>General</0> <1>></1> <0>Ledger Sync</0> <1>></1> <0>Turn on Ledger Sync</0> <1>></1> <0>Show QR</0>",
"step2": "Go to <0>Settings</0> <1>></1> <0>General</0> <1>></1> <0>Ledger Sync</0> <1>></1> <0>I already turned it on</0> <1>></1> <0>Show QR</0>",
"step3": "Scan QR code"
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,10 @@ const ActivationFlow = ({
return (
<>
<TrackScreen category={AnalyticsPage.ActivateLedgerSync} />
<Activation onSyncMethodPress={navigateToChooseSyncMethod} />
<Activation
onSyncMethodPress={onCreateKey}
navigateToChooseSyncMethod={navigateToChooseSyncMethod}
/>
</>
);
case Steps.ChooseSyncMethod:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,9 @@ import { useInitMemberCredentials } from "../../hooks/useInitMemberCredentials";
import { useFeature } from "@ledgerhq/live-common/featureFlags/index";
import { Linking } from "react-native";

type Props = { onSyncMethodPress: () => void };
type Props = { onSyncMethodPress: () => void; navigateToChooseSyncMethod: () => void };

const Activation: React.FC<Props> = ({ onSyncMethodPress }) => {
const Activation: React.FC<Props> = ({ onSyncMethodPress, navigateToChooseSyncMethod }) => {
const { colors } = useTheme();
const { t } = useTranslation();
const walletSyncFF = useFeature("llmWalletSync");
Expand All @@ -20,7 +20,7 @@ const Activation: React.FC<Props> = ({ onSyncMethodPress }) => {

const onPressSyncAccounts = () => onSyncMethodPress();

const onPressHasAlreadyCreatedAKey = () => onSyncMethodPress();
const onPressHasAlreadyCreatedAKey = () => navigateToChooseSyncMethod();
Comment on lines 21 to +23
Copy link
Member

Choose a reason for hiding this comment

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

Why do we redefine a new function at each render?
I think that we can use the one passed as props directly in the jsx, isn't it?


const onPressLearnMore = () => {
if (learnMoreLink) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ function View() {
marginTop: 114,
}}
>
<Activation onSyncMethodPress={onOpenDrawer} />
<Activation onSyncMethodPress={onOpenDrawer} navigateToChooseSyncMethod={onOpenDrawer} />
Copy link
Contributor

Choose a reason for hiding this comment

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

What's the difference?
We should have different analytics no?

or do another action?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The tracking is done in the children component and will be different for each button but we have to pass two different methods in the other place where we're using the Activation component
Screenshot 2025-01-17 at 10 27 29
Screenshot 2025-01-17 at 10 28 39


<ActivationDrawer
startingStep={Steps.ChooseSyncMethod}
Expand Down
Loading