-
Notifications
You must be signed in to change notification settings - Fork 327
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(llm): 🏷️ add memo/tag input in some coins send flow #8155
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 5 Skipped Deployments
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it possible to make one generic MemoTag? like
import React from "react";
import { AnimatedInput } from "@ledgerhq/native-ui";
import { MemoTagInputProps } from "LLM/features/MemoTag/types";
// Make the component generic, accepting a transaction type `T`.
export default function MemoTagInput<T>({ onChange, ...inputProps }: MemoTagInputProps<T>) {
const [value, setValue] = React.useState("");
const handleChange = (text: string) => {
const value = text;
setValue(value);
onChange({
patch: { memo: value || undefined },
isEmpty: !value,
});
};
return <AnimatedInput {...inputProps} value={value} onChangeText={handleChange} />;
}
and then use it in each family ?
import type { Transaction as CardanoTransaction } from "@ledgerhq/live-common/families/cardano/types";
<MemoTagInput<CardanoTransaction>
onChange={handleCardanoChange}
/* other props */
/>;
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice one 🙌
It really reduced the boilerplate on every coins
@@ -146,7 +159,17 @@ export default function SendSelectRecipient({ navigation, route }: Props) { | |||
setTransaction(bridge.updateTransaction(transaction, {})); | |||
}, [setTransaction, account, parentAccount, transaction]); | |||
|
|||
const onPressContinue = useCallback(async () => { | |||
const [memoTagDrawerState, setMemoTagDrawerState] = useState<"INITIAL" | "SHOWING" | "SHOWN">( | |||
"INITIAL", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe use a dedicated enum or type?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
✅ Checklist
npx changeset
was attached.📝 Description
Screen.Recording.2024-10-22.at.10.53.12.mov
Add a memo tag input on all coins where it had been implemented on the summary step except Stellar. Stellar will be done on a follow up PR.
List of the coins:
❓ Context
🧐 Checklist for the PR Reviewers