-
Notifications
You must be signed in to change notification settings - Fork 448
Feature/live 20724 [LLD][UI] Memo screen #13505
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
base: develop
Are you sure you want to change the base?
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. 4 Skipped Deployments
|
c2c5014 to
e6209fb
Compare
e6209fb to
e366821
Compare
e366821 to
2fa9d7d
Compare
2fa9d7d to
58c6911
Compare
58c6911 to
2991f8c
Compare
e797503 to
8ce386f
Compare
933cb0c to
19766e7
Compare
| import { useRecipientMemo } from "../screens/Recipient/hooks/useRecipientMemo"; | ||
| import { SEND_FLOW_STEP, type SendFlowStep, type SendStepConfig } from "../types"; | ||
| import { getRecipientDisplayValue, getRecipientSearchPrefillValue } from "./utils"; | ||
| import { useAvailableBalance } from "../hooks/useAvailableBalance"; |
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.
You can keep the import instead of adding the whole function inside the file
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.
done
| t, | ||
| showMemoControls, | ||
| currencyId, | ||
| memoViewModel.showMemoValueInput, |
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.
Can we regroup thoses 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.
done
| } | ||
|
|
||
| const address = entry.address; | ||
| const ensName = typeof entry.ensName === "string" ? entry.ensName : undefined; |
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.
Why the adds of all thoses isRecord / typeof checks ?
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.
old code, issue with commit hash, I had a previous version of the send flow, fixed now !
.zed/debug.json
Outdated
| // | ||
| // For more documentation on how to configure debug tasks, | ||
| // see: https://zed.dev/docs/debugger | ||
| [] |
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.
accidental file?
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.
yes, added to gitignore
19766e7 to
0097800
Compare
|
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.
Your SendHeader should use MVVM at this stage IMO
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.
Done. I will add unit test in an other PR for the new hook π
| setRecipient(recipient); | ||
| transactionHook.actions.setRecipient(recipient); | ||
| setRecipient(prev => (prev ? { ...prev, ...recipient } : recipient)); |
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.
why do it 2 times?
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.
fixed
| const getRecentDescription = (): string => { | ||
| const getRecentDescription = (): string | undefined => { | ||
| if (matchedRecentAddress) { | ||
| return `Already used Β· ${formatRelativeDate(matchedRecentAddress.lastUsedAt)}`; |
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.
should be translated no?
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.
done
6c962c4 to
f1209ce
Compare
f1209ce to
8feb951
Compare
|
|
/generate-screenshots |
1 similar comment
|
/generate-screenshots |


β Checklist
npx changesetwas attached.π Description
What does this PR do ?
This PR implements the memo management in the new Send flow.
Context
Some chain implements a system of
memolike for fiat transaction which allow to add a string to a string.We need to implement this into the new Send flow.
What have changed
Screenshots
β Context
π§ Checklist for the PR Reviewers