-
Notifications
You must be signed in to change notification settings - Fork 79
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(@desktop/wallet): Create a new send module to clear out old logic and switch the old one to the new one later, once the old sendModal is not used anymore #16986
base: master
Are you sure you want to change the base?
Conversation
Jenkins BuildsClick to see older builds (14)
|
8c89690
to
e0a6422
Compare
c883d39
to
77368f4
Compare
b6919a3
to
a40b6e6
Compare
…c and switch the old one to the new one later, once the old sendModal is not used anymore fixes #16919
77368f4
to
cb09738
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.
LGTM overall
Some minor stuff inline
/** input property to set estimated time **/ | ||
property string estimatedTime | ||
/** input property to set estimated fees in fiat **/ | ||
property string estimatedFiatFees | ||
/** input property to set estimated fees in crypto **/ | ||
property string estimatedCryptoFees | ||
|
||
/** property to set currently selected send type **/ | ||
property string sendType: Constants.SendType.Transfer |
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.
property string sendType: Constants.SendType.Transfer | |
property int sendType: Constants.SendType.Transfer |
It's an int
on Constants
@@ -120,6 +123,17 @@ StatusDialog { | |||
/** Output signal to inform that the forms been updated **/ | |||
signal formChanged() | |||
|
|||
/** function exposed to check if the form is filled correctly **/ | |||
function allValuesFilledCorrectly() { |
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.
Could be a readonly property
?
- key [string] - unique identifier of an asset | ||
- decimals [int] - decimals of the token | ||
- marketDetails [QObject] - collectible's contract address | ||
- currencyPrice [CurrenctAmount] - assets market price in CurrencyAmount |
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.
- currencyPrice [CurrenctAmount] - assets market price in CurrencyAmount | |
- currencyPrice [CurrencyAmount] - assets market price in CurrencyAmount |
currentCurrency: root.currentCurrency | ||
fnFormatCurrencyAmount: root.fnFormatCurrencyAmount | ||
fnResolveENS: root.fnResolveENS | ||
fnResolveENS: root.fnResolveENS |
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.
trailing space
fixes #16919
What does the PR do
Connects the Simple Send to backend.
Created new send module which removed all handling of UI states on the nim side
Uses the new format of router from go side, instead of the old one
Remaining:
Affected areas
SimpleSendModal
Architecture compliance
My PR is consistent with this document: Status Desktop Architecture Guide
Screenshot of functionality (including design for comparison)
Screen.Recording.2024-12-19.at.11.56.56.PM.mov