External signer wallet create and send#547
External signer wallet create and send#547johnny9 wants to merge 11 commits intobitcoin-core:qt6from
Conversation
johnny9
changed the title
|
Related to #525 |
epicleafies
left a comment
epicleafies
left a comment
There was a problem hiding this comment.
All tests pass expect the qml_test_external_signer.py. There's also some other non-blocking issues.
22f0b7d
| } | ||
| } | ||
|
|
||
| ContinueButton { |
There was a problem hiding this comment.
This should be centered on the page.
| } | ||
| } | ||
|
|
||
| Separator { |
There was a problem hiding this comment.
Separator at the bottom is inconsistent with rest of app design.
| } | ||
|
|
||
| ContinueButton { | ||
| id: confimationButton |
There was a problem hiding this comment.
Nit: confirmation misspelled, pre-existing but can fix here
| return QString::number(m_amount + m_fee); | ||
| } | ||
|
|
||
| BitcoinAmount* WalletQmlModelTransaction::totalAmount() const |
There was a problem hiding this comment.
Nit: only used in tests, probably should be removed if not used by qml.
| #include <qml/models/sendrecipientslistmodel.h> | ||
| #include <qml/models/walletqmlmodeltransaction.h> | ||
|
|
||
| #include <common/args.h> |
There was a problem hiding this comment.
Nit: unused imports: common/args.h, common/settings.h, univalue.h (may be needed by cpp, if so should only appear there).
| } | ||
| } | ||
|
|
||
| ContinueButton { |
There was a problem hiding this comment.
UX: The button can be enabled or disabled but even when enabled the verification can still fail. Seems like it should not ever be disabled and all logic for checking the path should be in the button or the button should only be enabled when the device is valid and the button is just for proceeding. Not sure why logic is split.
| onTextChanged: walletController.clearWalletLoadStatus() | ||
| } | ||
|
|
||
| CoreText { |
There was a problem hiding this comment.
UX: Error message for using name of wallet that already exists is too technical, should probably be something like "Wallet with this name already exists." Detailed error should be viewable in debug log or similar if user wants details. (May not be in scope)
| } | ||
| Component { | ||
| id: external_confirm | ||
| CreateConfirm { |
There was a problem hiding this comment.
Suggestion: Back button probably shouldn't be on the confirm screen as the action taken to get here isn't reversible.
There was a problem hiding this comment.
Suggestion: The multiple send review page style should match the style of the send page. Maybe a single send review component?
| font.pixelSize: 15 | ||
| color: Theme.color.neutral9 | ||
| } | ||
| LabeledValueField { |
There was a problem hiding this comment.
UI: The note text is left shifted when everything else is right shifted.
2 participants
Uh oh!
There was an error while loading. Please reload this page.