-
Notifications
You must be signed in to change notification settings - Fork 81
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(wallet): Update recipient data in send sign modal #17164
Conversation
ed7058a
to
07ea8f2
Compare
Jenkins Builds
|
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 in general
ens [string] - ens of recipient | ||
emoji [string] - emoji of recipient wallet | ||
color [string] - color of recipient wallet | ||
colorId [string] - colorId of recipient wallet |
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.
colorId [string] - colorId of recipient wallet | |
colorId [int] - colorId of recipient wallet |
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.
Also in the delegate itself, should be an int
, not string
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.
@@ -67,6 +81,29 @@ QObject { | |||
!!selectedAssetContractEntry.item ? | |||
selectedAssetContractEntry.item.address: "" | |||
|
|||
/** output property of the selected recipient address **/ |
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.
do these need to separate properties here? we can simple expose the item from here do the necessary logic in UI?
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.
They are passed as separate, because recipient can also be unknown address. In such case item would be empty.
I used separate properties to hide logic of checking for item availability. That way other components don't care whether it is wallet account or saved account or unknown address (only address property).
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.
Some minor comments from me
07ea8f2
to
54c6ef2
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!
Closes #17114
What does the PR do
Affected areas
Wallet - simple sign modal
Architecture compliance
My PR is consistent with this document: Status Desktop Architecture Guide
Screenshot of functionality (including design for comparison)